add incompatible async_hooks fns for backward compat

[anonrig]

The following methods are defined in unenv even though they are not defined in Node.js. This is due to a bug. AsyncHook class doesn't have these functions, but async hook callbacks does. Please take a look into Node.js documentation for async_hooks

In order to preserve backward compatibility and not introduce a breaking change on applications that depend on this functions (even though they are not doing anything), we have to implement them in workerd as well.

The issue is tracked on unjs/unenv#403 but removing these functions will be a breaking change, and wrangler still needs to provide these methods regardless of it's been removed from unenv or not. (Unless we remove polyfill and depend on async_hooks implementation of workerd, rather than the polyfill)

In a follow up pull-request, I'll add logging for these functions to track the usage over time. It won't be 100% correct, but it will give us an estimation about whether or not this is used in workers.

cc @IgorMinar @irvinebroque @jasnell @danlapid @vicb

[anonrig]

@jasnell just fyi: we need to land this pull-request before the next release, if we don't want to introduce a breaking change.

[petebacondarwin]

When you say "cannot remove them" do you mean "cannot not add them"?
They are not currently in workerd but we are going to remove them from unenv.
Is that right?

[irvinebroque]

we discussed and not going to add unenv-only stuff to the runtime just for backwards compat. will find other way.

[petebacondarwin]

Can we leave them in unenv and then have a breaking change Wrangler major release in the future where we also drop them from unenv (and/or drop unenv altogether).

[irvinebroque]

bingo! ^^ this

[kentonv]

Please let's not push wrangler releases that break existing code for any reason. We have a compat flag system, we can use it.

[jasnell]

To summarize where we have landed in internal discussions around this:

1. We will implement Node.js compat APIs in the runtime independently of what the polyfills do. If a worker is deployed to the runtime with a polyfill implementation of an API that is implemented in the runtime, the polyfill implementation will take precedence and will be what that worker uses.
2. The existence of the implementation in the runtime should not break the polyfill. That is, if the polyfill is included then the runtime implementation is ignored unless the polyfill itself chooses to pay attention to it and build on it (i.e. using the process.getBuiltinModule(...) API).
3. When an implementation of a Node.js API is added to the runtime, we will notify the wrangler team (and other users not using wrangler) and can work with that team on devising a reasonable transition path that would allow users to move off of the polyfill without risking breaking changes. But the mere existence of the implementation in the runtime does not automatically mean the polyfill goes away or needs to change.
4. In other words, we will not have the assumption that just because the implementation exists in the runtime that polyfills won't be used or that they'll go away entirely. There may be a period of overlap where wrangler still pushes workers with the polyfill even if the API exists in the runtime.

As long as we follow this approach, it is ok if the runtime implementation of an API does not match the polyfill-injected implementation of the API and such differences would not be considered breaking changes, making this PR unnecessary. Instead, when implementing the APIs in directly in the runtime we will focus only on implementing the behavior that is correct to ensure compatibility with Node.js and not compatibility with unenv.

To @kentonv's point... wrangler will need to ensure that such updates are done in a way that does not become a burden on users to reconcile. Compat flags in wrangler could help minimize the risk of breaks when transitioning between a polyfill-injected API and a runtime-provided API. If done properly, that can be handled entirely in the tooling without the need to add a bunch of new compat flag combinations to the runtime itself.

[jasnell]

Based on the above discussion, this PR shouldn't land. Aligning the built-in impl with the polyfill impl is not goal.

[vicb]

Based on the above discussion, this PR shouldn't land. Aligning the built-in impl with the polyfill impl is not goal.

So revert #3432?

[vicb]

Can we leave them in unenv and then have a breaking change Wrangler major release in the future where we also drop them from unenv (and/or drop unenv altogether).

Some thoughts here:

The extra functions are a bug in unjs/unenv so I think it should be fixed there. Because we do not control unjs/unenv (and because it wouldn't really make sense anyway) the fix will probably be async wrt wrangler releases.

We could make sure @cloudflare/unenv-preset still exposes those functions whether or not unjs/unenv is fixed. It would be a way to control exactly when we want to drop them (i.e. a wrangler major release). However I don't think it makes sense to bake a bug into @cloudflare/unenv-preset.

I think the correct fix this would be to extend the nodeless preset of unjs/unenv to add those functions. The presets hierarchy would be nodeless < nodeless-wrangler < cloudflare. nodeless-wrangler would add those functions and we could drop it on the major release.