vfs: allow imports without the node: prefix

[mcollina]

The experimental VFS module previously required the node: prefix. Allow both vfs and node:vfs in CommonJS and ESM while keeping --experimental-vfs gating unchanged.

The vfs module this shadow currently managed by @ljharb, who told me it's ok.

[nodejs-github-bot]

Review requested:

• @nodejs/loaders
• @nodejs/startup

[Renegade334]

Was there a consensus from the node:test TSC discussion against exposing any new barenamed core modules from now on, or am I making that up?

[panva]

The vfs module this shadow currently managed by @ljharb, who told me it's ok.

https://www.npmjs.com/package/vfs

Something doesn't add up here.

Details

[JakobJingleheimer]

I think we should not do this

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 90.37%. Comparing base (ebef774) to head (8c31f00).
⚠️ Report is 281 commits behind head on main.

Additional details and impacted files

@@            Coverage Diff             @@
##             main   #66414      +/-   ##
==========================================
+ Coverage   90.28%   90.37%   +0.09%     
==========================================
  Files         790      792       +2     
  Lines      271642   275682    +4040     
  Branches    51846    52854    +1008     
==========================================
+ Hits       245260   249156    +3896     
- Misses      16889    16939      +50     
- Partials     9493     9587      +94     

Files with missing lines	Coverage Δ
lib/internal/bootstrap/realm.js	96.98% <ø> (-0.01%)	⬇️

... and 194 files with indirect coverage changes

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

[ljharb]

I think we definitely should - requiring the node: prefix is not ideal. The vfs package should (and will, as soon as this PR is merged) publish a latest version that fails to install and has no runtime behavior, to ensure nobody installs it by default.

[panva]

I think we definitely should - requiring the node: prefix is not ideal. The vfs package should (and will, as soon as this PR is merged) publish a latest version that fails to install and has no runtime behavior, to ensure nobody installs it by default.

Why do those need to be tied together? vfs on npm being always error is a good measure if you managed to claim that name. At the same time node:vfs can stay node: only.

[marco-ippolito]

I think we should stay consistent with the policy we have put in place.

node/doc/contributing/collaborator-guide.md

Lines 516 to 533 in cede7e6

	New modules must only be added with the `node:` prefix, as `semver-minor`.
	When adding a "sub-module", e.g. a promise variant of an existing API (e.g.
	`node:inspector/promises`) that is available without the `node:` prefix, making
	the sub-module available without the prefix is possible behind a runtime flag,
	or as a `semver-major` change.
	If the new module name is free in npm, register
	a placeholder in the module registry as soon as possible. Link to the pull
	request that introduces the new core module in the placeholder's `README`.
	If the module name is not free and the module is
	not widely used, contact the owner to see if they would be willing to transfer
	it to the project.
	We register a placeholder without the `node:` prefix whenever
	possible to avoid confusion and typosquatting attacks.

[ShogunPanda]

I agree with Marco.

[JakobJingleheimer]

As Marco said: this violates our established policy.

[marco-ippolito]

... or change the policy first 😄

[JakobJingleheimer]

... or change the policy first 😄

I agree with the policy.

[ljharb]

It's unfortunate I missed #64648, as I would absolutely have blocked it. Clearly it wasn't widely circulated enough, as any policy change should be.

I would even argue that perhaps the policy change is invalid without notifying all collaborators and giving them a chance to weigh in.

[Renegade334]

I think we definitely should - requiring the node: prefix is not ideal.

What would you envisage the benefit to be?

[jsumners-nr]

As I have stated elsewhere, probably under my personal persona, it doesn't make sense to me that a new module is a breaking change, regardless of prefix. I think adding node:vfs and/or vfs should be a minor change.

As for the prefix, 🤷‍♂️. Either the project wants to move to namespaced modules or it doesn't. Having to keep track of which modules require a prefix and which ones have optional prefixes is silly. While I prefer a simple require('vfs'), I, along with many other people, have migrated to using the node: prefix. Add a lint rule if you need it.

[ljharb]

@Renegade334 for one, it is insanely valuable to be able to mock out requires/imports, and for anything with the node: prefix, that's impossible.

@jsumners-nr Linting rules can keep track of which is prefix-only and which isn't for you, so that's not an argument.

[mcollina]

Closing this for now. @ljharb if you want, feel free to propose a change in the policy.

[ljharb]

@mcollina fair, but i still claim the policy change is itself invalid since it wasn't properly surfaced to all collaborators.

[mcollina]

@mcollina fair, but i still claim the policy change is itself invalid since it wasn't properly surfaced to all collaborators.

I won't dispute that in this PR. If you want to, go ahead and propose to relax it!