Make site work with the Cloudflare OpenNext adapter #7383

dario-piotrowicz · 2025-01-02T18:50:56Z

This PR applies changes to make it so that the site can be deployed to Cloudflare workers using the open-next Cloudflare adapter

The app does seem to work as intended for the most part:

Deployment URL: https://nodejs-website.web-experiments.workers.dev

Warning

The PR is currently a single commit, I am planning to force push (with --force-with-lease) any new changes and keep this a single commit PR for now (so that it's easier to manage and rebase), if that's too annoying problematic for reviewers please let me know and I can just push standard commits if strongly preferred

Check List

I have read the Contributing Guidelines and made commit messages that follow the guideline.
I have run npm run format to ensure the code follows the style guide.
I have run npm run test to check if all tests are passing.
I have run npx turbo build to check if the website builds without errors.
I've covered new added functionality with unit tests if necessary.

vercel · 2025-01-02T18:50:59Z

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name	Status	Preview	Updated (UTC)
nodejs-org	✅ Ready (Inspect)	Visit Preview	Mar 3, 2025 0:00am

dario-piotrowicz · 2025-01-02T18:53:49Z

apps/site/next-data/providers/releaseData.ts

-const releaseData = await generateReleaseData();
-
-const provideReleaseData = cache(() => releaseData);
+const provideReleaseData = cache(() => generateReleaseData());


I made this change because in workerd async operations can't be executed at the top level of a dynamically imported module, and that's what was happening here

(I can add a code comment about it if it'd be helpful)

apps/site/scripts/prebuild.generator.mjs

apps/site/.cloudflare/opentelemetry.mjs

apps/site/instrumentation.ts

apps/site/next.config.mjs

apps/site/next.dynamic.mjs

apps/site/wrangler.toml

apps/site/tsconfig.json

apps/site/scripts/prebuild.generator.mjs

apps/site/package.json

apps/site/open-next.config.ts

apps/site/next.helpers.mjs

apps/site/next-data/providers/downloadSnippets.ts

ovflowd · 2025-02-01T20:17:15Z

@dario-piotrowicz do we have updates here? 👀

dario-piotrowicz · 2025-02-03T12:23:54Z

Hey @ovflowd 👋

Sorry for keeping the PR lingering, there are a few smaller issues we addressed in our adapter (that I need to reflect here), and also ISR should be coming soon (@vicb can provide more context)

Besides that I just need to rebase the PR, the only significant issue remaining should be filesystem access, but I wanted to clarify that with you, I'll drop you a message today to clarify things

(PS: I hope the PR is not bothering you 🙇, if you want I can close it and reopen it when we're ready?)

ovflowd · 2025-02-16T15:12:52Z

Sorry for keeping the PR lingering, there are a few smaller issues we addressed in our adapter (that I need to reflect here), and also ISR should be coming soon (@vicb can provide more context)

This is awesome news. Excited to hear from @vicb

Besides that I just need to rebase the PR, the only significant issue remaining should be filesystem access, but I wanted to clarify that with you, I'll drop you a message today to clarify things

I believe we sorted that out on Slack 🖖

(PS: I hope the PR is not bothering you 🙇, if you want I can close it and reopen it when we're ready?)

Not at all <3

apps/site/.cloudflare/node/fs.mjs

apps/site/.cloudflare/node/fs/promises.mjs

apps/site/.cloudflare/node/readline.ts

apps/site/.cloudflare/prepare-build.mjs

ovflowd · 2025-03-01T14:55:52Z

apps/site/app/[locale]/next-data/blog-data/[category]/[page]/route.ts

-        // if the page number is 0 or something smaller than 1
-        providePaginatedBlogPosts(params.category, requestedPage)
-      : provideBlogPosts(params.category);
+  const data = await (requestedPage >= 1


Can you separate the method call and await to somewhere else? Like add the await directly on return Response.json(await data, ...)

ovflowd · 2025-03-01T14:56:51Z

apps/site/app/[locale]/page.tsx

@@ -92,7 +92,7 @@ const getPage: FC<DynamicParams> = async props => {
  // Gets the current full pathname for a given path
  const pathname = dynamicRouter.getPathname(path);

-  const staticGeneratedLayout = DYNAMIC_ROUTES.get(pathname);
+  const staticGeneratedLayout = (await DYNAMIC_ROUTES()).get(pathname);


One line for await and another for the function call. Also, if DYNAMIC_ROUTES is no longer a constant but an async method, its name should be changed, and it should be moved to another file (in case of DYNAMIC_ROUTES declaration).

ovflowd · 2025-03-01T14:57:41Z

apps/site/next-data/downloadSnippets.ts

@@ -29,7 +29,7 @@ export default async function getDownloadSnippets(
    const { default: provideDownloadSnippets } = await import(
      '@/next-data/providers/downloadSnippets'
    );
-    return provideDownloadSnippets(lang)!;
+    return (await provideDownloadSnippets(lang))!;


I don't like ! assertions -- can we do something else? :)

but I didn't add the ! 🥹 , not to increase the PR's scope I'd be tempted to keep it as if here and address it in a separate PR?

Gotcha, can we at least separate the await with the return with (!) 🙏

ovflowd · 2025-03-01T14:57:56Z

apps/site/next-data/providers/blogData.ts

-    const categoryPosts = posts
+  async (category: BlogCategory): Promise<BlogPostsRSC> => {
+    blogData ??= await generateBlogData();
+    const categoryPosts = blogData.posts


Suggested change

const categoryPosts = blogData.posts

const categoryPosts = blogData.posts

ovflowd · 2025-03-01T14:58:12Z

apps/site/next-data/providers/blogData.ts


-const { categories, posts } = await generateBlogData();
+let blogData: {


Create a type for this, no inline types.

ovflowd · 2025-03-01T14:59:06Z

apps/site/next.dynamic.constants.mjs

+  const blogCategories = await provideBlogCategories();
+
+  const pages = [];
+  for (const c of blogCategories) {


Can we use Promise.all here instead of bare for? So you can use an Array.map and a Promise.all

ovflowd · 2025-03-01T14:59:14Z

apps/site/next.dynamic.constants.mjs

+
+  const pages = [];
+  for (const c of blogCategories) {
+    const categoryPages = (await provideBlogPosts(c)).pagination.pages;


Separate await from usage

ovflowd · 2025-03-01T14:59:29Z

apps/site/next.dynamic.mjs

@@ -93,7 +93,7 @@ const getDynamicRouter = async () => {

    return [...pathnameToFilename.keys()]
      .filter(shouldIgnoreStaticRoute)
-      .concat([...DYNAMIC_ROUTES.keys()]);
+      .concat([...(await DYNAMIC_ROUTES()).keys()]);


(Separate await from usage)

ovflowd

So far so good! If there's one keyword for this PR... It'd be await haha.

To be honest here, can we move these fs helpers from outside this package? I'd appreciate if you could create your own either personal or under the Cloudflare org package for these fs/promises, fs, readLine helpers :)

Just to no add this to our source-code. Or at the very least, another repository or package IDK!

apps/site/wrangler.jsonc

ovflowd · 2025-03-01T15:03:15Z

Thank you so much, @dario-piotrowicz for all the effort put in here so far!

github-actions · 2025-03-01T15:03:29Z

Lighthouse Results

URL	Performance	Accessibility	Best Practices	SEO	Report
/en	🟠 82	🟢 100	🟢 100	🟢 91	🔗
/en/about	🟢 100	🟢 100	🟢 100	🟢 91	🔗
/en/about/previous-releases	🟢 100	🟢 100	🟢 100	🟢 92	🔗
/en/download	🟢 90	🟢 100	🟢 100	🟢 91	🔗
/en/blog	🟢 100	🟢 100	🟢 96	🟢 92	🔗

github-actions · 2025-03-01T15:04:46Z

Unit Test Coverage Report

Lines	Statements	Branches	Functions
	88.75% (742/836)	76.1% (242/318)	87.65% (142/162)

Unit Test Report

Tests	Skipped	Failures	Errors	Time
182	0 💤	0 ❌	0 🔥	5.472s ⏱️

update the site application so that it can be build using the Cloudflare OpenNext adapter (`@opennextjs/cloudflare`) and thus deployed on Cloudflare Workers > [!Note] > This is very experimental and currently very slow > it's very much a work-in-progress right now

dario-piotrowicz · 2025-03-03T12:04:39Z

So far so good!

Thanks for having a look @ovflowd 🫶

If there's one keyword for this PR... It'd be await haha.

Yes, and actually I am waiting a bit before addressing your comments regarding the await-related changes because @vicb was working on an aspect of the Cloudflare adapter that might allow us to remove some of these code changes anyways 🤞

To be honest here, can we move these fs helpers from outside this package? I'd appreciate if you could create your own either personal or under the Cloudflare org package for these fs/promises, fs, readLine helpers :)
Just to no add this to our source-code. Or at the very least, another repository or package IDK!

That's totally fair, I'll see what I can do 🙂👍

@ovflowd besides the above also note that we still have that partial flash of unstyled contents issue which needs to be addressed 🙂

dario-piotrowicz · 2025-03-03T12:37:12Z

#7531 (comment) it does seem like the adapter is having issues after the rebase, I'll have a look 😓

ovflowd · 2025-03-03T20:47:34Z

#7531 (comment) it does seem like the adapter is having issues after the rebase, I'll have a look 😓

:F: -- sorry, @dario-piotrowicz 🤦

vercel bot deployed to Preview January 2, 2025 18:52 View deployment

dario-piotrowicz force-pushed the dario/opennextjs-cloudflare branch from b5e1217 to 0463451 Compare January 2, 2025 19:09

vercel bot deployed to Preview January 2, 2025 19:10 View deployment

dario-piotrowicz commented Jan 2, 2025

View reviewed changes

apps/site/wrangler.toml Outdated Show resolved Hide resolved

dario-piotrowicz commented Jan 2, 2025

View reviewed changes

apps/site/wrangler.toml Outdated Show resolved Hide resolved