Conversation
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>
|
Review requested:
|
|
Was there a consensus from the |
https://www.npmjs.com/package/vfs Something doesn't add up here. |
|
I think we should not do this |
Codecov Reportβ
All modified and coverable lines are covered by tests. 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
π New features to boost your workflow:
|
ljharb
left a comment
There was a problem hiding this comment.
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? |
marco-ippolito
left a comment
There was a problem hiding this comment.
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
|
I agree with Marco. |
JakobJingleheimer
left a comment
There was a problem hiding this comment.
As Marco said: this violates our established policy.
|
... or change the policy first π |
I agree with the policy. |
|
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. |
What would you envisage the benefit to be? |
|
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 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 |

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
vfsmodule this shadow currently managed by @ljharb, who told me it's ok.