-
Notifications
You must be signed in to change notification settings - Fork 2.7k
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
fix(fragment): fragment on query fix #3484
Conversation
Generated by 🚫 dangerJS |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This looks great @abhiaiyer91 - thanks for looking into this! Any chance you could add some tests for this? Your tests in apollographql/react-apollo#1987 were great; maybe they could be re-hashed for AC specifically? If you don't have time to finalize this right now, let me know and I'll dive in. Thanks again!
I shall! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi @abhiaiyer91 - just a couple more things, then we should be all set here:
- It looks like some of the
apollo-cache-inmemory
tests are failing due to these changes impacting the way inline fragments work. TheidValue.id === 'ROOT_QUERY'
check is returning true for all inline fragments in the failing tests, even though those tests intentionally have some inline fragments querying against invalid types. - The test you've added doesn't seem to fail without the
idValue.id === 'ROOT_QUERY'
change. It would be awesome if we could make sure it fails before these changes are applied, so we know we have our regression bases covered.
Thanks!
For this issue #3402