🦋 Changeset detected

Latest commit: b04debb

The changes in this PR will be included in the next version bump.

Not sure what this means? Click here to learn what changesets are.

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

Great finding @dulapahv !
I think we need to understand why Next does patch setImmediate before merging this and if this PR will have side effects.

Note that cloudflare/workerd#5662 should help here too

Thanks for the feedback. From what I understand is that the setImmediate patch (in this PR) changes the usual behavior of the event loop by making setImmediate run immediately after the current task instead of waiting for the next event loop iteration.

Next.js's fast-set-immediate.external.ts runs this at module initialization:

function install() {
  if (process.env.NEXT_RUNTIME === 'edge') {
    // Nothing to patch. The exported functions all error if used in the edge runtime,
    // so we're not going to violate any assumptions by not patching.
    return
  } else {
    debug?.('installing fast setImmediate patch')

    const nodeTimers = require('node:timers') as typeof import('node:timers')

    // ⚠️ THIS LINE FAILS WHEN BUNDLED TO ESM
    globalThis.setImmediate = nodeTimers.setImmediate =
      // Workaround for missing __promisify__ which is not a real property
      patchedSetImmediate as unknown as typeof setImmediate
    globalThis.clearImmediate = nodeTimers.clearImmediate =
      patchedClearImmediate

    const nodeTimersPromises =
      require('node:timers/promises') as typeof import('node:timers/promises')
    nodeTimersPromises.setImmediate =
      patchedSetImmediatePromise as typeof import('node:timers/promises').setImmediate

    process.nextTick = patchedNextTick
  }
}

// Called at module load time
install()

The problem is this part:

globalThis.setImmediate = nodeTimers.setImmediate = patchedSetImmediate

This works in Node.js with CommonJS because require('node:timers') returns a mutable object. However, when esbuild bundles this to ESM, node:timers becomes an ES module namespace object with immutable exports. The assignment to nodeTimers.setImmediate throws:

TypeError: Cannot assign to read only property 'setImmediate' of object '[object Module]'

This happens at module initialization time when install() is called, so the Worker crashed before any request is handled.

I don't fully understand what Next.js relies on this patch for beyond the timing optimization described in this PR. I can't confidently say there are no side effects from shimming this out.

Maybe until this PR is merged you could say it in the documentation. I'm sorry if I'm bothering but I just spent half my day trying to debug this just to find this PR.

Maybe until this PR is merged you could say it in the documentation. I'm sorry if I'm bothering but I just spent half my day trying to debug this just to find this PR.

The problem is that a lot of users don't read the docs https://opennext.js.org/cloudflare#supported-nextjs-versions ;)

Maybe until this PR is merged you could say it in the documentation. I'm sorry if I'm bothering but I just spent half my day trying to debug this just to find this PR.

The problem is that a lot of users don't read the docs https://opennext.js.org/cloudflare#supported-nextjs-versions ;)

Ok my bad but maybe you can put a link to the issue page as I think it could be very relevant for this specific project because Next.js is very easy to pinpoint what the known problem are?

Given that the root cause needs to be addressed at the runtime layer and that #1057 implements a proper fix, this shim approach should probably not be merged. Happy to close this in favor of #1057.

Given that the root cause needs to be addressed at the runtime layer and that #1057 implements a proper fix, this shim approach should probably not be merged. Happy to close this in favor of #1057.

Yep I think this can be close.
There are still issues with the other PRs and Next 16.1 but I'll take a look tomorrow.
Thanks!