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

implement multipart streaming for PreloadQuery #389

Open
wants to merge 20 commits into
base: next
Choose a base branch
from

Conversation

@phryneas phryneas changed the title implement multipart streaming implement multipart streaming for PreloadQuery Nov 13, 2024
Copy link

pkg-pr-new bot commented Nov 13, 2024

npm i https://pkg.pr.new/apollographql/apollo-client-nextjs/@apollo/client-react-streaming@389
npm i https://pkg.pr.new/apollographql/apollo-client-nextjs/@apollo/experimental-nextjs-app-support@389

commit: 5bacf2e

Copy link

relativeci bot commented Nov 13, 2024

#343 Bundle Size — 1.32MiB (+5.89%).

5bacf2e(current) vs 9d0a77a main#331(baseline)

Warning

Bundle contains 1 duplicate package – View duplicate packages

Bundle metrics  Change 7 changes Regression 2 regressions
                 Current
#343
     Baseline
#331
Regression  Initial JS 1.04MiB(+4.3%) 1016.77KiB
No change  Initial CSS 70B 70B
Change  Cache Invalidation 85.18% 0.08%
Change  Chunks 36(+5.88%) 34
Change  Assets 62(+5.08%) 59
Change  Modules 679(+6.76%) 636
Regression  Duplicate Modules 119(+12.26%) 106
Change  Duplicate Code 5.35%(+14.81%) 4.66%
No change  Packages 26 26
No change  Duplicate Packages 1 1
Bundle size by type  Change 2 changes Regression 2 regressions
                 Current
#343
     Baseline
#331
Regression  JS 1.31MiB (+5.86%) 1.24MiB
Regression  Other 10.01KiB (+10.12%) 9.09KiB
No change  CSS 70B 70B

Bundle analysis reportBranch pr/multipart-streamingProject dashboard


Generated by RelativeCIDocumentationReport issue

@phryneas phryneas changed the base branch from pr/framework-explorations to next December 18, 2024 09:56
Copy link

changeset-bot bot commented Dec 18, 2024

⚠️ No Changeset found

Latest commit: 5bacf2e

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

} from "graphql";
import { ObjMap } from "graphql/jsutils/ObjMap";

export class IncrementalSchemaLink extends ApolloLink {
Copy link
Member Author

Choose a reason for hiding this comment

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

For use in tests, essentially a SchemaLink with @defer support.

Comment on lines +80 to +86
rating: (source, args, context) => {
return new Promise((resolve) =>
setTimeout(resolve, Math.random() * 2 * args.delay, {
value: products.find((p) => p.id === source.id)?.rating,
env: getEnv(context),
})
);
Copy link
Member Author

Choose a reason for hiding this comment

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

adding a slow field that can be used to test @defer

return (
<ApolloWrapper>
<PreloadQuery
query={QUERY}
context={{
delay: 1000,
error: searchParams?.errorIn || undefined,
error: (await searchParams)?.errorIn || undefined,
Copy link
Member Author

Choose a reason for hiding this comment

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

These are still leftovers from changing to Next 15 and only surfaced now.

data: null,
errors: [new GraphQLError("Simulated error")],
});
if (errorConditions.includes("network_error")) {
Copy link
Member Author

Choose a reason for hiding this comment

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

This link can now simulate both GraphQLErrors and NetworkErrors in tests.

this.setLink(this.link);
}

setLink(newLink: ApolloLink) {
Copy link
Member Author

Choose a reason for hiding this comment

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

RSC: adds TeeToReadableStreamLink,
SSR & Browser: adds ReadFromReadableStreamLink for consumption

Comment on lines +34 to +37
}): Promise<React.ReactElement> {
const preloader = createTransportedQueryPreloader(await getClient());
const { query, ...transportedOptions } = options;
const queryRef = preloader(query, transportedOptions);
Copy link
Member Author

Choose a reason for hiding this comment

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

All the logic now moved into createTransportedQueryPreloader as that will also be used by other frameworks in SSR loaders

Comment on lines +59 to +65
const tryClose = () => {
try {
controller.close();
} catch {
// maybe we already tried to close the stream, nothing to worry about
}
};
Copy link
Member Author

Choose a reason for hiding this comment

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

This can be triggered from multiple different places, and we want the first call to close and the later calls to not error.

});
}
const client = useApolloClient();
reviveTransportedQueryRef(queryRef, client);
Copy link
Member Author

Choose a reason for hiding this comment

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

All of that logic now lives in reviveTransportedQueryRef as it will also be used by other framework implementations.

Comment on lines +117 to +133
client
.query({
query,
...options,
// ensure that this query makes it to the network
fetchPolicy: "network-only",
context: skipDataTransport(
teeToReadableStream(__injectIntoStream!, {
...options?.context,
// we want to do this even if the query is already running for another reason
queryDeduplication: false,
})
),
})
.catch(() => {
/* we want to avoid any floating promise rejections */
});
Copy link
Member Author

Choose a reason for hiding this comment

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

This is the core "preload into a ReadableStream" implementation

Comment on lines +178 to 193
const internalQueryRef = getSuspenseCache(client).getQueryRef(
cacheKey,
() =>
client.watchQuery({
...hydratedOptions,
fetchPolicy: "network-only",
context: skipDataTransport(
readFromReadableStream(stream.pipeThrough(new JSONDecodeStream()), {
...hydratedOptions.context,
queryDeduplication: true,
})
),
})
);
Object.assign(queryRef, wrapQueryRef(internalQueryRef));
}
Copy link
Member Author

Choose a reason for hiding this comment

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

This is the core "recreate a normal queryRef from a transported queryRef" implementation

@phryneas phryneas marked this pull request as ready for review December 18, 2024 11:12
@phryneas phryneas requested a review from a team as a code owner December 18, 2024 11:12
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.

1 participant