From 819aa312aa7255ecc89f7c217034553bf26d6001 Mon Sep 17 00:00:00 2001 From: Guy Bedford Date: Wed, 8 Sep 2021 10:36:52 -0700 Subject: [PATCH 1/5] module: deprecate trailing slash pattern mappings --- doc/api/deprecations.md | 7 +++++++ doc/api/esm.md | 2 ++ lib/internal/modules/esm/resolve.js | 18 ++++++++++++++++++ test/es-module/test-esm-exports.mjs | 6 ++++++ test/es-module/test-esm-local-deprecations.mjs | 5 +++++ .../es-modules/pattern-trailing-slash.mjs | 1 + .../node_modules/pkgexports/package.json | 3 ++- .../pkgexports/trailing-pattern-slash/index.js | 1 + 8 files changed, 42 insertions(+), 1 deletion(-) create mode 100644 test/fixtures/es-modules/pattern-trailing-slash.mjs create mode 100644 test/fixtures/node_modules/pkgexports/trailing-pattern-slash/index.js diff --git a/doc/api/deprecations.md b/doc/api/deprecations.md index aa4d0d024a4980..c9bfabdaefe8ea 100644 --- a/doc/api/deprecations.md +++ b/doc/api/deprecations.md @@ -2814,6 +2814,13 @@ Type: Documentation-only (supports [`--pending-deprecation`][]) The `'hash'` and `'mgf1Hash'` options are replaced with `'hashAlgorithm'` and `'mgf1HashAlgorithm'`. +### DEP0155: Trailing slashes in pattern specifier resolutions + +Type: Runtime + +The remapping of specifiers ending in `"/"` like `import 'pkg/x/'` is deprecated +for package `"exports"` and `"imports"` pattern resolutions. + [Legacy URL API]: url.md#legacy-url-api [NIST SP 800-38D]: https://nvlpubs.nist.gov/nistpubs/Legacy/SP/nistspecialpublication800-38d.pdf [RFC 6066]: https://tools.ietf.org/html/rfc6066#section-3 diff --git a/doc/api/esm.md b/doc/api/esm.md index c5e19b91b20717..7ca67e7e85c565 100644 --- a/doc/api/esm.md +++ b/doc/api/esm.md @@ -1191,6 +1191,8 @@ _isImports_, _conditions_) > _expansionKey_ up to but excluding the first _"*"_ character. > 1. If _patternBase_ is not **null** and _matchKey_ starts with but is not > equal to _patternBase_, then +> 1. If _matchKey_ ends with _"/"_, throw an _Invalid Module Specifier_ +> error. > 1. Let _patternTrailer_ be the substring of _expansionKey_ from the > index after the first _"*"_ character. > 1. If _patternTrailer_ has zero length, or if _matchKey_ ends with diff --git a/lib/internal/modules/esm/resolve.js b/lib/internal/modules/esm/resolve.js index ca0e1b316fc5b6..05e68e38a871e3 100644 --- a/lib/internal/modules/esm/resolve.js +++ b/lib/internal/modules/esm/resolve.js @@ -97,6 +97,21 @@ function emitFolderMapDeprecation(match, pjsonUrl, isExports, base) { ); } +function emitTrailingSlashPatternDeprecation(match, pjsonUrl, isExports, base) { + const pjsonPath = fileURLToPath(pjsonUrl); + if (emittedPackageWarnings.has(pjsonPath + '|' + match)) + return; + emittedPackageWarnings.add(pjsonPath + '|' + match); + process.emitWarning( + `Use of deprecated trailing slash pattern mapping "${match}" in the ${ + isExports ? '"exports"' : '"imports"'} field module resolution of the ` + + `package at ${pjsonPath}${base ? ` imported from ${fileURLToPath(base)}` : + ''}. Mapping specifiers ending in "/" is no longer supported.`, + 'DeprecationWarning', + 'DEP0155' + ); +} + /** * @param {URL} url * @param {URL} packageJSONUrl @@ -630,6 +645,9 @@ function packageExportsResolve( if (patternIndex !== -1 && StringPrototypeStartsWith(packageSubpath, StringPrototypeSlice(key, 0, patternIndex))) { + if (StringPrototypeEndsWith(packageSubpath, '/')) + emitTrailingSlashPatternDeprecation(packageSubpath, packageJSONUrl, + true, base); const patternTrailer = StringPrototypeSlice(key, patternIndex + 1); if (packageSubpath.length >= key.length && StringPrototypeEndsWith(packageSubpath, patternTrailer) && diff --git a/test/es-module/test-esm-exports.mjs b/test/es-module/test-esm-exports.mjs index a99814b0950d10..4b76c74fab072f 100644 --- a/test/es-module/test-esm-exports.mjs +++ b/test/es-module/test-esm-exports.mjs @@ -41,13 +41,19 @@ import fromInside from '../fixtures/node_modules/pkgexports/lib/hole.js'; ['pkgexports/dir2/dir2/trailer', { default: 'index' }], ['pkgexports/a/dir1/dir1', { default: 'main' }], ['pkgexports/a/b/dir1/dir1', { default: 'main' }], + + // Deprecated: + ['pkgexports/trailing-pattern-slash/', + { default: 'trailing-pattern-slash' }], ]); if (isRequire) { validSpecifiers.set('pkgexports/subpath/file', { default: 'file' }); validSpecifiers.set('pkgexports/subpath/dir1', { default: 'main' }); + // Deprecated: validSpecifiers.set('pkgexports/subpath/dir1/', { default: 'main' }); validSpecifiers.set('pkgexports/subpath/dir2', { default: 'index' }); + // Deprecated: validSpecifiers.set('pkgexports/subpath/dir2/', { default: 'index' }); } else { // No exports or main field diff --git a/test/es-module/test-esm-local-deprecations.mjs b/test/es-module/test-esm-local-deprecations.mjs index a1945f66f3422f..a9030b40912ddb 100644 --- a/test/es-module/test-esm-local-deprecations.mjs +++ b/test/es-module/test-esm-local-deprecations.mjs @@ -9,10 +9,14 @@ const selfDeprecatedFolders = const deprecatedFoldersIgnore = fixtures.path('/es-modules/deprecated-folders-ignore/main.js'); +const deprecatedTrailingSlashPattern = + fixtures.path('/es-modules/pattern-trailing-slash.mjs'); + const expectedWarnings = [ '"./" in the "exports" field', '"#self/" in the "imports" field', '"./folder/" in the "exports" field', + '"./trailing-pattern-slash/" in the "exports" field', ]; process.addListener('warning', (warning) => { @@ -28,5 +32,6 @@ process.on('exit', () => { (async () => { await import(pathToFileURL(selfDeprecatedFolders)); await import(pathToFileURL(deprecatedFoldersIgnore)); + await import(pathToFileURL(deprecatedTrailingSlashPattern)); })() .catch((err) => console.error(err)); diff --git a/test/fixtures/es-modules/pattern-trailing-slash.mjs b/test/fixtures/es-modules/pattern-trailing-slash.mjs new file mode 100644 index 00000000000000..e289305ee026b9 --- /dev/null +++ b/test/fixtures/es-modules/pattern-trailing-slash.mjs @@ -0,0 +1 @@ +import 'pkgexports/trailing-pattern-slash/'; diff --git a/test/fixtures/node_modules/pkgexports/package.json b/test/fixtures/node_modules/pkgexports/package.json index 6c80d5cb15a67e..9efb2e817afbd9 100644 --- a/test/fixtures/node_modules/pkgexports/package.json +++ b/test/fixtures/node_modules/pkgexports/package.json @@ -57,6 +57,7 @@ "./subpath/": "./subpath/", "./subpath/sub-*": "./subpath/dir1/*.js", "./subpath/sub-*.js": "./subpath/dir1/*.js", - "./features/*": "./subpath/*/*.js" + "./features/*": "./subpath/*/*.js", + "./trailing-pattern-slash*": "./trailing-pattern-slash*index.js" } } diff --git a/test/fixtures/node_modules/pkgexports/trailing-pattern-slash/index.js b/test/fixtures/node_modules/pkgexports/trailing-pattern-slash/index.js new file mode 100644 index 00000000000000..613bddbb6a44dd --- /dev/null +++ b/test/fixtures/node_modules/pkgexports/trailing-pattern-slash/index.js @@ -0,0 +1 @@ +module.exports = 'trailing-pattern-slash'; From 266a0a6d9ccaa9fe2e1d8f72c7957662117169f8 Mon Sep 17 00:00:00 2001 From: Guy Bedford Date: Wed, 8 Sep 2021 13:56:10 -0700 Subject: [PATCH 2/5] fixup: exports deprecation check tests --- test/es-module/test-esm-exports-deprecations.mjs | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/test/es-module/test-esm-exports-deprecations.mjs b/test/es-module/test-esm-exports-deprecations.mjs index 2dd2756e2ee844..58146d4300a69c 100644 --- a/test/es-module/test-esm-exports-deprecations.mjs +++ b/test/es-module/test-esm-exports-deprecations.mjs @@ -5,13 +5,17 @@ let curWarning = 0; const expectedWarnings = [ '"./sub/"', '"./fallbackdir/"', + '"./trailing-pattern-slash/"', '"./subpath/"', + '"./subpath/dir1/"', + '"./subpath/dir2/"', 'no_exports', 'default_index', ]; process.addListener('warning', mustCall((warning) => { assert(warning.stack.includes(expectedWarnings[curWarning++]), warning.stack); + console.log(expectedWarnings[curWarning - 1] + ' passed'); }, expectedWarnings.length)); await import('./test-esm-exports.mjs'); From 5129ba6e924721339d100e8c0ebeb4fdfe6a3f66 Mon Sep 17 00:00:00 2001 From: Guy Bedford Date: Wed, 8 Sep 2021 14:20:38 -0700 Subject: [PATCH 3/5] fixup: make documentation-only deprecation Co-authored-by: Antoine du Hamel --- doc/api/deprecations.md | 6 ++++++ lib/internal/modules/esm/resolve.js | 2 ++ test/es-module/test-esm-exports-deprecations.mjs | 1 + test/es-module/test-esm-local-deprecations.mjs | 2 ++ 4 files changed, 11 insertions(+) diff --git a/doc/api/deprecations.md b/doc/api/deprecations.md index c9bfabdaefe8ea..a7b70b6b567d5f 100644 --- a/doc/api/deprecations.md +++ b/doc/api/deprecations.md @@ -2815,6 +2815,12 @@ The `'hash'` and `'mgf1Hash'` options are replaced with `'hashAlgorithm'` and `'mgf1HashAlgorithm'`. ### DEP0155: Trailing slashes in pattern specifier resolutions + Type: Runtime diff --git a/lib/internal/modules/esm/resolve.js b/lib/internal/modules/esm/resolve.js index 05e68e38a871e3..627e17afce69d9 100644 --- a/lib/internal/modules/esm/resolve.js +++ b/lib/internal/modules/esm/resolve.js @@ -40,6 +40,7 @@ const { sep, relative, resolve } = require('path'); const preserveSymlinks = getOptionValue('--preserve-symlinks'); const preserveSymlinksMain = getOptionValue('--preserve-symlinks-main'); const typeFlag = getOptionValue('--input-type'); +const pendingDeprecation = getOptionValue('--pending-deprecation'); const { URL, pathToFileURL, fileURLToPath } = require('internal/url'); const { ERR_INPUT_TYPE_NOT_ALLOWED, @@ -98,6 +99,7 @@ function emitFolderMapDeprecation(match, pjsonUrl, isExports, base) { } function emitTrailingSlashPatternDeprecation(match, pjsonUrl, isExports, base) { + if (!pendingDeprecation) return; const pjsonPath = fileURLToPath(pjsonUrl); if (emittedPackageWarnings.has(pjsonPath + '|' + match)) return; diff --git a/test/es-module/test-esm-exports-deprecations.mjs b/test/es-module/test-esm-exports-deprecations.mjs index 58146d4300a69c..48c191a5fab2df 100644 --- a/test/es-module/test-esm-exports-deprecations.mjs +++ b/test/es-module/test-esm-exports-deprecations.mjs @@ -1,3 +1,4 @@ +// Flags: --pending-deprecation import { mustCall } from '../common/index.mjs'; import assert from 'assert'; diff --git a/test/es-module/test-esm-local-deprecations.mjs b/test/es-module/test-esm-local-deprecations.mjs index a9030b40912ddb..8d946b6650ed3b 100644 --- a/test/es-module/test-esm-local-deprecations.mjs +++ b/test/es-module/test-esm-local-deprecations.mjs @@ -1,3 +1,5 @@ +// Flags: --pending-deprecation + import '../common/index.mjs'; import assert from 'assert'; import fixtures from '../common/fixtures.js'; From 8a0989a3321d8136c80e8ef3e1a6e39a46ea0378 Mon Sep 17 00:00:00 2001 From: Guy Bedford Date: Wed, 8 Sep 2021 14:51:04 -0700 Subject: [PATCH 4/5] fixup: deprecation docs Co-authored-by: Antoine du Hamel --- doc/api/deprecations.md | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/doc/api/deprecations.md b/doc/api/deprecations.md index a7b70b6b567d5f..e25e855c085e9b 100644 --- a/doc/api/deprecations.md +++ b/doc/api/deprecations.md @@ -2819,10 +2819,11 @@ and `'mgf1HashAlgorithm'`. changes: - version: REPLACEME pr-url: https://github.com/nodejs/node/pull/40039 - description: Documentation-only. + description: Documentation-only deprecation + with `--pending-deprecation` support. --> -Type: Runtime +Type: Documentation-only (supports [`--pending-deprecation`][]) The remapping of specifiers ending in `"/"` like `import 'pkg/x/'` is deprecated for package `"exports"` and `"imports"` pattern resolutions. From 2dc4199c56438a45f4577445f6f96ef0360e56f7 Mon Sep 17 00:00:00 2001 From: Guy Bedford Date: Wed, 8 Sep 2021 14:57:36 -0700 Subject: [PATCH 5/5] fixup: log Co-authored-by: Antoine du Hamel --- test/es-module/test-esm-exports-deprecations.mjs | 1 - 1 file changed, 1 deletion(-) diff --git a/test/es-module/test-esm-exports-deprecations.mjs b/test/es-module/test-esm-exports-deprecations.mjs index 48c191a5fab2df..8c7a07b0204b9a 100644 --- a/test/es-module/test-esm-exports-deprecations.mjs +++ b/test/es-module/test-esm-exports-deprecations.mjs @@ -16,7 +16,6 @@ const expectedWarnings = [ process.addListener('warning', mustCall((warning) => { assert(warning.stack.includes(expectedWarnings[curWarning++]), warning.stack); - console.log(expectedWarnings[curWarning - 1] + ' passed'); }, expectedWarnings.length)); await import('./test-esm-exports.mjs');