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

introduce transformer from latest incremental format to legacy format #4319

Open
wants to merge 16 commits into
base: main
Choose a base branch
from

Conversation

yaacovCR
Copy link
Contributor

@yaacovCR yaacovCR commented Jan 7, 2025

The last attempt to support the both the current and legacy incremental delivery format in #4316 was a flop.1 This PR introduces a new one:

import { legacyExecuteIncrementally } from `graphql`;

const result = legacyExecuteIncrementally( ... );

The legacyExecuteIncrementally() function works just like experimentalExecuteIncrementally() from the latest graphql v17 alpha but returns the legacy format.2

The plan if this approach meets goals would probably be to introduce this as a separate library, perhaps capable of even more complex transformations. For now, you can use it instead of the latest graphql v17 alpha via canary release graphql@canary-pr-4319

Footnotes

  1. In that version we built a non-standard execution plan with one execution group per delivery group where the different execution groups had duplicated fields, instead of the spiffy non-duplicated version of the current format.

    Why did this not work?

    Consider the following operation:

    query {
      hero { nonNullName }
      ... @defer { hero { name } }
    }
    

    Assuming Hero.nonNullName is a non-nullable field and happens to return null, the null will "bubble up" to hero, i.e. the initial response for our new algorithm will simply be:

    {
      "data": {
        "hero": null,
      },
      "errors": [
        {
          "message": "Cannot return null for non-nullable field Hero.nonNullName.",
          "locations": [{ "line": 4, "column": 11 }],
          "path": ["hero", "nonNullName"],
        },
      ],
    }
    

    No deferred fragment will be delivered, because there is no where for a client to store this data => without undoing a received null. Our new algorithm manages this by simply discarding any requests to send deferred data whenever a null is returned.

    This could be kind of tricky; after all, the deferred fragment is declared at the root, before hero is encountered. The way this works is the all overlapping fields simultaneously at each level, and uses the information gathered to decide when to initiate deferral in a separate "execution plan step". For the root fields in our example, no special action is taken even though there is a deferred fragment at root, because the deferred hero field overlaps with the non-deferred hero. When hero returns null, we discard the request for nested deferred data.

    If deferral is not initiated, though, our algorithm still saves details about any requested deferral on all of the child selections. So for the non-error case, when planning the subfields of hero, the algorithm can still "remember" that Hero.name was requested as deferred and deliver just that necessary bit.

    In introduce legacy incremental delivery entrypoint #4316, we hacked that "execution plan step" to create that separate deferred fragment as if that overlap didn't occur, but we forgot that this will break the previous case! If we kick off a deferred fragment separately at root, we will return that fragment even if the initial result returns null.

  2. legacyExecuteIncrementally() uses experimentalExecuteIncrementally() under the hood, transforming the resulting latest format into the legacy format. If the new type of null-bubbling introduced within the latest format is encountered, the error will bubble up to the root of the deferred fragment.

Copy link

netlify bot commented Jan 7, 2025

Deploy Preview for compassionate-pike-271cb3 ready!

Name Link
🔨 Latest commit c9c11cf
🔍 Latest deploy log https://app.netlify.com/sites/compassionate-pike-271cb3/deploys/6785818edcc8c500085130b9
😎 Deploy Preview https://deploy-preview-4319--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 configuration.

Copy link

github-actions bot commented Jan 7, 2025

Hi @yaacovCR, 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

@yaacovCR

This comment has been minimized.

Copy link

@github-actions publish-pr-on-npm

@yaacovCR The latest changes of this PR are available on NPM as
graphql@17.0.0-alpha.7.canary.pr.4319.e7cfada212d5cdd7dc09890bef82ac9da32026eb
Note: no gurantees provided so please use your own discretion.

Also you can depend on latest version built from this PR:
npm install --save graphql@canary-pr-4319

@yaacovCR yaacovCR marked this pull request as ready for review January 14, 2025 12:00
@yaacovCR yaacovCR requested a review from a team as a code owner January 14, 2025 12:00
@yaacovCR

This comment has been minimized.

Copy link

@github-actions publish-pr-on-npm

@yaacovCR The latest changes of this PR are available on NPM as
graphql@17.0.0-alpha.7.canary.pr.4319.55799077dbb6c5ab0c33471482a66b91f048a516
Note: no gurantees provided so please use your own discretion.

Also you can depend on latest version built from this PR:
npm install --save graphql@canary-pr-4319

@yaacovCR yaacovCR marked this pull request as draft January 14, 2025 12:13
@yaacovCR yaacovCR changed the title initial version of current to legacy format transformer introduce transformer from latest incremental format to legacy format Jan 14, 2025
@yaacovCR yaacovCR marked this pull request as ready for review January 14, 2025 12:43
@yaacovCR

This comment has been minimized.

Copy link

@github-actions publish-pr-on-npm

@yaacovCR The latest changes of this PR are available on NPM as
graphql@17.0.0-alpha.7.canary.pr.4319.09f8d400782ed385ec6f8fb9d151e221414af613
Note: no gurantees provided so please use your own discretion.

Also you can depend on latest version built from this PR:
npm install --save graphql@canary-pr-4319

@yaacovCR yaacovCR added the PR: feature 🚀 requires increase of "minor" version number label Jan 14, 2025
@yaacovCR

This comment has been minimized.

Copy link

@github-actions publish-pr-on-npm

@yaacovCR The latest changes of this PR are available on NPM as
graphql@17.0.0-alpha.7.canary.pr.4319.abe5e8e0bb4a916f37d6d1b7fdcf128155a24a46
Note: no gurantees provided so please use your own discretion.

Also you can depend on latest version built from this PR:
npm install --save graphql@canary-pr-4319

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
PR: feature 🚀 requires increase of "minor" version number
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant