Proposal: add all new core modules under a scope? (too late for http2)

[ljharb]

(per https://twitter.com/MylesBorins/status/920833637351862272)

Prior to unflagging http2 in node 8, it would be worth considering if instead of adding a new core module that might break userland code; adds new complexity in determining what is a core module - something that was already done in v0.11 and v1; and requires adding a "bailout" flag to node 8 LTS.

Alternatively, if all new core modules were added under a scope, say @node/, then we'd get the following benefits:

• no need for a new bailout flag for http2
• never again have any conflicts with userland for new core modules
• any "is core module" code would simply be able to add name.startsWith('@node/'), making it infinitely future-compatible
• move from nonzero risk of breakage, to zero risk

(Whether the scope is @node or @nodejs or whatever is irrelevant; we just have to find one that's available, or where the owner is willing to give it up)

While http2 landing in node 8 and/or 9 unflagged does not block this proposal for all future core modules, we have a brief, rare window here to avoid any breakage around http2 if we do this now, prior to unflagging http2.

[Fishrock123]

Fwiw I have admin access to https://www.npmjs.com/org/nodejs via Chris Dickinson.

[refack]

IMHO it should be considered to alias all modules with the prefixed named as well, so that new code could be uniform.

[refack]

RE the specific issue of the http2 conflict there's already an opt out by using require('http2/') to explicitly reference the npm package.

https://twitter.com/feross/status/920835098794082305 https://twitter.com/substack/status/920833919666106368

[ljharb]

@refack Definitely; we'd want to alias all core modules under the namespace as well, for consistency.

Those aliases could also be added in a node 6 minor, even.

Regarding the opt-out, that's great in that it provides a mechanism for both the userland and core module to coexist - but that's not an option for published modules that will no longer be updated that depends on the userland http2.

[mcollina]

I'm -1 on this for http2. But it's worth discussing as a long-term plan.

[ljharb]

@mcollina can you elaborate on why?

[mcollina]

@ljharb It has been extensively discussed before (since https://github.com/nodejs/http2/issues/29), and the module has been widely publicized. require('http2') is happening in Node 9, and it will be part of the next RC. The only question we discussed in the latest TSC meeting is how (if at all) http2 should ship in Node 8 LTS.

To note, the module maintainer is onboard with removing the flag.

[ljharb]

None of that changes that unflagging with "http2" can still cause actual breakage, and will complicate code in anything that needs to detect core modules - including resolve, which is a transitive dependency of a very large number of modules. The module maintainer being on board in no way eliminates the breakage it could cause.

That it's been discussed before doesn't mean it's the best solution; in that entire thread, this solution wasn't brought up at all.

[mcollina]

If this is about http2, then open an issue on nodejs/node and let's discuss it there on why we should revert require('http2') for the upcoming 9 release, and not ship it at all for the next 8 release.

This issue is about process than we should definitely discuss things here. We are currently derailing this conversation which is actually very interesting, but is is a major shift and we should not rush it. BTW we shipped https://nodejs.org/api/perf_hooks.html as a new module in July, and this was not brought up.

[bmeck]

I think we should also talk about reserving the prefixes of modules:

RE the specific issue of the http2 conflict there's already an opt out by using require('http2/') to explicitly reference the npm package.

Seems like we could/should reserve those prefixes. If only in ESM which has slightly different rules, that seems fine but I would like both CJS and ESM. Especially if we start having subpackages which has been brought up a couple times w/ versioning or different async implementations. Better to reserve before we need I think.

[mhdawson]

On the surface it sounds like a good idea to me. Ignoring specific instances (ex http2) are there any negatives to the approach ?

[MylesBorins]

@mhdawson the only negative I can think of would be the User Experience of slightly more verbose module names. That being said I feel that is appropriately offset by the various benefits

[BridgeAR]

I like to idea of using scopes in general. That way everyone could also directly see if it is a userland module or not without knowing all core modules. Someone requested something in that direction, but I can not find the issue to it anymore.

[mcollina]

I’m -1. It would have been fantastic if scopes were available in the beginning. However, we are left with the current state of things, were the module scope is global.

I don’t see any reason to do this unless we plan to add a lot of new modules. We don’t. The things that have been added are either part of web ecosystem (that could have been global) or are of critical importance to increase the adoption of Node (async_hooks, http2).

[ofrobots]

Using a scope for core modules would also help address install time security issues with same named packages on npm. For example: http gets installed 129k times a month. If the login credentials of this module author were to get compromised by a rouge party then a lot of users could be affected by malicious code executed at install time.

[refack]

Is there a similar precedent in a different runtime?

[evanlucas]

+1 from me. I like the idea

[bmeck]

@refack In other languages? Pretty much every single one. Java packages under java.*, Rust modules under std::, etc.

[refack]

@refack In other languages? Pretty much every single one. Java packages under java.*, Rust modules under std::, etc.

Thanks! Is the rust prefix exclusive? AFAIK java.* isn't nor is System.* for dotnet, i.e. userland can add classes in that namespace.

[ljharb]

@refack to be fair, node could choose to let @node/foo, when foo is not a core module, fall back to the filesystem - which would allow for userland "core" modules - but it's probably better not to leave that open.

[ljharb]

Although now that I think about it, that method would allow older node versions to polyfill core modules from newer node versions - so maybe that's a good idea :-)

[refack]

I kinda like strict separation (I'm thinking static analysis). Escape hatches could exist in a loader hook.

[ljharb]

Aren't loader hooks for ESM, not CJS?

[bmeck]

@ljharb anything going through import. Part of the reason I push hard to get people to only write import/import() going forward.

[jasnell]

I've got mixed feelings on this. I can definitely understand and agree with @mcollina's point of view and my knee jerk reaction is to -1 this... but, having core modules in their own namespace does have a benefit even if at the cost of backwards consistency and usability. It would also make sense in light of the "built-in modules" concept being looked at for ESM (e.g. import * from "node:fs").

That said, given that the author of the existing http2 module has eagerly agreed to hand it over this really should not be as big of an issue as it is being made out to be.

There are a couple of things necessary here to consider:

1. A change would need to be made to the existing loader to support the @nodejs/ scope for native modules. There's not yet a PR we can look at and I'm building the 9.0.0 binary on Sunday and we've already messaged that http2 will be included in both 9.0.0 and the 8.x LTS.

2. On the plus side, http2 support is still Experimental and everyone who would be using it should know that, especially since we still have a nice process warning emitted when it is first used.

3. Getting http2 into the hands of users is a much higher priority. This is obviously something users want very much.

4. The existing userland module is unsupported, most of it's dependent modules have less than 100 downloads in the past month (if I'm being generous), and we've messaged about this consistently for well over a year now. Even the author of the the existing http2 module is looking forward to this being in core.

5. Keep in mind that the requiring the scope for new modules would have the side effect of limiting the ability to backport those modules to LTS versions unless we also backported the loader changes necessary to support them. That's definitely not out of the question but it does raise the necessary considerations. Would we consider adding the requirement that the @nodejs/ scope is reserved for built-in modules a semver-major change? (keeping in mind the fact that npm currently allows users to associate any arbitrary scope with a private registry). It is unlikely that this would break anyone, but I would argue that it's as easily unlikely that just having require('http2') would really break anyone given the current realities of that module.

[ofrobots]

I would propose we split the discussion of scopes in general and http2. I am +1 on scopes, but I am -1 on holding up http2 module for this.

[ljharb]

@jasnell please note that the http2 maintainer's willingness isn't the issue, it's existing modules that depend on it that are the problem. Also note that the only part that backporting might break is reserving the scope - simply backporting a new core module with the name @nodejs/http2 would be semver-minor. It is a fair point that it'd be the same category of possible breakage as shipping "http2", but it'd only break if they used @nodejs/http2 specifically - it wouldn't break for any other names under that scope.

@ofrobots thanks, I appreciate splitting the issue - while I think it's a missed opportunity if http2 ships as-is, if it's the last unscoped core module that's added it's still better than nothing.

[apapirovski]

In regards to http2, as far as I can tell, that module literally does not work in the latest v8.x. There's a fork that fixes those issues and it's required via http2.js. I feel like that's a somewhat pertinent point in all these discussions around breaking existing user-code...

[targos]

I like the idea but I'd prefer to use a scheme like node:fs instead of a scope.

[ljharb]

A downside of that is that things added in newer nodes couldn't be polyfilled in older ones.

Won't require('node:something') look in node_modules as well?

[targos]

Good point. I didn't think about that.

Won't require('node:something') look in node_modules as well?

Yes I think so, but node:something is not a valid npm name.

[ljharb]

Sure, but npm names aren't the only conflict, it's primarily the filesystem - because that's where require looks :-)

[ljharb]

@bmeck @refack that means tho that "loader hooks" aren't a sufficient escape hatch imo; whatever the mechanism is would have to work for require and for import.

[rvagg]

+0 from me, I'm generally positive on the idea and think there's some interesting possibilities for taking it even further and breaking up core modules a bit more into components. But, I quite like the fact that it's difficult to add more cruft to core, having roadblocks to this is a good thing IMO, it shouldn't be a trivial thing to expand our "small core" so the more pressure to stay small the better!

[MylesBorins]

I'd like to propose that we hold off on adding any other new core modules until we have reached consensus on this

/cc @nodejs/tsc

edit: http2 obviously not included 😅

[jasnell]

we hold off on ....

That kind of blanket ban isn't necessary. We have a guidelines in place for this already and there are no proposals for new modules currently planned. On the off chance something does come up, we can hold off landing it until this is settled.

[jasnell]

Perhaps what would be most productive at this point is a PR that proposes the changes necessary to the loader to support the @nodejs/ prefix for use with internal modules. Without a PR that actually proposes such changes, this is largely moot.

[mgol]

@apapirovski

In regards to http2, as far as I can tell, that module literally does not work in the latest v8.x. There's a fork that fixes those issues and it's required via http2.js. I feel like that's a somewhat pertinent point in all these discussions around breaking existing user-code...

Well, it did break at least Karma tests, see https://travis-ci.org/karma-runner/karma/jobs/292519595#L2639

Granted, it's easy to fix it but it did, nevertheless, cause a breakage.

[jasnell]

This discussion appears to have stalled. Is there reason to keep this open?

[ljharb]

Yes; it’s still pending a PR. I’d say it needs to remain open until the next new core module pops up, or, until we decide to definitively or definitely not go with this proposal.

[devsnek]

i started messing around with this, there are a lot of new variables to account for (like esm) which make this quite interesting.

the code itself for this isn't very complex, but the behaviours are infinitely bikeshedable.

for the moment what i've been experimenting with is:

• Module._resolveFilename resolves unnamespaced to namespaced (fs -> @nodejs/fs instead of fs -> fs)
• esm uses @nodejs/ as a sentinel for a builtin, gating NativeModule.nonInternalExists

some thoughts about what i've observed so far:

• changing cjs is a tangled mess of evil (the behaviours, not the code) and it might be worth only using namespaces with esm
• putting @nodejs on an import/require feels really really cool

beyond @nodejs namespaces, i'm wondering about how we can push for stuff like compression/brotli and compression/zlib instead of brotli and zlib

i'd also like to get more ecosystem feedback about what namespacing modules would do. i'd like to open up these names to see what userland does with them but i don't know the impact it might have.

[ljharb]

I think the whole point is finding names that userland can never use, so that core is free to experiment, and so that userland tools can unambiguously determine - from the name alone - what’s a core module.

[devsnek]

another interesting idea is that since we have the nodejs namespace on npm we could publish packages that were on their own version track while still being supported/developed/tested as core modules

[ljharb]

Sure! I'd still want another namespace tho, that could never be on npm, that could only ever be core modules (and new core modules could only ever be under that namespace).

[ljharb]

OK so - apparently fs/promises is a new (unflagged) core module in node 10, despite the proposed policy to not add ANY new core modules until this issue is resolved.

Can we put fs/promises behind a flag, or remove it pending this issue?

[joyeecheung]

I think we should discuss about this in the TSC meeting. It's probably not too late while fs/promises is experimental (albeit without a flag).

[devsnek]

reading up a bit more it seems like you all want a pr, however I've found that the stress I get from opening this type of pr is not something I want to continue inviting. I'm happy to give my code to whoever wants it and engage in the bikeshedding though :)

[Trott]

@devsnek I'm happy to take the code and either try to shepherd it through the bike-shed or else hand it off to someone else who might be willing to shepherd the PR. (I would love to see @ljharb do it, but that may be asking a lot.)

[Trott]

Summary of today's discussion at the TSC:

• We added fs/promises which could result in a conflict with real world modules. This proposal was intended to prevent that. Do we need to move forward with this? Do we need to change fs/promises? It is experimental but also not behind a flag.

• We need someone to do the work to introduce the namespace into our module loader. @devsnek has started work on it. People seem in favor, it’s just a matter of getting an implementation.

• There was discussion and a lack of agreement about whether it would be best to change fs/promises to use a namespace or not. On the one hand, it's experimental. On the other hand, it's not behind a flag so people are going to start using it despite the warning it emits.

For more details (and to see who said what) check out the minutes (which are currently in a pull request and not yet in the TSC repo, here's the PR: https://github.com/nodejs/TSC/pull/537

Ping @nodejs/collaborators for input. (Yeah, I know we're trying to ping Collaborators as a whole less and less. I don't do it often. This seems important. It's something that may have a large impact. Sorry if it's not of any interest.)

[ljharb]

Reading the notes, I see "it would be best to apply an exception for this one" - which was the same excuse given for the http2 module.

I think it's very very important that fs/promises be removed, or at least put behind a flag, ASAP - ideally in a patch. "One-off" exceptions are going to keep sneaking in unless the policy is to ensure no new unflagged core modules, which includes reverting ones that make it in, which includes fs/promises. (i say this despite very much desiring fs/promises to exist)