Skip to content

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

Description

@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.

Activity

  1. Fishrock123 commented on Oct 19, 2017

    @Fishrock123
    Contributor

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

  2. refack commented on Oct 19, 2017

    @refack

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

  3. refack commented on Oct 19, 2017

    @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

  4. ljharb commented on Oct 19, 2017

    @ljharb
    SponsorMemberAuthor

    @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.

  5. mcollina commented on Oct 19, 2017

    @mcollina
    SponsorMember

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

  6. ljharb commented on Oct 19, 2017

    @ljharb
    SponsorMemberAuthor

    @mcollina can you elaborate on why?

  7. mcollina commented on Oct 19, 2017

    @mcollina
    SponsorMember

    @ljharb It has been extensively discussed before (since nodejs/http2#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.

  8. ljharb commented on Oct 19, 2017

    @ljharb
    SponsorMemberAuthor

    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.

  9. mcollina commented on Oct 19, 2017

    @mcollina
    SponsorMember

    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.

  10. bmeck commented on Oct 19, 2017

    @bmeck
    Member

    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.

  11. mhdawson commented on Oct 19, 2017

    @mhdawson
    Member

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

  12. MylesBorins commented on Oct 19, 2017

    @MylesBorins
    Contributor

    @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

  13. BridgeAR commented on Oct 19, 2017

    @BridgeAR
    Member

    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.

  14. mcollina commented on Oct 19, 2017

    @mcollina
    SponsorMember

    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).

  15. ofrobots commented on Oct 19, 2017

    @ofrobots
    Contributor

    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.

  16. 246 remaining items

  17. cjihrig commented on Jul 19, 2018

    @cjihrig
    Contributor
  18. ljharb commented on Jul 19, 2018

    @ljharb
    SponsorMemberAuthor

    The optimism in that tweet overlooks the strong blocking objections to ever standardizing something similar from multiple committee members.

  19. MylesBorins commented on Jul 19, 2018

    @MylesBorins
    Contributor
  20. Trott commented on Jul 31, 2018

    @Trott
    Member

    Re-added the tsc-agenda label on behalf of @ljharb who requests:

    i'd like the TSC to decide that new core modules will indeed go under a scope, so that we can begin the process of debating a specific implementation in a PR (that i'm happy to file/modify). I don't think the current PR is sufficient to create movement.

  21. mhdawson commented on Aug 1, 2018

    @mhdawson
    Member

    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.

  22. ljharb commented on Aug 1, 2018

    @ljharb
    SponsorMemberAuthor

    @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.

  23. benjamingr commented on Aug 2, 2018

    @benjamingr
    Member

    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 @nodejs/worker.

  24. ljharb commented on Aug 2, 2018

    @ljharb
    SponsorMemberAuthor

    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).

  25. mhdawson commented on Aug 2, 2018

    @mhdawson
    Member

    @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

  26. Trott commented on Aug 21, 2018

    @Trott
    Member

    @ljharb Since this was a proposal and it has received provisional approval, should the issue be closed and discussion moved to implementation PRs?

  27. ljharb commented on Aug 21, 2018

    @ljharb
    SponsorMemberAuthor

    @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.

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

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions