feat(nextjs): Add cloudflare waitUntil detection
#18336
Conversation
JPeer264
left a comment
JPeer264
left a comment
There was a problem hiding this comment.
To verify. This would only work with OpenNext, right?
| * https://github.com/opennextjs/opennextjs-cloudflare/blob/b53a046bd5c30e94a42e36b67747cefbf7785f9a/packages/cloudflare/src/cli/templates/init.ts#L17 | ||
| */ | ||
| function _getCloudflareContext(): MinimalCloudflareContext | undefined { | ||
| const cfContextSymbol = Symbol.for('__cloudflare-context__'); |
There was a problem hiding this comment.
super-l: Since this is Opennext specific we could theoretically rename it to make it more obvious.
There was a problem hiding this comment.
I though the comments would suffice, but you are right, it should be clearer with the naming. I will do that 👍
| * https://github.com/opennextjs/opennextjs-cloudflare/blob/b53a046bd5c30e94a42e36b67747cefbf7785f9a/packages/cloudflare/src/cli/templates/init.ts#L17 | ||
| */ | ||
| function _getCloudflareContext(): MinimalCloudflareContext | undefined { | ||
| const cfContextSymbol = Symbol.for('__cloudflare-context__'); |
There was a problem hiding this comment.
l: Do you think it would be possible to add this Symbol somehow in our Cloudflare SDK so we don't have to rely 100% on opennext?
There was a problem hiding this comment.
I think our SDK can do something similar by putting the context on the global scope, so that we don't have to toss it around. But yes this is 100% opennext specific.
Will be very useful for SDKs that do use cloudflare SDK setup I suppose, but not necessarily the Next.js one at its current state with Opennext adapters.
This is part of the work done for supporting Next.js on cloudflare workers in #14931, one of the issues is our flusher didn't take cloudflare into account and the events were being cut off due to early shutdowns by the worker environment.
We cannot use our core's
flushIfServerlessbecause it requires explicitcloudflareWaitUntilto be passed down to it.Also The symbol we are grabbing here is specific to opennext and isn't something that is globally available on the worker environment. I had initially created #18330 because I mistook it for a worker environment API thing.
So this PR adds that detection to Next.js since it is relevant here and will use it if available, local tests show flushing is done correctly and events aren't being cut off.