new core modules go under a namespace

[ljharb]

Per nodejs/TSC#389 (comment)

Do not merge this - this is solely a demonstration of how a namespaced module could exist.

Existing core modules would be aliased, either like this, using symlinks, or in the CJS/ESM loaders.

New modules would live inside lib/@nodejs, just like current ones live inside lib/.

Checklist

• make -j4 test (UNIX), or vcbuild test (Windows) passes
• tests and/or benchmarks are included
• documentation is changed or added
• commit message follows commit guidelines

[Trott]

I pushed a commit to fix some lint issues, but this still won't compile for me, I believe because @ ends up in variable names inside the generated node_javascript.cc file. Looks like @devsnek has fixed this. Go team!

[jasnell]

@nodejs/npm @nodejs/security @nodejs/modules ... we really need to make sure we are coordinating on this to make sure we do not accidentally introduce any security issues with older versions of Node.js that do not understand the @nodejs/ prefix.

[MylesBorins]

@jasnell I believe we can (and should) backport the resolver to all current LTS versions of the platform. If we consider it a security patch we can do so as a patch

[ljharb]

@jasnell as long as node owned the nodejs account, I’m not aware of any possible security issues, backported or not (I’m always in favor of maximal backporting, ofc)

[targos]

Existing core modules would be aliased, either like this, using symlinks, or in the CJS/ESM loaders.

While I'm +1 on the idea of putting new modules under a namespace, I don't think the PR enabling this should alias existing modules.
There are several people including me who think we could take advantage of the namespace to do some cleanup:

• Make Promise APIs first class: require('@nodejs/fs') === require('fs').promises
• Do not expose underscored modules: require('@nodejs/_http_agent') or require('@nodejs/sys') errors
• Do not expose undocumented legacy exports: require('@nodejs/util')._errnoException === undefined

[ljharb]

I don’t think it’s a good idea to be opportunistic here and try to pile on “improvements” on top of namespacing.

[targos]

I know not everyone will agree. That's why I think it should be a separate discussion that shouldn't block new modules to be under a namespace.

[mcollina]

I do not think this is the correct approach, and a full implementation will require doing this via the resolver.

Every file that we add increase the startup time, and we are currently trying to reduce it.

[jasnell]

I agree with @mcollina ... This is something that can be done within the resolver. Adding this kind of indirection is not the right approach.

[ljharb]

That’s totally fine; the implementation of that is equally straightforward. @mcollina do i need to put up a new PR that contains a string to string mapping, or would we be able to defer debating the implementation details until after the underlying direction has been decided?

[mcollina]

We can debate on the implementation details after the @nodejs/tsc agrees with the approach.

[ljharb]

Update: TSC has provisionally agreed on the approach; I'll update this PR with an actually workable implementation (this was originally just a prototype)

[jasnell]

What's the progress on this?

[ljharb]

@jasnell i have to update the PR based on #21551 (comment); conferences, travel, and life have interfered for the last month or two. I'm still planning to get on it soon :-)

[Trott]

@ljharb Are you able to rebase this and we can pick it up again?

[ljharb]

@Trott sure, i'll try to do that now. (edit: i talked to myles; he'll replace my commits with his own)

[MylesBorins]

I had started working on something after seeing @Trott's ping and with @ljharb's blessing I've force pushed an alternative implementation of this that works via the resolver. I'm pretty sure that this can be improved by moving some of the logic to C++, but wanted to get some feedback before pushing much further.

The majority of the implementation is in 4fdc880

48779a5 updates tests and benchmarks... so I've made it a separate commit to make it easier to review without the noise.

My initial implementation was with node:. Some rational for that approach

The @nodejs/ namespace is already reserved on npm and there are plans to
use this namespace for modules we publish to npm. Making a distinction
between modules prepackaged in node and those being distributed externally
seems like a prudent choice.

As the PR here had originally been made with @nodejs/ I patched that behavior in with 48779a5

One thing to note, the implementation currently throws if you attempt to load anything from the @nodejs/ namespace that doesn't already exist there. Without removing this we would be unable to polyfill modules, this may be a feature.

TL;DR of what I think we need to reach consensus on (which may require separate issues)

• what is the mechanism for namespacing
  • nodejs: vs @nodejs/ vs. ???
• Is the logic in the right place?
• Does the negatively affect any current workflows (thinking specifically apm's)

/cc @nodejs/tsc @nodejs/modules

[Fishrock123]

I would like to avoid, if at all possible, a situation where we migrate twice:

• (un-namespaced existing modules)
• some transitional namespacing (e.g. @nodejs/)
• an official langauge-supported namespace (e.g. nodejs:)

This leaves us in a situation where:

1. Which modules go under what namespace? Do new ones go both under @nodejs/ and nodejs:?
2. Which namespace do you use to require what (as a user)? Seems confusing.

(Which, to me, seems like unnecessary technical and UX debt.)

[sam-github]

Because we cross-posted, just want to say I share @Fishrock123's concerns, he decribes the worst-case that we don't want, but it sounds like the URI-like prefixes (like nodejs:) is going to be how these things are done, but in the browser and in servers.

[guybedford]

Shipping nodejs: for all current core modules at this point seems completely sensible to me. Will someone update this PR or create a new one for that?

[SMotaal]

I'd like to propose a different look at this: Schemes versus Namespaces

That said, nodejs: (or [whatever]:) working as described above being the spec'd resolution behaviour for that scheme, and that possibly extending to cover resolutions of nodejs:[whitelisted-vendor-specifiers-only] against node_modules/@nodejs/… somehow (ie #28062) with a lot more details to be figured out obviously.

[sam-github]

This was discussed at last TSC meeting, does it still need to be on the upcoming meeting's agenda?

[MylesBorins]

removed from the agenda. Conversations need to be had in this or another thread.

[devsnek]

Was there some sort of outcome from the tsc meeting?

[MylesBorins]

@devsnek the outcome was the move forward with namespaces... in a new PR.

Folks did not have strong opinions but in general were erring on the side of nodejs: rather than @nodejs/. I've followed up with @ljharb to see if he is ok with us closing this and opening a new PR.

[jkrems]

Do we have a feeling for how close we are on a call here? We're starting to use node: internally for multiple purposes (ESM resolver, open PR for policies #28767). Some people have mentioned nodejs: as the official scheme. Unfortunately @nodejs/ isn't really an alternative since it doesn't create valid URLs. So at least internally (and it's likely to leak) we'd have to still use some scheme in addition.

Unless we make another call, I assume node: is the thing that exists and would stick around..?

[devsnek]

node: is an implementation detail of the esm loader, for the purposes of public schemes i think we should discount its existence.

[jkrems]

node: is an implementation detail of the esm loader, for the purposes of public schemes i think we should discount its existence.

It's an implementation detail until it stops being one. The longer we delay moving that implementation detail to the official solution, the higher the risk that we unflag with it, it does leak in some use cases, and we're stuck with it unless we break the ecosystem.

[bmeck]

@devsnek

node: is an implementation detail of the esm loader, for the purposes of public schemes i think we should discount its existence.

I needed some URL mapping for policies, which are not integrated with ESM so it also exists in policies under the PR @jkrems linked. So it exists in at least 2 places currently. For policies it is meant to be publicly used.

[SMotaal]

I recall first noticing it in electron and nwjs — in dev tools — not that I am saying it should not, just how it ends up there can be relevant to keep in mind.

[Trott]

What should we do with this? Close it? Push it forward? Put it on the TSC agenda to see if it can get traction with someone else?

[MylesBorins]

I'm still planning on picking this up. The built in modules proposal is still being worked on at tc39 and there is hope for stage advancement at next week's meeting. If it lands at TC39 I will likely explore and experimental implementation of it for core

…

On Thu, Nov 28, 2019, 12:05 AM Rich Trott ***@***.***> wrote: What should we do with this? Close it? Push it forward? Put it on the TSC agenda to see if it can get traction with someone else? — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#21551?email_source=notifications&email_token=AADZYVZMOMJWTB5ZB6WSMRLQV5GTHA5CNFSM4FHDCYU2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEFLON6A#issuecomment-559343352>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AADZYV5WTQTK2TWBKSY23QDQV5GTHANCNFSM4FHDCYUQ> .

[ljharb]

It’s not on the agenda for next week’s meeting and the stage advancement deadline has passed.

[MylesBorins]

I have mispoken then, didn't check agenda, was under impression it was going to be on agenda Apologies

…

On Thu, Nov 28, 2019, 11:50 AM Jordan Harband ***@***.***> wrote: It’s not on the agenda for next week’s meeting and the stage advancement deadline has passed. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#21551?email_source=notifications&email_token=AADZYV7DT5XJ3GKQ6UK6D3DQV7ZGLA5CNFSM4FHDCYU2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEFNDB5Y#issuecomment-559558903>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AADZYV2ZCLUX3T2W5JK7LG3QV7ZGLANCNFSM4FHDCYUQ> .

[devsnek]

this pr isn't about js std anyway right?

[MylesBorins]

This PR isn't, but we have discussed which semantics to use for the namespace and similar behavior to what TC39 uses for built in modules would be desirable IMHO

…

On Thu, Nov 28, 2019, 12:51 PM Gus Caplan ***@***.***> wrote: this pr isn't about js std anyway right? — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#21551?email_source=notifications&email_token=AADZYV6I2KPNF3GSQMKXL7TQWAAJDA5CNFSM4FHDCYU2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEFNGOPI#issuecomment-559572797>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AADZYV43LBPKTPLRN4SGDTTQWAAJDANCNFSM4FHDCYUQ> .

[jasnell]

This has not been updated in quite a while, has not seen further progress, is out of date. Closing but can reopen if it is picked back up again and updated