-
Notifications
You must be signed in to change notification settings - Fork 1.8k
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
Allow configurable @defer
/@stream
names
#4467
Conversation
@captbaritone has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator. |
Looks like the "CI / Compiler output check" CI step is failing. Can update https://github.com/facebook/relay/blob/main/scripts/config.tests.json to opt back into the old names? |
6fffd63
to
9be04c8
Compare
9be04c8
to
a34e85f
Compare
@captbaritone all checks are passing now. It was failing because I updated |
@captbaritone has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator. |
1 similar comment
@captbaritone has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator. |
@robrichard Can you add a followup PR (I'll try to merge this one as is) to make the panics in this file into diagnostics? relay/compiler/crates/relay-transforms/src/defer_stream/directives.rs Lines 32 to 66 in 6fe9d76
These can now happen as a result of not specifying the the right config for your schema and no user error should result in a panic from the compiler. |
@captbaritone merged this pull request in 58da806. |
This is not an attempt to get Relay to conform to the current defer/stream spec draft, but rather just a small step in that direction by allowing configurable directive & argument names.
Currently Relay requires the argument
initial_count
on@stream
, but the GraphQL spec uses camelCase for all names.This PR updates Relay compiler to follow the same pattern used by
ConnectionInterface
, introducingDeferStreamInterface
to allow configuring the names and arguments of the defer & stream directive.The first commit adds the configuration updating all touch points to reference the configuration.
The second commit changes the defaults to camelCase, to match the defaults used in
ConnectionInterface
. This makes the diff substantially larger as many tests and fixtures need to be updated. I'm happy to remove this commit from this PR if desired.