Skip to content

Conversation

@tmeasday
Copy link
Contributor

See notes on commit, in particular

Note that we are simply checking fragment strings for equality, not ensuring that fragment definitions within the larger graphql document have exact equality.

We could do the above, but it'd be quite a bit more complicated, and I'm not sure if there's much benefit. AFAICT the main reason for wanting that before was exactly this functionality.

Also normalize document strings (for whitespace anyway).

Note that we are simply checking fragment strings for equality, not ensuring that fragment definitions within the larger graphql document have exact equality.

We could do the above, but it'd be quite a bit more complicated, and I'm not sure if there's much benefit. AFAICT the main reason for wanting that before was exactly this functionality.
@tmeasday
Copy link
Contributor Author

@stubailo might want to have a look at this also.

@helfer
Copy link
Contributor

helfer commented Nov 30, 2016

@tmeasday I think this looks great! I think it's perfectly fine to be restrictive and require that a fragment with the same name be identical. I'd even go so far as to not strip whitespace.

Can't wait to remove fragment stuff from Apollo Client! I hear 0.6 is right around the corner... 😉

@tmeasday tmeasday merged commit 055feb6 into master Dec 1, 2016
@stubailo
Copy link
Contributor

stubailo commented Dec 1, 2016

Small test fixes needed: apollographql/apollo-client#976

@helfer helfer deleted the fragment-warnings branch December 13, 2016 10:30
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.

4 participants