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

Namespace conflict with webonyx/graphql-php #60

Open
kirkmadera opened this issue Jun 14, 2021 · 4 comments
Open

Namespace conflict with webonyx/graphql-php #60

kirkmadera opened this issue Jun 14, 2021 · 4 comments

Comments

@kirkmadera
Copy link

webonyx/graphql-php is a popular PHP GraphQL server library. I was going to use this module until I discovered that it uses the namespace GraphQl and directly conflicts with webonyx/graphql-php. Any plans to put this under a namespace like MGhoneimy\GraphQl\? This is the only way I could use this module.

@kirkmadera
Copy link
Author

I figured out that it happens to work, so long as there are no class naming conflicts. I am only using this as a temporary solution to proxy a middleware until I can replace all GraphQL calls, so this is acceptable. I do think it would make sense to prefix the namespace though. Assuming ownership of \GraphQl makes this not play nice with other libraries. To be honest, webonyx/graphql-php is doing the same thing and should not try to own this namespace, but it's already an established library.

@xaviered
Copy link

@kirkmadera if you want, you can get started with the PRs I just merged on my fork.
@mghoneimy feel free to merge to your repo if you'd like. Wasn't sure what vendor name to pick, so I used the one on packagist.org

@mghoneimy
Copy link
Owner

Hi @kirkmadera ,
While you are completely correct about this, making such a change to the namespaces would break usage for all clients which is not acceptable, and to be honest, I haven't had many complaints about this.
So here's what I think I should do:

  1. Keep this as it is for the time being to maintain backward-compatibility
  2. Modify the namespaces for V2.0 since it's going to be backwards incompatible anyways

@kirkmadera
Copy link
Author

Definitely makes sense to make a change like this in a major version change. This is functional at the moment with sharing the same namespace because there are not any class name conflicts as far as I have seen. When/if you do push a 2.x with a namespace change, we will be able to migrate to it.

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

No branches or pull requests

3 participants