-
Notifications
You must be signed in to change notification settings - Fork 2.1k
No {Ignored}
tokens when parsing schema coordinates
#4450
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
No {Ignored}
tokens when parsing schema coordinates
#4450
Conversation
This reverts commit ed6f978.
- Add support for meta-fields (e.g. `__typename`) - Add support for introspection types - Revert back from FieldCoordinate+ValueCoordinate -> MemberCoordinate
@magicmark I was starting to write an alternative here, but decided to just implement it, so check out #4451 - it adds a single commit on top of this PR that refactors to using an alternative lexer for schema coordinates. (If you like it just cherry-pick that commit into this PR and we can close the other one.) |
@benjie done! cherry picked. I also added a test for |
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.
Looks good to me, but @JoviDeCroock / @yaacovCR should sign off in the changes to the public API
describe('SchemaCoordinateLexer', () => { | ||
it('can be stringified', () => { | ||
const lexer = new SchemaCoordinateLexer(new Source('Name.field')); | ||
expect(Object.prototype.toString.call(lexer)).to.equal( |
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 should be roughly equivalent?
expect(Object.prototype.toString.call(lexer)).to.equal( | |
expect(String(lexer)).to.equal( |
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.
deferring to prior art / established convention https://github.com/search?q=repo%3Agraphql%2Fgraphql-js%20Object.prototype.toString.call&type=code
(but I agree with you)
I took a stab at @benjie's suggestion (which I 100% agree with) to disallow ignored tokens in schema coordinates.
(This caught an instance of whitespace in on of our tests!)
cc @yaacovCR @martinbonnin