cloudflare / next-on-pages

CLI to build and develop Next.js apps for Cloudflare Pages
https://www.npmjs.com/package/@cloudflare/next-on-pages
MIT License
1.24k stars 119 forks source link

13.4, Server Actions and Turbopack #218

Open GregBrimble opened 1 year ago

GregBrimble commented 1 year ago

Might want to break this down into multiple things, but this issue tracks the work to check our compatibility with these latest features:

james-elicx commented 1 year ago

From what I've been able to tell, the only blocker at the moment is node:buffer (#199), which is taken care of by https://github.com/cloudflare/workers-sdk/pull/3133. Using the Wrangler prerelease, I've had success with basic server actions and 13.4 - both seem to be functioning as expected 🙂

ellemedit commented 1 year ago

Anyone else who faced node:async_hook module resolution problem?

I'm using next.js 13 (app router & server action enabled) with @cloudflare/next-on-pages.

In the build result .vercel/output/_worker.js, I can see:

// ../../../../tmp/functions-g41wrtb3ztm.js
var AsyncLocalStoragePromise, __BUILD_OUTPUT__;
var init_functions_g41wrtb3ztm = __esm({
  "../../../../tmp/functions-g41wrtb3ztm.js"() {
    AsyncLocalStoragePromise = import("node:async_hooks").then(({ AsyncLocalStorage: AsyncLocalStorage2 }) => {
      globalThis.AsyncLocalStorage = AsyncLocalStorage2;
    }).catch(() => void 0);
    __BUILD_OUTPUT__ = { "/404.html": {
      type: "override",
      path: "/404.html",
      headers: { "content-type": "text/html; charset=utf-8" }
    }, "/500.html": {
      type: "override",
      path: "/500.html",
      headers: { "content-type": "text/html; charset=utf-8" }
    }, "/_next/static/chunks/800-9e0cf48029271fa9.js": { type: "static" }, "/_next/static/chunks/app/layout-876cdf19b325dd89.js": { type: "static" }, "/_next/static/chunks/app/page-ae43855f96c3b6c6.js": { type: "static" },
// ... skip
"/_next/static/yvP752PVRYqo9GMwF85QY/_buildManifest.js": { type: "static" }, "/_next/static/yvP752PVRYqo9GMwF85QY/_ssgManifest.js": { type: "static" }, "/favicon.ico.body": {
      type: "override",
      path: "/favicon.ico.body",
      headers: { "cache-control": "public, max-age=0, must-revalidate", "content-type": "image/x-icon", "x-next-cache-tags": "/favicon.ico/route", "vary": "RSC, Next-Router-State-Tree, Next-Router-Prefetch" }
    }, "/next.svg": { type: "static" }, "/vercel.svg": { type: "static" }, "/index": {
      type: "function",
      entrypoint: AsyncLocalStoragePromise.then(() => Promise.resolve().then(() => (init_index_func(), index_func_exports))),
      matchers: [{ "regexp": "^/$", "originalSource": "/" }]
    }, "/": {
      type: "function",
      entrypoint: AsyncLocalStoragePromise.then(() => Promise.resolve().then(() => (init_index_func(), index_func_exports))),
      matchers: [{ "regexp": "^/$", "originalSource": "/" }]
    }, "/404": {
      type: "override",
      path: "/404.html",
      headers: { "content-type": "text/html; charset=utf-8" }
    }, "/500": {
      type: "override",
      path: "/500.html",
      headers: { "content-type": "text/html; charset=utf-8" }
    } };
  }
});

In development, it just work but in cloudflare pages production it throws error like:

03:57:20.577 | ✨ Success! Uploaded 11 files (488 already uploaded) (1.17 sec)
03:57:20.577 |  
03:57:20.866 | ✨ Upload complete!
03:57:22.717 | Success: Assets published!
03:57:23.825 | Error: Failed to publish your Function. Got error: Uncaught Error: No such module "node:async_hooks".   imported from "bundledWorker-0.16362082139793266.mjs"

sorry if this is known or unrelated this issue.

more clues: env: NODE_VERSION=18 cloudflare page build command: npm i -g pnpm && pnpm dlx @cloudflare/next-on-pages@beta output dir: /.vercel/output/static root dir: /

wrangler.toml:

compatibility_flags = [ "nodejs_compat" ]
node_compat = true
ellemedit commented 1 year ago

It is not possible to polyfill all Node APIs or behaviors, but it is possible to polyfill some of them.

This is currently powered by @esbuild-plugins/node-globals-polyfill which in itself is powered by rollup-plugin-node-polyfills.

maybe, the plugin doesn't resolve async_hook as https://github.com/ionic-team/rollup-plugin-node-polyfills/ listed no async_hook. but cloudflare documentation isn't

I guess next.js server requires async hook. perhaps, i missed something but hard to know.

james-elicx commented 1 year ago

Anyone else who faced node:async_hook module resolution problem?

Hello @Beingbook, have you added the nodejs_compat compatibility flag in your Pages project settings, per the readme?

ellemedit commented 1 year ago

oops, I missed that. Thank you @james-elicx ! I could find in English dashboard.

added: it just work! succeeded to deploy in production and I can see server action work. thank you so much.

james-elicx commented 1 year ago

oops, I missed that. Thank you @james-elicx ! I could find in English dashboard.

added: it just work! succeeded to deploy in production and I can see server action work. thank you so much.

No worries, glad to hear all is working for you! :)

dario-piotrowicz commented 1 year ago

I'm ticking 13.4 and server actions, since they seem to be working well, as you can see from this app that @james-elicx made: https://github.com/dario-piotrowicz/next-apps-for-testing/tree/master/apps/simple-app-dir-13.4-server-actions

And which I've deployed here: https://87ac4020.next-on-pages-test-5h3.pages.dev/

GregBrimble commented 1 year ago

Ah, I missed that Turbopack is currently only available for the dev server. Looks like you can't currently output a build with Turbopack, so no way to even start thinking about this yet since we have no idea what the output would look like. We can put this on hold until Turbopack supports building :)

zizifn commented 1 year ago

Seems revalidatePath in server-actions https://nextjs.org/docs/app/building-your-application/data-fetching/server-actions#on-demand-revalidation will not work properly..

The warning message is below:

POST https://****/actions - Ok @ 5/14/2023, 7:49:14 AM
  (warn) Failed to revalidate tag /actions TypeError: Fetch API cannot load: undefined/v1/suspense-cache/revalidate?tags=/actions

Sample source for this case is, https://github.com/zizifn/myTrash/blob/main/src/app/actions/page.tsx#L8

james-elicx commented 1 year ago

Seems revalidatePath in server-actions https://nextjs.org/docs/app/building-your-application/data-fetching/server-actions#on-demand-revalidation will not work.

The error is below:

POST https://****/actions - Ok @ 5/14/2023, 7:49:14 AM
  (warn) Failed to revalidate tag /actions TypeError: Fetch API cannot load: undefined/v1/suspense-cache/revalidate?tags=/actions

Sample source for this case is, https://github.com/zizifn/myTrash/blob/main/src/app/actions/page.tsx#L8

Hello @zizifn, it looks like revalidatePath is intended to be used for revalidating prerendered / ISR pages (statically generated pages). This is something that is not supported with next-on-pages, as prerendered pages depend on nodejs functions to work correctly. 🙁

Further, it looks like that error comes from the fetch cache, and the default fetch cache normally depends on retrieving an URL that Vercel injects into requests for their own caching infrastructure. And the API endpoint that it is trying to hit is not included with Edge runtime builds, further pointing to it being part of Vercel's own infra.

zizifn commented 1 year ago

Seems revalidatePath in server-actions https://nextjs.org/docs/app/building-your-application/data-fetching/server-actions#on-demand-revalidation will not work. The error is below:

POST https://****/actions - Ok @ 5/14/2023, 7:49:14 AM
  (warn) Failed to revalidate tag /actions TypeError: Fetch API cannot load: undefined/v1/suspense-cache/revalidate?tags=/actions

Sample source for this case is, https://github.com/zizifn/myTrash/blob/main/src/app/actions/page.tsx#L8

Hello @zizifn, it looks like revalidatePath is intended to be used for revalidating prerendered / ISR pages (statically generated pages). This is something that is not supported with next-on-pages, as prerendered pages depend on nodejs functions to work correctly. 🙁

Further, it looks like that error comes from the fetch cache, and the default fetch cache normally depends on retrieving an URL that Vercel injects into requests for their own caching infrastructure. And the API endpoint that it is trying to hit is not included with Edge runtime builds, further pointing to it being part of Vercel's own infra.

oh... Thanks! Make sense...

tom-huntington commented 1 year ago

Further, it looks like that error comes from the fetch cache, and the default fetch cache normally depends on retrieving an URL that Vercel injects into requests for their own caching infrastructure. And the API endpoint that it is trying to hit is not included with Edge runtime builds, further pointing to it being part of Vercel's own infra.

I managed to get revalidatePath working with the following options (actually just silence the error. It seems to work fine with the error)

export const runtime = 'edge';
export const revalidate = false;
export const dynamic = 'force-dynamic';
export const fetchCache = 'force-no-store';

I can now get progressively enhanced forms working https://youtu.be/O94ESaJtHtM?t=200

Revalidating data through new APIs revalidatePath and revalidateTag mean that mutating, re-rendering the page, or redirecting can happen in one network roundtrip, ensuring the correct data is displayed on the client, even if the upstream provider is slow. https://nextjs.org/blog/next-13-4#server-actions-alpha

james-elicx commented 1 year ago

Further, it looks like that error comes from the fetch cache, and the default fetch cache normally depends on retrieving an URL that Vercel injects into requests for their own caching infrastructure. And the API endpoint that it is trying to hit is not included with Edge runtime builds, further pointing to it being part of Vercel's own infra.

I managed to get revalidatePath working with the following options (actually just silence the error. It seems to work fine with the error)

export const runtime = 'edge';
export const revalidate = false;
export const dynamic = 'force-dynamic';
export const fetchCache = 'force-no-store';

Hello,

Is this on an app that you've deployed to Cloudflare Pages / tested locally through Wrangler that's not receiving the fetch error? Or next dev? If it's next dev, please try using Wrangler locally or deploying to Pages.

I have tried adding the route segment config options that you suggest and it still throws the error from the fetch cache, as one would expect it to since we aren't injecting Vercel's cache infra URL.

Failed to revalidate tag /test TypeError: Fetch API cannot load: undefined/v1/suspense-cache/revalidate?tags=/test

I can now get progressively enhanced forms working https://youtu.be/O94ESaJtHtM?t=200

Revalidating data through new APIs revalidatePath and revalidateTag mean that mutating, re-rendering the page, or redirecting can happen in one network roundtrip, ensuring the correct data is displayed on the client, even if the upstream provider is slow. https://nextjs.org/blog/next-13-4#server-actions-alpha

It's worth noting that I was referring to revalidating caches as the documentation only talks about it being used to revalidate cached/prerendered data (i.e. on-demand ISR):

How it works regarding the excerpt that you quoted is pretty hard to understand as there is zero documentation about it, and it seems to be rather experimental, otherwise, it would have proper support that doesn't depend on an incompatible cache. I've been having a look at what appears to be the server action handler code this morning, but honestly, it's still really not at all that clear how exactly they establish the response based on the static generation cache's revalidation tags.

james-elicx commented 1 year ago

I used

npx wrangler pages dev .vercel/output/static --compatibility-flag=nodejs_compat

Here's the code https://github.com/tom-huntington/revalidatePath-on-pages

To be clear revalidatePath called from a server action will re-render the server server component in one network round trip, in-addition to the on demand ISR revalidation.

The route segment config options are intended to disable all ISR and next/cache behaviour (the error is not an ISR error but a next/cache one, I believe).

Hello, it does look like there is still an error occurring with your repo, albeit ever so slightly different because it's having trouble crafting an URL this time to use for revalidating cached pages:

Failed to revalidate tag / TypeError: Invalid URL string.

It's still ultimately the same error though. It's coming from this line, where it is trying to send a fetch request to revalidate cached pages (i.e. ISR). Revalidating paths/tags with next/cache is a form of on-demand ISR, just with a slightly different name.

It may be possible that server actions hook into the incremental cache in some form to retrieve revalidation tags, and they might try and use that in some way to re-render the current page from the response the server action returns, but that doesn't mean it isn't intended to revalidate cached data. That is ultimately what I am talking about when referring to those functions, because that is what they are designed to be used for, and documented to be used for.

To be honest, I think it's a significant design mistake on Next.js' part to have server actions hijack the incremental cache functionality to decide what response they should return.

tom-huntington commented 1 year ago

Hello, it does look like there is still an error occurring with your repo, albeit ever so slightly different because it's having trouble crafting an URL this time to use for revalidating cached pages:

Failed to revalidate tag / TypeError: Invalid URL string.

Whoops. When it worked, I forgot to check if it was erroring.

To be honest, I think it's a significant design mistake on Next.js' part to have server actions hijack the incremental cache functionality to decide what response they should return.

Yes, I agree.

What's going to happen about this with next-on-pages? Can we send the right response without without calling revalidateTag? Is throwing, catching and logging an error in revalidateTag each time fine? Is it a fatal flaw for next-on-pages server actions?

james-elicx commented 1 year ago

What's going to happen about this with next-on-pages? Can we send the right response without without calling revalidateTag? Is throwing, catching and logging an error in revalidateTag each time fine? Is it a fatal flaw for next-on-pages server actions?

I haven't really explored playing around with the revalidateTag function, but I supposed it's possible that we could inject our own URL and hijack the request instead of it erroring, but as far as its intended use for cache revalidation goes, there isn't much that can be done because ISR is not compatible with the edge runtime.

If you are having success with using revalidatePath to refresh the current page from a server action, by all means, continue to do so - you'll just have that pesky error that you'll have to ignore.

dario-piotrowicz commented 1 year ago

I've added the blocked by external label as we need to wait for Vercel to support turbopack builds

geekyayush commented 1 year ago

https://87ac4020.next-on-pages-test-5h3.pages.dev/

Site loading time is too much. Website is vert slow. I am facing the same issue. Ideally it takes less than 500ms to open nextjs sites hosted on cloudflare pages. But my app router setup is taking 3-4 seconds for the request (when checking in network tab). Don't know if the problem is in cloudflare pages or nextjs itself.

GregBrimble commented 1 year ago

Looks like Turbopack may now be available under an experimental option: npx next experimental-compile --experimental-turbo.

dario-piotrowicz commented 12 months ago

@GregBrimble it seems like the vercel cli is not yet compatible with that, if I try running npx vercel build and use your command as the build script I get: Screenshot 2023-09-20 at 13 12 19 (so the vercel cli tries to read a file that the build command no longer produces)

I'd assume this is something they'll need to fix before releasing the turbopack build as stable 🤔

smakosh commented 10 months ago

Server actions are crashing the app when deployed. I'm revalidating a tag within a server action. I'm using Next.js 14.0.1 and @cloudflare/next-on-pages@beta

james-elicx commented 10 months ago

Server actions are crashing the app when deployed. I'm revalidating a tag within a server action. I'm using Next.js 14.0.1 and @cloudflare/next-on-pages@beta

Please can you open a new issue with a reproduction @smakosh so that it can be tracked properly.

martior commented 10 months ago

Server actions are crashing the app when deployed. I'm revalidating a tag within a server action. I'm using Next.js 14.0.1 and @cloudflare/next-on-pages@beta

If it was the same error I got it was because of cross site protection and X-Forwarded-Host not being set.

The following worker will add it for you until it is fixed.

export default {
  async fetch(request, env, ctx) {
    let newRequest = new Request(request)
    newRequest.headers.set('X-Forwarded-Host', newRequest.headers.get('host'))
    let response = await fetch(newRequest)
    return response
  }
};
smakosh commented 10 months ago

My issue got fixed on last version

ArianSha commented 10 months ago

Hey @smakosh did you find out/have a clue about what the issue was?