From 719ae9d97b0d194af904e15cfe2a2647319b22bb Mon Sep 17 00:00:00 2001 From: Antoine du Hamel Date: Sun, 21 Aug 2022 13:30:55 +0200 Subject: [PATCH] module: fix `null` restrictions in imports and exports patterns --- lib/internal/modules/esm/resolve.js | 31 ++++++++++++------- test/es-module/test-esm-exports.mjs | 7 +++++ test/es-module/test-esm-imports.mjs | 22 ++++++++++--- .../es-modules/pkgimports/package.json | 2 ++ .../node_modules/pkgexports/package.json | 1 + 5 files changed, 48 insertions(+), 15 deletions(-) diff --git a/lib/internal/modules/esm/resolve.js b/lib/internal/modules/esm/resolve.js index a923508f2fd866..828dcb8dec9e24 100644 --- a/lib/internal/modules/esm/resolve.js +++ b/lib/internal/modules/esm/resolve.js @@ -573,8 +573,12 @@ function packageExportsResolve( for (let i = 0; i < keys.length; i++) { const key = keys[i]; const patternIndex = StringPrototypeIndexOf(key, '*'); + const normalizedSubpath = patternIndex !== -1 && exports[key] === null && + StringPrototypeIncludes(packageSubpath, '//') ? + RegExpPrototypeSymbolReplace(/\/+/g, packageSubpath, '/') : + packageSubpath; if (patternIndex !== -1 && - StringPrototypeStartsWith(packageSubpath, + StringPrototypeStartsWith(normalizedSubpath, StringPrototypeSlice(key, 0, patternIndex))) { // When this reaches EOL, this can throw at the top of the whole function: // @@ -582,18 +586,18 @@ function packageExportsResolve( // throwInvalidSubpath(packageSubpath) // // To match "imports" and the spec. - if (StringPrototypeEndsWith(packageSubpath, '/')) - emitTrailingSlashPatternDeprecation(packageSubpath, packageJSONUrl, + if (StringPrototypeEndsWith(normalizedSubpath, '/')) + emitTrailingSlashPatternDeprecation(normalizedSubpath, packageJSONUrl, base); const patternTrailer = StringPrototypeSlice(key, patternIndex + 1); - if (packageSubpath.length >= key.length && - StringPrototypeEndsWith(packageSubpath, patternTrailer) && + if (normalizedSubpath.length >= key.length && + StringPrototypeEndsWith(normalizedSubpath, patternTrailer) && patternKeyCompare(bestMatch, key) === 1 && StringPrototypeLastIndexOf(key, '*') === patternIndex) { bestMatch = key; bestMatchSubpath = StringPrototypeSlice( - packageSubpath, patternIndex, - packageSubpath.length - patternTrailer.length); + normalizedSubpath, patternIndex, + normalizedSubpath.length - patternTrailer.length); } } } @@ -666,18 +670,22 @@ function packageImportsResolve(name, base, conditions) { for (let i = 0; i < keys.length; i++) { const key = keys[i]; const patternIndex = StringPrototypeIndexOf(key, '*'); + const normalizedSubpath = patternIndex !== -1 && imports[key] === null && + StringPrototypeIncludes(name, '//') ? + RegExpPrototypeSymbolReplace(/\/+/g, name, '/') : + name; if (patternIndex !== -1 && - StringPrototypeStartsWith(name, + StringPrototypeStartsWith(normalizedSubpath, StringPrototypeSlice(key, 0, patternIndex))) { const patternTrailer = StringPrototypeSlice(key, patternIndex + 1); - if (name.length >= key.length && - StringPrototypeEndsWith(name, patternTrailer) && + if (normalizedSubpath.length >= key.length && + StringPrototypeEndsWith(normalizedSubpath, patternTrailer) && patternKeyCompare(bestMatch, key) === 1 && StringPrototypeLastIndexOf(key, '*') === patternIndex) { bestMatch = key; bestMatchSubpath = StringPrototypeSlice( - name, patternIndex, name.length - patternTrailer.length); + normalizedSubpath, patternIndex, normalizedSubpath.length - patternTrailer.length); } } } @@ -1124,6 +1132,7 @@ module.exports = { const { defaultGetFormatWithoutErrors, } = require('internal/modules/esm/get_format'); +const console = require('console'); if (policy) { const $defaultResolve = defaultResolve; diff --git a/test/es-module/test-esm-exports.mjs b/test/es-module/test-esm-exports.mjs index c08fc9f8d9a54a..b2f132fccad648 100644 --- a/test/es-module/test-esm-exports.mjs +++ b/test/es-module/test-esm-exports.mjs @@ -74,7 +74,14 @@ import fromInside from '../fixtures/node_modules/pkgexports/lib/hole.js'; ['pkgexports/invalid1', './invalid1'], ['pkgexports/invalid4', './invalid4'], // Null mapping + ['pkgexports/sub/internal/test', './sub/internal/test'], + ['pkgexports/sub//internal/test', './sub//internal/test'], + ['pkgexports/sub/internal//test', './sub/internal//test'], + ['pkgexports/sub//internal//test', './sub//internal//test'], + ['pkgexports/sub/////internal/////test', './sub/////internal/////test'], ['pkgexports/null', './null'], + ['pkgexports//null', './/null'], + ['pkgexports/////null', './////null'], ['pkgexports/null/subpath', './null/subpath'], // Empty fallback ['pkgexports/nofallback1', './nofallback1'], diff --git a/test/es-module/test-esm-imports.mjs b/test/es-module/test-esm-imports.mjs index 77726c06f101b2..d4e11f69bc9889 100644 --- a/test/es-module/test-esm-imports.mjs +++ b/test/es-module/test-esm-imports.mjs @@ -79,10 +79,17 @@ const { requireImport, importImport } = importer; '#missing', // Explicit null import '#null', + '#subpath/null', // No condition match import '#nullcondition', // Null subpath shadowing '#subpath/nullshadow/x', + // Null pattern + '#subpath/internal/test', + '#subpath//internal/test', + '#subpath/internal//test', + '#subpath//internal//test', + '#subpath/////internal/////test', ]); for (const specifier of undefinedImports) { @@ -94,10 +101,17 @@ const { requireImport, importImport } = importer; } // Handle not found for the defined imports target not existing - loadFixture('#notfound').catch(mustCall((err) => { - strictEqual(err.code, - isRequire ? 'MODULE_NOT_FOUND' : 'ERR_MODULE_NOT_FOUND'); - })); + const nonDefinedImports = new Set([ + '#notfound', + '#subpath//null', + '#subpath/////null', + ]); + for (const specifier of nonDefinedImports) { + loadFixture(specifier).catch(mustCall((err) => { + strictEqual(err.code, + isRequire ? 'MODULE_NOT_FOUND' : 'ERR_MODULE_NOT_FOUND'); + })); + } }); // CJS resolver must still support #package packages in node_modules diff --git a/test/fixtures/es-modules/pkgimports/package.json b/test/fixtures/es-modules/pkgimports/package.json index 5b2f2a2ac5dd5f..6971e8b6fbcdea 100644 --- a/test/fixtures/es-modules/pkgimports/package.json +++ b/test/fixtures/es-modules/pkgimports/package.json @@ -6,6 +6,8 @@ "require": "./requirebranch.js" }, "#subpath/*": "./sub/*", + "#subpath/internal/*": null, + "#subpath/null": null, "#subpath/*.asdf": "./test.js", "#external": "pkgexports/valid-cjs", "#external/subpath/*": "pkgexports/sub/*", diff --git a/test/fixtures/node_modules/pkgexports/package.json b/test/fixtures/node_modules/pkgexports/package.json index b686257875bc42..80edb21c767398 100644 --- a/test/fixtures/node_modules/pkgexports/package.json +++ b/test/fixtures/node_modules/pkgexports/package.json @@ -5,6 +5,7 @@ "./space": "./sp%20ce.js", "./valid-cjs": "./asdf.js", "./sub/*": "./*", + "./sub/internal/*": null, "./belowdir/*": "../belowdir/*", "./belowfile": "../belowfile", "./null": null,