Skip to content
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

Introduce a DecoratableTypeResolver base class #1213

Open
wants to merge 13 commits into
base: 8.x-4.x
Choose a base branch
from

Conversation

chrfritsch
Copy link
Contributor

While working on the graphql integration for the Thunder distribution, I discovered this useful base class in the OpenSocial project.
I think it would make a lot of sense to have it in the main graphql module.

@Kingdutch
Copy link
Contributor

Kingdutch commented Jun 17, 2021

For anyone interested you can view the current implementation in Open Social.

Could you run the following to add an interface? This would allow Open Social to implement the interface and have an upgrade path :D

curl https://gist.githubusercontent.com/Kingdutch/5990c24a2734c5fb01e26cf444f44c18/raw/a79b97a3411f65faef6121ceef36d7350a982bf6/type-resolver-interface.patch | git am

You can view the patch file and commit message at https://gist.github.com/Kingdutch/5990c24a2734c5fb01e26cf444f44c18

Kingdutch and others added 3 commits June 18, 2021 09:42
The interface allows alternative but compatible decoratable type
resolvers to be created. It also provides an upgrade path for projects
that were already using the pattern.
@klausi klausi added the 4.x label Jul 3, 2021
@klausi
Copy link
Contributor

klausi commented Jul 3, 2021

Thanks, I think this could be useful. I don't understand it fully yet, could you describe the use case in the description above a bit? Like with a concrete schema + type, the resolver setup snippet and what benefit this brings vs. a conventional resolver.

I got some hints from the Open Social comments, so we should definitely pull them in. Thanks @Kingdutch for supplying that here.

@klausi
Copy link
Contributor

klausi commented Jul 3, 2021

Ah, and we should have some test coverage that uses this class and demonstrates that it works.

@Kingdutch
Copy link
Contributor

Kingdutch commented Jul 7, 2021

I can add test coverage later this week. A quick usage example as seen in Open Social would be:

$registry->addTypeResolver('Actor', new UserActorTypeResolver($registry->getTypeResolver('Actor')));

I debated adding a $registry->chainTypeResolver('Actor', UserActorTypeResolver::class) method but it added a lot more code and unclarity for saving about 10 characters.

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

Successfully merging this pull request may close these issues.

4 participants