Skip to content

Conversation

robrichard
Copy link
Contributor

@robrichard robrichard commented Sep 19, 2022

Discussion topic: graphql/defer-stream-wg#55

This implements what was agreed on in the November 2022 (Secondary, EU) WG. There was consensus to move forward with disabling defer/stream on subscriptions via a field error. This also adds a new validation rule that prevents @defer or @stream from being used inside a subscription operation without an if argument that can be used to disable defer/stream.

@robrichard robrichard self-assigned this Sep 19, 2022
@netlify
Copy link

netlify bot commented Sep 19, 2022

Deploy Preview for compassionate-pike-271cb3 ready!

Name Link
🔨 Latest commit ebb5efb
🔍 Latest deploy log https://app.netlify.com/sites/compassionate-pike-271cb3/deploys/63a207e7ea12d200085fa6dd
😎 Deploy Preview https://deploy-preview-3742--compassionate-pike-271cb3.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site settings.

@robrichard robrichard removed their assignment Sep 19, 2022
@github-actions
Copy link

Hi @robrichard, I'm @github-actions bot happy to help you with this PR 👋

Supported commands

Please post this commands in separate comments and only one per comment:

  • @github-actions run-benchmark - Run benchmark comparing base and merge commits for this PR
  • @github-actions publish-pr-on-npm - Build package from this PR and publish it on NPM

@robrichard robrichard force-pushed the robrichard/disable-subscribe branch from e528a7e to 5a674d0 Compare September 19, 2022 17:55
Copy link
Contributor

@yaacovCR yaacovCR left a comment

Choose a reason for hiding this comment

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

Other than possibly tweaking a comment, these changes look good to me.

More generally, I am also fine with leaving incremental delivery support with subscriptions for a potential later stage to help advance the initial release.

@robrichard robrichard force-pushed the robrichard/disable-subscribe branch from 5a674d0 to 7fc599f Compare September 20, 2022 11:40
@robrichard
Copy link
Contributor Author

@IvanGoncharov this is ready for review, now implementing what was discussed at the November 2022 (Secondary, EU) WG. The spec PR has also been updated to reflect this.

@robrichard robrichard force-pushed the robrichard/disable-subscribe branch from 081f177 to ebb5efb Compare December 20, 2022 19:07
@IvanGoncharov IvanGoncharov added the PR: breaking change 💥 implementation requires increase of "major" version number label Dec 22, 2022
@IvanGoncharov IvanGoncharov merged commit 1bf71ee into graphql:main Dec 22, 2022
@robrichard robrichard deleted the robrichard/disable-subscribe branch December 22, 2022 13:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
PR: breaking change 💥 implementation requires increase of "major" version number
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants