Skip to content

Cloudflare integration test#458

Open
vladinator1000 wants to merge 9 commits into
apollographql:mainfrom
vladinator1000:pr/react-router-cloudflare-itest
Open

Cloudflare integration test#458
vladinator1000 wants to merge 9 commits into
apollographql:mainfrom
vladinator1000:pr/react-router-cloudflare-itest

Conversation

@vladinator1000

Copy link
Copy Markdown

Adds an integration test for Cloudflare Workers. Useful to check if we run well on runtimes that don't support Node APIs such as renderToPipeableStream.

@changeset-bot

changeset-bot Bot commented Mar 25, 2025

Copy link
Copy Markdown

⚠️ No Changeset found

Latest commit: 9ca17e3

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

@vercel

vercel Bot commented Mar 25, 2025

Copy link
Copy Markdown

@vladinator1000 is attempting to deploy a commit to the Apollo Client - Next package - integration tests Team on Vercel.

A member of the Team first needs to authorize it.

@vladinator1000 vladinator1000 left a comment

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

@phryneas got another one for you 👀. This adds an integration test for Cloudflare Workers.
The idea is to check runtimes that don't support Node's pipeable stream API. I duplicated the existing React Router template and added comments to the files I changed. The most significant change is in entry.server.tsx, where I'm trying to get it to work with cross-platform streaming APIs.

The diff is a little noisy because I piggybacked on your async loader branch, so I'll wait before that's in before undrafting the PR, but I'm curious about your first opinions on the additions. What do you think?

Comment thread integration-test/playwright/src/cc-dynamic.test.ts Outdated
Comment on lines +30 to +74
const abortController = new AbortController();
setTimeout(() => {
abortController.abort(`Rendering exceed timeout of ${streamTimeout}ms`);
}, streamTimeout + 1000);

responseHeaders.set("Content-Type", "text/html");

const stream = await renderToReadableStream(
<ApolloProvider client={client}>
<ServerRouter
context={routerContext}
url={request.url}
nonce={options?.nonce}
/>
</ApolloProvider>,
{
...options,
signal: abortController.signal,
onError(error: unknown) {
responseStatusCode = 500;
console.error(error);
},
}
);

// BUG: I tried to port this over /integration-test/react-router/app/entry.server.tsx
// But it breaks with a hydration error if I manually set shouldWaitForAllContent = true.
// Ensure requests from bots and SPA Mode renders wait for all content to load before responding
// https://react.dev/reference/react-dom/server/renderToPipeableStream#waiting-for-all-content-to-load-for-crawlers-and-static-generation
const shouldWaitForAllContent = (userAgent && isbot(userAgent)) || routerContext.isSpaMode;

if (shouldWaitForAllContent) {
await stream.allReady;
}

return new Response(stream, {
headers: responseHeaders,
status: responseStatusCode,
})
}

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Tried to port https://github.com/apollographql/apollo-client-nextjs/blob/next/integration-test/react-router/app/entry.server.tsx

But it doesn't seem to work yet, because I expected to see "Queried in SSR environment", but got "Queried in Browser environment"
image

The bot waiting is also broken in the same way.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I'd assume it might be better if you start with whatever template React Router has for cloudflare and then apply the changes from our setup instructions to that.


export default {
fetch(request, env, ctx) {
return requestHandler(request, {

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Server entrypoint.

ssr: true,
presets,
future: {
unstable_viteEnvironmentApi: true,

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Enables use of @cloudflare/vite-plugin in vite.config.ts


export default defineConfig({
plugins: [
cloudflare({ viteEnvironment: { name: "ssr" } }),

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Use the workerd runtime in development.

@phryneas phryneas force-pushed the pr/react-router-cloudflare-itest branch from e16452a to c46fa38 Compare March 26, 2025 09:02
@phryneas

phryneas commented Mar 26, 2025

Copy link
Copy Markdown
Member

Heads up: I'm rebasing & force-pushing this to prevent an unrelated commit from making it in - please pull before you continue to work on this.

If this causes you too much trouble, just force-push your version again, I can do this later, too. It just makes reviewing easier for me right now :)

@vladinator1000 vladinator1000 marked this pull request as ready for review March 26, 2025 14:09
@vladinator1000 vladinator1000 requested a review from a team as a code owner March 26, 2025 14:09

@vladinator1000 vladinator1000 left a comment

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Here it is @phryneas 👀

The one thing that confuses me is, when I go to http://localhost:3000/asyncLoader, why does the react-router-cloudflare display
image

While the react-router test shows
image

"react": "^19.0.0",
"react-dom": "*",
"react-router": "^7.2.0-pre.3",
"react-router": "^7.4.0",

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Update to React Router 7.4.0

Comment thread integration-test/playwright/src/cc-dynamic.test.ts Outdated
const userAgent = request.headers.get("user-agent");
const client = makeClient(request);

const abortController = new AbortController();

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

@phryneas

Copy link
Copy Markdown
Member

My day is coming to an end soon, but I hope I can get to this tomorrow - I'll also see if I can set up a cloudflare deployment so we can test this as a deployed integration test.

@phryneas phryneas deleted the branch apollographql:main March 31, 2025 10:14
@phryneas phryneas closed this Mar 31, 2025
@phryneas phryneas reopened this Apr 3, 2025
@phryneas phryneas changed the base branch from next to main April 3, 2025 15:08
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.

2 participants