Skip to content

vfs: allow imports without the node: prefix - #66414

Open
mcollina wants to merge 1 commit into
nodejs:mainfrom
mcollina:vfs-schemeless
Open

mcollina wants to merge 1 commit into
nodejs:mainfrom
mcollina:vfs-schemeless

Conversation

@mcollina

Copy link
Copy Markdown
Member

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.

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.

Assisted-by: Pi
Signed-off-by: Matteo Collina <hello@matteocollina.com>
@nodejs-github-bot

Copy link
Copy Markdown
Collaborator

Review requested:

  • @nodejs/loaders
  • @nodejs/startup

@nodejs-github-bot nodejs-github-bot added lib / src Issues and PRs involving general changes in the lib/ or src/ directories. needs-ci PRs that need a full CI run. labels Sep 30, 2026
@Renegade334

Copy link
Copy Markdown
Member

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

panva commented Sep 30, 2026 •

Copy link
Copy Markdown
Member

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 image

@JakobJingleheimer

Copy link
Copy Markdown
Member

I think we should not do this

@codecov

codecov Bot commented Sep 30, 2026 •

Copy link
Copy Markdown

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 ljharb left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

panva commented Oct 1, 2026

Copy link
Copy Markdown
Member

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 marco-ippolito left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

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

Copy link
Copy Markdown
Contributor

I agree with Marco.

@JakobJingleheimer JakobJingleheimer left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As Marco said: this violates our established policy.

@marco-ippolito

Copy link
Copy Markdown
Member

... or change the policy first πŸ˜„

@JakobJingleheimer

Copy link
Copy Markdown
Member

... or change the policy first πŸ˜„

I agree with the policy.

@ljharb

ljharb commented Oct 1, 2026 •

Copy link
Copy Markdown
Member

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

Copy link
Copy Markdown
Member

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

What would you envisage the benefit to be?

@jsumners-nr

Copy link
Copy Markdown

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

lib / src Issues and PRs involving general changes in the lib/ or src/ directories. needs-ci PRs that need a full CI run.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

9 participants