-
Notifications
You must be signed in to change notification settings - Fork 134
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Proposal: add all new core modules under a scope? (too late for http2) #389
Comments
Fwiw I have admin access to https://www.npmjs.com/org/nodejs via Chris Dickinson. |
IMHO it should be considered to alias all modules with the prefixed named as well, so that new code could be uniform. |
RE the specific issue of the http2 conflict there's already an opt out by using https://twitter.com/feross/status/920835098794082305 |
@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. |
I'm -1 on this for http2. But it's worth discussing as a long-term plan. |
@mcollina can you elaborate on why? |
@ljharb It has been extensively discussed before (since nodejs/http2#29), and the module has been widely publicized. To note, the module maintainer is onboard with removing the flag. |
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 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. |
If this is about http2, then open an issue on nodejs/node and let's discuss it there on why we should revert 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. |
I think we should also talk about reserving the prefixes of modules:
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. |
On the surface it sounds like a good idea to me. Ignoring specific instances (ex http2) are there any negatives to the approach ? |
@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 |
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. |
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). |
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. |
Is there a similar precedent in a different runtime? |
+1 from me. I like the idea |
@refack In other languages? Pretty much every single one. Java packages under |
Thanks! |
@refack to be fair, node could choose to let |
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 :-) |
I kinda like strict separation (I'm thinking static analysis). Escape hatches could exist in a loader hook. |
Aren't loader hooks for ESM, not CJS? |
@ljharb anything going through |
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. That said, given that the author of the existing There are a couple of things necessary here to consider:
|
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. |
@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 @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. |
In regards to |
I like the idea but I'd prefer to use a scheme like |
A downside of that is that things added in newer nodes couldn't be polyfilled in older ones. Won't |
I'm not convinced, but I think this should not wait for me - if the rest of the TSC is on board with this, it could go to a vote. |
@nodejs/tsc can you chime in on whether you believe there is enough information/has been enough efforts on reaching a consensus that we should move to a vote? |
+1 to a namespace, but only if done as suggested in nodejs/node#21551 (comment). I've also heard rumors that TC39 may introduce namespaces at some point. If this is the case, we should wait for that, even if it is some time out. |
@cjihrig not sure where you heard any such rumors; nothing like that is anywhere close to being reality. |
It was mentioned that there was some discussion at the TC39 but the recommendation from that group was not to wait for it as it would be quite a while before there was anything concrete. |
@ljharb this is where I saw it: https://twitter.com/MylesBorins/status/1006938554025603074 |
The optimism in that tweet overlooks the strong blocking objections to ever standardizing something similar from multiple committee members. |
The explicit recommendation from TC39 was to not wait. This is part of the reason I am bullish on using the @ symbol for namespacing. If we rely on the mechanisms already supported by the ecosystem we lower the risk of further fragmentation
… On Jul 19, 2018, at 9:51 AM, Jordan Harband ***@***.***> wrote:
The optimism in that tweet overlooks the strong blocking objections to ever standardizing something similar from multiple committee members.
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub <#389 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AAecVzOvjveRqJUnE2T7P9Pnq18MYsZuks5uILkZgaJpZM4P-n4p>.
|
Re-added the tsc-agenda label on behalf of @ljharb who requests:
|
In the last TSC meeting we discussed again, there were no strong objections. What would be good is a list of modules that would move into scopes, how we are handling old apis and rules around where new modules go. |
@mhdawson i think the list would either be “empty”, or “all existing core modules“ (and they wouldn’t move into scopes; they’d just also be available under the scope). New modules would go in the same place old modules go, but they’d only be requireable under the scope. |
I'm +1 on moving existing modules with new APIs as suggested in nodejs/node#21551 (comment) as well as moving modules like worker_threads to |
I think creating new APIs while moving existing modules is as poor idea, but that can be debated separately from moving all the current experimental core modules (and creating a place for new ones to go). |
@ljharb I think the key point was that we should document a suggestion on moving/not moving (possibly in stages or in different categories like existing, experimental, new etc.) so that we can have the debate |
@ljharb Since this was a proposal and it has received provisional approval, should the issue be closed and discussion moved to implementation PRs? |
@Trott sure! To reiterate: yes, all new core modules should go under a scope. I'll update nodejs/node#21551 (comment) so that it reflects an actual implementation, and we can discuss that there. |
(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:name.startsWith('@node/')
, making it infinitely future-compatible(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 aroundhttp2
if we do this now, prior to unflagginghttp2
.The text was updated successfully, but these errors were encountered: