Skip to content

module: exports & imports invalid slash deprecation

Rodrigo Muino Tomonari requested to merge github/fork/guybedford/exports-double-slash-validate into main

Fixes https://github.com/nodejs/node/issues/44316 with an alternative to https://github.com/nodejs/node/pull/44328 based on validation instead of normalization.

This PR creates a documentation-only deprecation for double slashes (//) in exports targets, subpaths and patterns available via --pending-deprecations. It also deprecates pattern matches starting with or ending with / as these could also create double slashing when substituted into a position immediatel before or after a slash.

The expectation for this PR would be to move to a runtime deprecation for the next major release, before moving to a validation error in a subsequent release.

Examples of deprecated cases for values of the "exports" field that would give an invalid target error when this moves to a runtime validation:

  • { "./x": ".//x/.js" } - invalid double slash in target
  • { "./*": "./x*" } for import 'pkg/x/z.js - invalid matched pattern starting with a /
  • { "./*": "./x*" } for import 'pkg/xz/ - invalid matched pattern ending with a /
  • { "./*": "./*x" } for import 'pkg//x' - invalid matched pattern starting with a /

Once the deprecation has been fully made, this would then effectively fix the case in https://github.com/nodejs/node/issues/44316 by throwing a validation error.

We still allow cases such as import 'pkg///x where there is an exports entry { ".///x": "./x.js" }, that is - the validation applies to the target and target replacements and not to the left hand match.

The disabling of leading and trailing / in pattern matching may be seen to be the most risky here, since there might be valid usage of:

{
  "exports": {
    "./x*": "./x*.js"
  }
}

supporting import 'pkg/xy' resolving into pkg/xy.js as well as import 'pkg/x/y' resolving into pkg/x/y.js.

If there is pushback on such cases, we could further restrict the pattern matching aspect to only disable cases that really end up resolving into double slashes, but I couldn't think of a case where the above matching / boundaries really would be wanted practically so it seems better off to make the restriction.

For review, I would advise starting with the spec diff (which also includes some unrelated refactorings), then the tests. Some minor refactorings are also included in the code.

//cc @nodejs/modules

Merge request reports