unenv: Should we avoid importing from node:... from internal modules?

[petebacondarwin]

Consider

unenv/src/runtime/node/timers/internal/scheduler.ts

Lines 1 to 3 in e8ea22f

	import type timers from "node:timers/promises";
	import { setTimeout, setImmediate } from "node:timers/promises";
	import type { TimerOptions } from "node:timers";

I feel like this code should actually be:

import { setTimeoutFallbackPromises } from "./set-timeout";
import { setImmediateFallbackPromises } from "./set-immediate";
export class Scheduler implements timers.Scheduler {
  wait(
    delay?: number | undefined,
    options?: Pick<TimerOptions, "signal"> | undefined,
  ) {
    return setTimeoutFallbackPromises(delay, void 0, options);
  }
  yield() {
    return setImmediateFallbackPromises();
  }
}

[petebacondarwin]

Once we are inside an internal file, I think we should stay there and not pop back out to the polyfilling system.

This is a blocker I hit when trying to implement node.js compat in the Cloudflare Vite Dev server plugin.

[pi0]

(moved to unenv discussions)

The main reason for this switch was the starting of hybrid polyfill support. node: imports point to the desired polyfill which can be either hybrid or not based on preset configuration.

This is a blocker I hit when trying to implement node.js compat in the Cloudflare Vite Dev server plugin.

I guess if vite is supporting aliases, it should properly resolve no? We do have it both for Nuxt vite builder behind a flag and Nitro rollup builder.

[petebacondarwin]

The problem is that I am using a crazy mixed approach of pre-bundling modules into workerd so that they get used ahead of built-ins (e.g. a module in the workerd bundle called node:path will be used instead of the built-in one when imported). But then these unenv entry-points have additional internal imports, which I am handling via the workerd module-fallback mechanism.

For some reason this particular import (node:timers/promises) cannot be found from the internally imported file... The simplest approach was to patch unenv, but today I will dig into it a bit more.

---

But to dig into the philosophical position a bit more... the unenv/src/runtime/node/timers/internal/scheduler.ts file is only imported from within the node:timers/promises package, so one has already committed to being inside the polyfill; importing from node:times/promises here is basically a circular dependency, which feels off.

[pi0]

I understand we can update this particular case (which self-references).

Generally aliases works. our workerd tests are also implemented with workerd fallabck server and esbuiod preset aliases.