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

Add support of types from vendor packages #657

Closed
fogrye opened this issue Mar 6, 2024 · 9 comments · Fixed by #664
Closed

Add support of types from vendor packages #657

fogrye opened this issue Mar 6, 2024 · 9 comments · Fixed by #664

Comments

@fogrye
Copy link
Contributor

fogrye commented Mar 6, 2024

This is mainly for input types but could be considered for output also.

While you can use classes from various libraries you are limited to only current project folders in case you moved some input types to external package. In my case I have some where/sort classes + logic moved as shared library and want to have them used as types, I have added all attributes but SchemaFactory will ignore them as GlobClassExplorer only looks into project src.

I'm happily contribute to this one + https://github.com/thecodingmachine/class-explorer but that is a big change for class explorer and maintainers of that lib may not be happy about it.

I hope my request to support vendor packages types will find supporters as IMHO it's must have for complex projects.

Happily receive any feedback on this, thx.

@oojacoboo
Copy link
Collaborator

@fogrye Can you not just include the namespace for these classes in the SchemaFactory configuration?

SchemaFactory:: addTypeNamespace('Vendor\\Namespace');

@fogrye
Copy link
Contributor Author

fogrye commented Mar 7, 2024

@oojacoboo I wish so, but as readme in class explorer says - it just goes through project files but not also vendor files. Thats why I want to suggest this feature here and in class-explorer project.

@oojacoboo
Copy link
Collaborator

@fogrye I think it's reasonable to assume types might exist as vendor files and I agree that's something that should be optionally supported. That said, I'm assuming there are some performance considerations - that needs to be taken into account somehow.

I'm not a maintainer for class-explorer, but I'm assuming we could get a PR merged there if it's not breaking and there aren't any performance implications, or performance implications are optional based on vendor access needs.

@fogrye
Copy link
Contributor Author

fogrye commented Mar 12, 2024

@oojacoboo so seems to me that class-explorer is kinda dead, I found that @moufmouf was working on something what I actually need here but haven't finished his work. What do you think if I will finish that and we change dependency for this project to my fork?

@oojacoboo
Copy link
Collaborator

I'm wondering if there might be a better lib for handling all of this? Have you looked? Swapping out that lib might be a better option in the long term.

@fogrye
Copy link
Contributor Author

fogrye commented Mar 13, 2024

@oojacoboo I found a couple but not that good:

I'm personally more for 1st option, what do you think?

@oojacoboo
Copy link
Collaborator

oojacoboo commented Mar 14, 2024

@fogrye @oprypkhantc suggested looking at https://github.com/alekitto/class-finder/ as well. Looks like it supports Composer, which would be PSR0/4, etc.

fogrye added a commit to fogrye/graphqlite that referenced this issue Mar 14, 2024
Main issue is to let type mappers find types in vendor packages which class-explorer and maintainer is not updating the project.

Symfony and Laravel bundles have to be updated too.
Fixes: thecodingmachine#657
@fogrye
Copy link
Contributor Author

fogrye commented Mar 19, 2024

@oojacoboo so could you pls check the PR, I believe we can have it merged

oojacoboo pushed a commit that referenced this issue Mar 20, 2024
…664)

* feat: replace thecodingmachine/class-explorer with kcs/class-finder

Main issue is to let type mappers find types in vendor packages which class-explorer and maintainer is not updating the project.

Symfony and Laravel bundles have to be updated too.
Fixes: #657

* test: check that incorrect classes don't trigger autoloading errors

Thanks @oprypkhantc

* fix: stop caching reflections

* fix: static checks

* fix: stop using cached classes if restoring fails

* test: improve coverage

* fix: prevent possible issue with long-running apps

As each condition applied to finder stays there each place it is applied should be done on cloned instance to avoid accumulation. Main instance of finder is kept with all the conditions provided during configuring. NS is created only from NSFctory where it's cloned so no need to add same clone inside NS.
@oojacoboo
Copy link
Collaborator

Thanks again - merged.

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 a pull request may close this issue.

2 participants