doc: stabilize subpath patterns

[guybedford]

This removes the experimental status from the subpath patterns section.

Checklist

• make -j4 test (UNIX), or vcbuild test (Windows) passes
• documentation is changed or added
• commit message follows commit guidelines

[nodejs-github-bot]

Review requested:

• @nodejs/modules

[jkrems]

Can we link some packages using it in the wild in the PR description to highlight successful adoption?

[MylesBorins]

LGTM

[MylesBorins]

@jkrems I'm not 100%, but my gut is that we won't see broader adoption of this field until it ships in 12.x next week

[benjamingr]

🎉

[ljharb]

Have subpath patterns been backported to v12 yet?

[guybedford]

@ljharb the backport is on 12-staging but still due to be released in the next 12 minor. I don't believe backport availability is a concern for this PR landing though.

[MylesBorins]

Yeah, stability has nothing to do with availability on LTS versions, it is solely a signal about the potential of change on the version of node that it is considered stable on

[MylesBorins]

Landed in 0c3e100

[ljharb]

Doesn't stability depend on adequate usage tho? Have subpath patterns had that usage if they're not available on both node 12 and 14?

[guybedford]

In terms of the data, the following packages on npm are currently shipping subpath patterns:

 1) "sugarss"
 2) "postcss-parser-tests"
 3) "@rdfine/rdfs"
 4) "@rdfine/schema"
 5) "@rdfine/shacl"
 6) "vanilla-hamburger"
 7) "postcss-scss"
 8) "postcss-js"
 9) "vanilla-colorful"
10) "@rdfine/sioc"
11) "fxjs"
12) "@ljharb/has-package-exports-patterns"
13) "@rdfine/owl"
14) "eslint-formatter-vscode"
15) "postcss"
16) "@rdfine/hydra"
17) "nfl-nerd"
18) "@rdfine/rdf"
19) "yopl"
20) "@rdfine/foaf"
21) "assemblyscript"
22) "deep6"
23) "numbo"
24) "@rdfine/wgs"
25) "@rdfine/doap"
26) "@zazuko/rdf-vocabularies"
27) "@nsbarsukov/preza"

[MylesBorins]

@ljharb there is not an explicit definition of what is required to stabilize an API in core. While adoption is a metric, imho the more important bits are a combination of how stable the code is (from a bug / perf perspective) and how likely we are to make a change to the API.

The API shipped in Node.js 14 over a month ago (sept 29), is currently in 14.x LTS and will go out in 12.x LTS tomorrow. As 12.x is going to be in maintenance after this release the odds of changing the API on that release are extremely slim. It will remain experimental in those releases, as we are not backporting marking it stable just yet. We can also make large changes to a stable API in a Semver-Major change.

To me marking this as stable is a commitment that we are not planning to make large changes so that folks can start adopting this API in good faith. If you disagree with this decision please open a new PR reverting this change.

[ljharb]

Thanks for the extra context - to be clear, I don't disagree with marking them stable, but it does "feel" too soon to me, so since yall believe it's ready, I was asking more to help calibrate my sense of "readiness" for the future.

[DerekNonGeneric]

I agree, feels too soon (never had the opportunity to muck around w/ it personally).

I would like some clarity on something real quick though: does this follow the same pattern format as .gitignore?

If it doesn't, I was planning on proposing a change to align the two formats and wouldn't want to see too much early-adopters break if having to wait for semver-major. 😐

[DerekNonGeneric]

It seems like the relevant PR is #34718.

It's possible that what I had in mind wouldn't be breaking the current pattern format (only one asterisk), but expanding it to give the same special meaning as two consecutive asterisks ("**") detailed at the bottom of that section in Git's docs. Might be alright to mark as stable if that's the case. I will have to get around to investigating that in the near term.

[guybedford]

@DerekNonGeneric it's not the same as the git syntax because a wildcard will match separators.

I do see the appeal in a double star proposal, but I'm still not sure how it would handle the use cases. What is implemented follows the rule of least power, and the novel problem space here is very different to globs and files and routing although has a lot of similarities. It would have been nice to have you involved in the process during the last few months - these are good questions to be asking.

Consider eg a features directory with a private lib folder. The way I suggest controlling deep exports at the point is by shadowing, something like:

{
  "exports": {
    "./*": "./features/*.js",
    "./lib/*": null
  }
}

that would then ensure that ./features/lib/* is not exposed by the exports field, but that only the direct files in features are.

We can certainly implement extensions to the system, and have discussed extensions like multiple patterns, but it is important that is done in a backwards compatible way at this point for the stability of the module system as a whole.

If you still disagree, and would like to revert this PR, you are still welcome to according to process though.

[DerekNonGeneric]

It would have been nice to have you involved in the process during the last few months - these are good questions to be asking.

I believe I became aware of the problem space when #33460 was opened.

We can certainly implement extensions to the system, and have discussed extensions like multiple patterns, but it is important that is done in a backwards compatible way at this point for the stability of the module system as a whole.

I would feel most comfortable if we could keep [as experimental] all modules features that make use of the package.json "exports" key's value being anything other than a string. Basically what I'm proposing is keeping the features that use this field's Object data structure as experimental. That would include conditional exports, self-reference specifiers, and more.

It seems to me like stabilizing this feature would lead to a slippery slope of stabilizing all the other features making use of the Object data structure and I don't particularly feel like this is more stable than something like self-reference specifiers, which has been available for several months.

To me marking this as stable is a commitment that we are not planning to make large changes so that folks can start adopting this API in good faith. If you disagree with this decision please open a new PR reverting this change.

I'm not thoroughly convinced that the Object data structure is well tested and would prefer to not commit to being stuck w/ it at this point. Are there any counter-points to opening a PR to revert this change? I would like more time to continue the discussion around this portion of module features and perhaps see about adding more tests for these experimental features.

[guybedford]

@DerekNonGeneric all exports features except for patterns were already marked as stable previously in #35742 and this is an important aspect of the stabilization of the modules system.

[DerekNonGeneric]

Are there plans on backporting #35742 to LTS? The whole 15.x release line is supposed to be experimental, so #35742 doesn't seem like a problem to me, I just wouldn't want to see that backported to 14.x for a while.

[DerekNonGeneric]

I have cycles towards the end of the week (Thursday, Friday) and will get back to y'all about this once I've used that time to flesh out my outstanding concerns in greater detail. Until then, I request that these two PRs retain the dont-land-on-v14.x label.

[MylesBorins]

@DerekNonGeneric I've removed the dont-land-on-v14.x label and switched it for the baking-for-lts label. In general we only put the dont-land labels when we know for sure that something won't land on that branch as we don't want to accidentally lose it in the future. In general please avoid adding that label to PRs that may eventually land on a given branch as it can making harder for the backporting team to keep track of what needs to land.

As mentioned above I don't think there is any intent to backport the stability in the near term. I've added the same label to both #35781 and #35742 to ensure they don't get backported as well.

In general decisions regarding what is included in an LTS release is delegated to the Release Working Group and is part of the working group's charter. So in the end the decision on if the stability is backported will fall to that group and isn't something that can be blocked via the normal consensus seeking process used to manage the master branch.

Regarding this specific feature, the original proposal was in July with a pull request being opened in August. We gathered feedback from multiple stakeholders. It was discussed in multiple modules meetings before eventually landing september 17 and going out in a 14.x release September 29th. It is included in the v12.20.0 release which is scheduled to go out tomorrow. We did receive ecosystem feedback as we were developing the feature that was very positive and actually resulted in the deprecation of subpath folders.

Considering that we have spent extensive time designing and building consensus around the current behavior, have released it for multiple months in a release line that is Active LTS and have a planned release tomorrow for 12.x which is going into maintenance mode... I think it is safe to say the bar is going to be extremely high to make any changes. It is worth noting that #35742 has already gone out in a 15.x release (making it officially stable) and #35781 is slated to go out in tomorrow's 15.x release (making it officially stable). 15.x is not an unstable branch, it is indeed more experimental than LTS but it has all the same commitments as far as API stability is concerned.

While I can appreciate that you have concerns about the design of the API, but there were many months and multiple opportunities to attempt to influence the API design concerns you might have.

[DerekNonGeneric]

We gathered feedback from multiple stakeholders. It was discussed in multiple modules meetings before eventually landing september 17 and going out in a 14.x release September 29th.

As an experimental feature on an experimental LTS release line. No problem.

It is included in the v12.20.0 release which is scheduled to go out tomorrow.

As an experimental feature on an LTS release line. No problem.

It is worth noting that #35742 has already gone out in a 15.x release (making it officially stable) and #35781 is slated to go out in tomorrow's 15.x release (making it officially stable).

As stable features on an experimental release line. Is that a problem?

15.x is not an unstable branch, it is indeed more experimental than LTS but it has all the same commitments as far as API stability is concerned.

For some reason, I thought that the whole ESM subsystem being developed on 13.x wasn't bound by SemVer due to 13.x being the experimental line.

While I can appreciate that you have concerns about the design of the API, but there were many months and multiple opportunities to attempt to influence the API design concerns you might have.

Marking an API in use by 27 packages as finalized seems premature to me, so I'm glad that this and related features remain experimental on LTS for the time being. As far as for the bar being extremely high to make any changes, it's likely that what I had in mind for a double star proposal could be done in a backward-compatible way (or maybe would have no effect on the 27 published), so the baking for LTS label seems very appropriate here and is probably my new favorite label. 💯

[MylesBorins]

As an experimental feature on an experimental release line. No problem.

That is not accurate. 14.x was never "experimental", and it is now LTS.

As stable features on an experimental release line. Is that a problem?

Again Current is not Experimental... it has as different stability contract than LTS, but not experimental.

For some reason, I thought that the whole ESM subsystem being developed on 13.x wasn't bound by SemVer due to 13.x being the experimental line.

Again, not an experimental release line. The feature was experimental, thus we can make breaking changes without having to do a Semver-Major. We arguably did this in 12.17.0 when we unflagged ESM. This was not without it's criticism as it broke expected behavior folks had for ESM in 12.x, but overall it was early enough in the 12.x LTS lifecycle and helped us to align the ESM implementation across releases.

double star proposal could be done in a backward-compatible way

I have no doubt that it could be backwards compatible, the challenge is also being forward compatible. We did a lot of work and planning over the last couple months to ensure the majority of behavior for the package features was going to be the same in 12 and 14 (and eventually 16). As a package author is is hard to support even subpath patterns without a semver-major right now as no version of 12.x before 12.20.0 is going to support it (including the version hosted in many serverless cloud providers).

The reason the bar is going to be so high right now is that introducing new, backwards compatible, features will mean that packages will be authored that can be resolved in 14.x / 15.x / 16.x but will fail on 12.x as it is likely too late to backport more features to a maintenance LTS release. That means we have another 18 months of support for that branch officially, and a much longer tail where the ecosystem might need to support legacy projects. For us to make a change that would result in packages not being supported on 12 we need a very very strong reason.

[DerekNonGeneric]

Determined that what I had in mind would actually be a new feature — would not conflict w/ subpath patterns (or any others) — essentially it would introduce a new capability, but wouldn't interfere w/ any of the pre-existing features. That being said, whether or not I can find the time to make an official proposition for this is up in the air, so bake on. 👍

[viT-1]

I don't know where is better to ask about, but can anyone answer, if I have node v16.13.1 and eslint 8.4.1 should eslint (without "settings" in eslintrc file) use node resolving for aliases configured with package.json?
package.json

...
"imports": {
"#common/*": "./src/common/*.js"
},
...

[ljharb]

@viT-1 eslint doesn’t do any resolving of anything. If you’re asking about eslint-plugin-import, file an issue there and I’ll explain.

[viT-1]

@ljharb Done there