Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

module: deprecate trailing slash pattern mappings #40039

Closed
wants to merge 5 commits into from
Closed
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
13 changes: 13 additions & 0 deletions doc/api/deprecations.md
Expand Up @@ -2814,6 +2814,19 @@ Type: Documentation-only (supports [`--pending-deprecation`][])
The `'hash'` and `'mgf1Hash'` options are replaced with `'hashAlgorithm'`
and `'mgf1HashAlgorithm'`.

### DEP0155: Trailing slashes in pattern specifier resolutions
guybedford marked this conversation as resolved.
Show resolved Hide resolved
<!-- YAML
changes:
- version: REPLACEME
pr-url: https://github.com/nodejs/node/pull/40039
description: Documentation-only.
guybedford marked this conversation as resolved.
Show resolved Hide resolved
-->

Type: Runtime
guybedford marked this conversation as resolved.
Show resolved Hide resolved
guybedford marked this conversation as resolved.
Show resolved Hide resolved

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
Expand Down
2 changes: 2 additions & 0 deletions doc/api/esm.md
Expand Up @@ -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
Expand Down
20 changes: 20 additions & 0 deletions lib/internal/modules/esm/resolve.js
Expand Up @@ -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,
Expand Down Expand Up @@ -97,6 +98,22 @@ 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;
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
Expand Down Expand Up @@ -630,6 +647,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) &&
Expand Down
5 changes: 5 additions & 0 deletions test/es-module/test-esm-exports-deprecations.mjs
@@ -1,17 +1,22 @@
// Flags: --pending-deprecation
import { mustCall } from '../common/index.mjs';
import assert from 'assert';

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');
guybedford marked this conversation as resolved.
Show resolved Hide resolved
}, expectedWarnings.length));

await import('./test-esm-exports.mjs');
6 changes: 6 additions & 0 deletions test/es-module/test-esm-exports.mjs
Expand Up @@ -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
Expand Down
7 changes: 7 additions & 0 deletions 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';
Expand All @@ -9,10 +11,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) => {
Expand All @@ -28,5 +34,6 @@ process.on('exit', () => {
(async () => {
await import(pathToFileURL(selfDeprecatedFolders));
await import(pathToFileURL(deprecatedFoldersIgnore));
await import(pathToFileURL(deprecatedTrailingSlashPattern));
})()
.catch((err) => console.error(err));
1 change: 1 addition & 0 deletions test/fixtures/es-modules/pattern-trailing-slash.mjs
@@ -0,0 +1 @@
import 'pkgexports/trailing-pattern-slash/';
3 changes: 2 additions & 1 deletion test/fixtures/node_modules/pkgexports/package.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.