async_hooks.AsyncHook hooks has extra methods

[anonrig]

Environment

N/A

Reproduction

unenv/src/runtime/node/async_hooks/internal/async-hook.ts

Line 25 in ada7c40

init(asyncId: number, type: string, triggerAsyncId: number, resource: any) {

AsyncHook class should not have functions that should be defined in async hook callbacks. Specifically any methods except enable and disable should be removed.

Describe the bug

async_hook polyfill is wrong.

Additional context

No response

Logs

[pi0]

Can you please provide more context why it is a bug? (is there any library that depends on this behavior)

From what I read, the class method calls this._callbacks.init.

But also can confirm async_hooks.createHook({}) seems keeping them as internal (getters?)

> async_hooks.createHook({})
AsyncHook {
  [Symbol(init)]: undefined,
  [Symbol(before)]: undefined,
  [Symbol(after)]: undefined,
  [Symbol(destroy)]: undefined,
  [Symbol(promiseResolve)]: undefined
}

[anonrig]

But also can confirm async_hooks.createHook({}) seems keeping them as internal (getters?)

These are the symbols (not methods) whereas in unenv it is implemented as a method. Node.js used symbols to have "private properties" before #privateAttribute existed.

[anonrig]

Here is the node.js implementation: https://github.com/nodejs/node/blob/55cc372b963af76dcbd37994f28f1a663c9298ee/lib/async_hooks.js#L75

[pi0]

Yeah got it. They are extra. But what bug it causes currently?

[anonrig]

Yeah got it. They are extra. But what bug it causes currently?

Removing them will make a breaking change, which means that while removing this polyfill from Cloudflare Workers, we need to implement these methods even though they are not node.js compatible.

[pi0]

Ah i see it is precaution. If had time do you mind to drop a PR then? 🙏🏼