Node: Backporting `util/types`, `path/posix` and `path/win32` to v15 proposal

Created on 24 Oct 2020  Â·  11Comments  Â·  Source: nodejs/node

Is your feature request related to a problem? Please describe.

I’d like to get https://github.com/nodejs/node/pull/34055 and https://github.com/nodejs/node/pull/34962 backported to v15.x.


Note that no backport PRs exist yet, and the above PRs were only merged into v16.0.

Describe the solution you'd like

I propose the following options:

  1. Backport https://github.com/nodejs/node/pull/34055 and https://github.com/nodejs/node/pull/34962 as‑is to v15, which, as an odd numbered release, is considered an unstable non‑LTS release, and thus some minor breakage might be somewhat acceptable for it.

  2. Backport https://github.com/nodejs/node/pull/34055 and https://github.com/nodejs/node/pull/34962 to v15, but restrict them to the node: scheme until v16, with the following sub‑options:

    1. Add support for require("node:<built‑in‑id>") and thus require("node:util/types"), require("node:path/posix") and require("node:path/win32"). (see also: https://github.com/nodejs/node/issues/36098)

    2. Don’t add support for require("node:<built‑in‑id>"), thus only supporting import("node:util/types"), import("node:path/posix") and import("node:path/win32"), but not require("node:util/types"), require("node:path/posix") and require("node:path/win32")

Most helpful comment

util/types luckily doesn't conflict with https://unpkg.com/browse/[email protected]/, the browserify shim for util. path/posix and path/win32luckily do not conflict with https://unpkg.com/browse/[email protected]/ either. In general though, there's no such thing as a "reserved name" when it comes to appending file paths on the end.

There's still the chance someone in an enterprise app has named a private package, or manually added a dir, that conflicts with these module names - obviously it's exceedingly unlikely.

If it's decided to backport them, I can ensure that resolve resolves them properly prior to the release PR landing, so CIGTM can pass.

All 11 comments

v15, which, as an odd numbered release, is considered an unstable non‑LTS release

I don't think that's true, v15 should not be considered unstable – the odd numbered release rule was in place in the early days of Node.js 0.x IIRC, nowdays Node.js follows semver for the most part. Stability should not be the focus anyway, breaking changes have been landed on even numbered releases too (E.G.: #27417 landed in v12.2.0).

Correct me if I'm wrong, the PR at stake here are not introducing breaking changes, but they do add a new core module which is the reason they got labeled semver-major (based on discussion around https://github.com/nodejs/node/pull/34055#issuecomment-711433832).

Here's what I could find in the docs regarding the add of a new modules in core:

https://github.com/nodejs/node/blob/70834250e83fa89e92314be37a9592978ee8c6bd/doc/guides/collaborator-guide.md#L328-L346

Nothing here stipulates the PR MUST be considered as semver-major, I'd say it SHOULD land as semver-minor in v15 because:

  • the new core modules are not "top-level", they uses already-reserved names (path and util).
  • both PRs checks all the points in the above policy (well, maybe it would require a backport PR adding the experimental status first).

util/types luckily doesn't conflict with https://unpkg.com/browse/[email protected]/, the browserify shim for util. path/posix and path/win32luckily do not conflict with https://unpkg.com/browse/[email protected]/ either. In general though, there's no such thing as a "reserved name" when it comes to appending file paths on the end.

There's still the chance someone in an enterprise app has named a private package, or manually added a dir, that conflicts with these module names - obviously it's exceedingly unlikely.

If it's decided to backport them, I can ensure that resolve resolves them properly prior to the release PR landing, so CIGTM can pass.

@nodejs/tsc @nodejs/releasers @nodejs/modules-active-members

Just in case I'm not at the TSC meeting: I'm in favor of backporting these changes to the 15.x line.

Is there anyone against that?

+1 from me.

@mhdawson
This shouldn’t be closed yet, as https://github.com/nodejs/node/pull/34055 and https://github.com/nodejs/node/pull/34962 haven’t yet been backported to v15.x.

I removed the semver-major label from those PRs. They'll be picked up automatically when the next release is prepared.

@targos is this a general policy change about new core modules being semver-major? Or were those two PRs included in the one-off decision about the diagnostics module?

@ExE-Boss k, thanks I will remove from the TSC agenda instead

This has now shipped in v15.3.0 (https://github.com/nodejs/node/pull/34055).

Follow‑up work to add support for node:‑prefixed imports to require(…) is being tracked in https://github.com/nodejs/node/issues/36098.

Was this page helpful?
0 / 5 - 0 ratings

Related issues

mcollina picture mcollina  Â·  3Comments

vsemozhetbyt picture vsemozhetbyt  Â·  3Comments

ksushilmaurya picture ksushilmaurya  Â·  3Comments

Brekmister picture Brekmister  Â·  3Comments

jmichae3 picture jmichae3  Â·  3Comments