Skip to content

feat: Support for generic types #2503

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 10 commits into
base: 5.x
Choose a base branch
from

Conversation

bnowak
Copy link
Contributor

@bnowak bnowak commented Jun 20, 2025

Description

It adds support for generating api schema of generic types.

What type of PR is this? (check all applicable)

  • Bug Fix
  • Feature
  • Refactor
  • Deprecation
  • Breaking Change
  • Documentation Update
  • CI

Checklist

  • I have made corresponding changes to the documentation (docs/)
  • I have made corresponding changes to the changelog (CHANGELOG.md)

@bnowak bnowak changed the title Support for generic types feat: Support for generic types Jun 20, 2025
@bnowak bnowak force-pushed the support-generic-types branch from 8b9c521 to cd32262 Compare June 27, 2025 07:43
@bnowak bnowak marked this pull request as ready for review June 27, 2025 11:41
@bnowak
Copy link
Contributor Author

bnowak commented Jun 27, 2025

Regarding generated model/component schema names for generic type classes, I was also thinking about improving it a bit in the same fashion as in phpdoc (eg. GenericClass<string>, GenericClass<int>, GenericClass<list<string>> etc.). IMO that would be much more readable for humans rather than current default approach with enumerating (GenericClass1, GenericClass2, GenericClass3 ...).

I have some wip code about that, however I spotted some issues:

  • ideally to handle names like that we'd need replace Model's legacy type property with version from TypeInfo component. It can store generic type variables, so that's enough to simply refactor/adjust ModelRegistry::getTypeShortName method and that's it. However, replacing that would be more work and we'd need to drop support for SF property-info component and make SF type-info required (now it's optional). So I guess that's option for future and rather long term solution.
  • the other option is to use introduced in this PR Model::$serializationContext[GenericClassDescriber::TEMPLATES_KEY] types map to resolve generic type names, however it seems like workaround with a few type-based and type-info component-based conditions in ModelRegistry codebase. It would be dirty, but would work.

@DjordyKoert could you kindly take a look on that PR please?

also, please respond with your feelings about above generation of model names for generics. If option 2 if fine for you - I could address that in separate PR :)

cheers

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant