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

Fragment can't be imported more than once #1

Open
alexlafroscia opened this issue Sep 19, 2018 · 6 comments
Open

Fragment can't be imported more than once #1

alexlafroscia opened this issue Sep 19, 2018 · 6 comments

Comments

@alexlafroscia
Copy link

alexlafroscia commented Sep 19, 2018

I noticed that a particular fragment can't be imported more than once.

This becomes a bit confusing in cases where a fragment imports a fragment. I think importing the fragment a second time should result in a no-op rather than an error around the fragment being defined more than once.

Is this something you'd be interested in a PR to fix? If so, I would be happy to put one together!

@csantero
Copy link
Collaborator

I'm not sure I understand the problem. I've definitely reused fragments in my apps before. Do you have an example query that fails?

@alexlafroscia
Copy link
Author

alexlafroscia commented Sep 19, 2018

Yeah! So for example, if there is a "tree" of imports like this:

query.graphql
-> fragment-a.graphql
-> fragment-b.graphql
  -> fragment-a.graphql

an error occurs because fragment-a.graphql is defining a fragment more than once, and the names collide.

I'm kind of new to GraphQL but it seems like importing fragment-a the second time could safely be ignored to avoid this error.

Making sure not to import a fragment multiple times becomes challenging with many queries and fragments, where you are importing a given fragment many times from many different queries.

@csantero
Copy link
Collaborator

@alexlafroscia Ah I see - yeah I don't think I've ever had that sort of construct in my app. I'll be happy to look at a PR if you can include a failing test case. Thanks

@alexlafroscia
Copy link
Author

Sure thing 👍

@csantero
Copy link
Collaborator

@alexlafroscia Is this still a problem for you now that #9 has been merged?

@alexlafroscia
Copy link
Author

I don't really have a good way to test this anymore -- I'm no longer at the company where we were running into this. @jasonmit or @offirgolan might be able to follow up and check it out!

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

2 participants