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

[WIP] Nonnull operator #5

Closed
wants to merge 50 commits into from
Closed

[WIP] Nonnull operator #5

wants to merge 50 commits into from

Conversation

twof
Copy link
Collaborator

@twof twof commented May 29, 2021

No description provided.

IvanGoncharov and others added 30 commits May 28, 2021 18:41
…graphql#3088)

using this `find src -name "*.js" -exec sh -c 'git mv "$0" "${0%.js}.ts"' {} \;` shell command renamed all files in `src`
Find this pattern `(\$)FlowFixMe\[(.*?)\]` and replace all
Using this patttern`(\$)FlowExpectedError\[(.*?)\]`
Comment on lines +658 to +661
// This is invalid since an object could potentially be both the Object
// type IntBox and the interface type NonNullStringBox1. While that
// condition does not exist in the current schema, the schema could
// expand in the future to allow this. Thus it is invalid.
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this comment needs to be updated to describe the test. It looks like this test is expected to succeed, but the comment indicates that it should fail

Comment on lines +711 to +714
// This is invalid since an object could potentially be both the Object
// type IntBox and the interface type NonNullStringBox1. While that
// condition does not exist in the current schema, the schema could
// expand in the future to allow this. Thus it is invalid.
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same thing here

Comment on lines +1331 to +1332
message: 'Cannot return null for non-nullable field Food.name.',
path: ['food', 'theNameOfTheFood'],
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In this case, we'd want the message to use the alias instead of the field name right?

@twof
Copy link
Collaborator Author

twof commented Jun 9, 2021

Added a bunch of cases to executor-tests.ts. I'm not sure I'm following mocha conventions though, so if someone TS savvy wants to take a look, that'd be awesome.

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.

6 participants