From f65182989334638e5fca7d4ce637bdf471b9b874 Mon Sep 17 00:00:00 2001 From: Niklas Mischkulnig <4586894+mischnic@users.noreply.github.com> Date: Fri, 12 Nov 2021 15:35:59 +0100 Subject: [PATCH] Fix side effects glob matching (#7288) --- .../node-resolver-core/src/NodeResolver.js | 22 +++-- .../side-effects-false-glob/a/index.js | 0 .../side-effects-false-glob/b/index.js | 0 .../side-effects-false-glob/package.json | 8 ++ .../side-effects-false-glob/sub/a/index.js | 0 .../side-effects-false-glob/sub/b/index.js | 0 .../side-effects-false-glob/sub/index.js | 0 .../side-effects-false-glob/sub/index.json | 1 + .../utils/node-resolver-core/test/resolver.js | 93 +++++++++++++++++++ 9 files changed, 117 insertions(+), 7 deletions(-) create mode 100644 packages/utils/node-resolver-core/test/fixture/node_modules/side-effects-false-glob/a/index.js create mode 100644 packages/utils/node-resolver-core/test/fixture/node_modules/side-effects-false-glob/b/index.js create mode 100644 packages/utils/node-resolver-core/test/fixture/node_modules/side-effects-false-glob/package.json create mode 100644 packages/utils/node-resolver-core/test/fixture/node_modules/side-effects-false-glob/sub/a/index.js create mode 100644 packages/utils/node-resolver-core/test/fixture/node_modules/side-effects-false-glob/sub/b/index.js create mode 100644 packages/utils/node-resolver-core/test/fixture/node_modules/side-effects-false-glob/sub/index.js create mode 100644 packages/utils/node-resolver-core/test/fixture/node_modules/side-effects-false-glob/sub/index.json diff --git a/packages/utils/node-resolver-core/src/NodeResolver.js b/packages/utils/node-resolver-core/src/NodeResolver.js index 6f54806974c..9d5bdb4c359 100644 --- a/packages/utils/node-resolver-core/src/NodeResolver.js +++ b/packages/utils/node-resolver-core/src/NodeResolver.js @@ -1170,13 +1170,21 @@ export default class NodeResolver { case 'boolean': return pkg.sideEffects; case 'string': { - let sideEffects = pkg.sideEffects; - invariant(typeof sideEffects === 'string'); - return micromatch.isMatch( - path.relative(pkg.pkgdir, filePath), - sideEffects, - {matchBase: true}, - ); + let glob = pkg.sideEffects; + invariant(typeof glob === 'string'); + + let relative = path.relative(pkg.pkgdir, filePath); + if (!glob.includes('/')) { + glob = `**/${glob}`; + } + + // Trim off "./" to make micromatch behave correctly, + // `path.relative` never returns a leading "./" + if (glob.startsWith('./')) { + glob = glob.substr(2); + } + + return micromatch.isMatch(relative, glob, {dot: true}); } case 'object': return pkg.sideEffects.some(sideEffects => diff --git a/packages/utils/node-resolver-core/test/fixture/node_modules/side-effects-false-glob/a/index.js b/packages/utils/node-resolver-core/test/fixture/node_modules/side-effects-false-glob/a/index.js new file mode 100644 index 00000000000..e69de29bb2d diff --git a/packages/utils/node-resolver-core/test/fixture/node_modules/side-effects-false-glob/b/index.js b/packages/utils/node-resolver-core/test/fixture/node_modules/side-effects-false-glob/b/index.js new file mode 100644 index 00000000000..e69de29bb2d diff --git a/packages/utils/node-resolver-core/test/fixture/node_modules/side-effects-false-glob/package.json b/packages/utils/node-resolver-core/test/fixture/node_modules/side-effects-false-glob/package.json new file mode 100644 index 00000000000..f748689fc82 --- /dev/null +++ b/packages/utils/node-resolver-core/test/fixture/node_modules/side-effects-false-glob/package.json @@ -0,0 +1,8 @@ +{ + "name": "side-effects-false-glob", + "sideEffects": [ + "a/*.js", + "./sub/*.js", + "*.json" + ] +} diff --git a/packages/utils/node-resolver-core/test/fixture/node_modules/side-effects-false-glob/sub/a/index.js b/packages/utils/node-resolver-core/test/fixture/node_modules/side-effects-false-glob/sub/a/index.js new file mode 100644 index 00000000000..e69de29bb2d diff --git a/packages/utils/node-resolver-core/test/fixture/node_modules/side-effects-false-glob/sub/b/index.js b/packages/utils/node-resolver-core/test/fixture/node_modules/side-effects-false-glob/sub/b/index.js new file mode 100644 index 00000000000..e69de29bb2d diff --git a/packages/utils/node-resolver-core/test/fixture/node_modules/side-effects-false-glob/sub/index.js b/packages/utils/node-resolver-core/test/fixture/node_modules/side-effects-false-glob/sub/index.js new file mode 100644 index 00000000000..e69de29bb2d diff --git a/packages/utils/node-resolver-core/test/fixture/node_modules/side-effects-false-glob/sub/index.json b/packages/utils/node-resolver-core/test/fixture/node_modules/side-effects-false-glob/sub/index.json new file mode 100644 index 00000000000..0967ef424bc --- /dev/null +++ b/packages/utils/node-resolver-core/test/fixture/node_modules/side-effects-false-glob/sub/index.json @@ -0,0 +1 @@ +{} diff --git a/packages/utils/node-resolver-core/test/resolver.js b/packages/utils/node-resolver-core/test/resolver.js index c993dc8ce12..dce617a5e9d 100644 --- a/packages/utils/node-resolver-core/test/resolver.js +++ b/packages/utils/node-resolver-core/test/resolver.js @@ -1015,6 +1015,99 @@ describe('resolver', function () { }); }); + describe('sideEffects: globs', function () { + it('should determine sideEffects correctly (matched)', async function () { + let resolved = await resolver.resolve({ + env: BROWSER_ENV, + filename: 'side-effects-false-glob/a/index', + specifierType: 'esm', + parent: path.join(rootDir, 'foo.js'), + }); + assert.deepEqual( + {filePath: resolved?.filePath, sideEffects: resolved?.sideEffects}, + { + filePath: path.resolve( + rootDir, + 'node_modules/side-effects-false-glob/a/index.js', + ), + sideEffects: undefined, + }, + ); + }); + it('should determine sideEffects correctly (unmatched)', async function () { + let resolved = await resolver.resolve({ + env: BROWSER_ENV, + filename: 'side-effects-false-glob/b/index.js', + specifierType: 'esm', + parent: path.join(rootDir, 'foo.js'), + }); + assert.deepEqual( + {filePath: resolved?.filePath, sideEffects: resolved?.sideEffects}, + { + filePath: path.resolve( + rootDir, + 'node_modules/side-effects-false-glob/b/index.js', + ), + sideEffects: false, + }, + ); + }); + it('should determine sideEffects correctly (matched dotslash)', async function () { + let resolved = await resolver.resolve({ + env: BROWSER_ENV, + filename: 'side-effects-false-glob/sub/index.js', + specifierType: 'esm', + parent: path.join(rootDir, 'foo.js'), + }); + assert.deepEqual( + {filePath: resolved?.filePath, sideEffects: resolved?.sideEffects}, + { + filePath: path.resolve( + rootDir, + 'node_modules/side-effects-false-glob/sub/index.js', + ), + sideEffects: undefined, + }, + ); + }); + it('should determine sideEffects correctly (unmatched, prefix in subdir)', async function () { + let resolved = await resolver.resolve({ + env: BROWSER_ENV, + filename: 'side-effects-false-glob/sub/a/index.js', + specifierType: 'esm', + parent: path.join(rootDir, 'foo.js'), + }); + assert.deepEqual( + {filePath: resolved?.filePath, sideEffects: resolved?.sideEffects}, + { + filePath: path.resolve( + rootDir, + 'node_modules/side-effects-false-glob/sub/a/index.js', + ), + sideEffects: false, + }, + ); + }); + it('should determine sideEffects correctly (only name)', async function () { + let resolved = await resolver.resolve({ + env: BROWSER_ENV, + filename: 'side-effects-false-glob/sub/index.json', + specifierType: 'esm', + parent: path.join(rootDir, 'foo.js'), + }); + assert.deepEqual( + {filePath: resolved?.filePath, sideEffects: resolved?.sideEffects}, + { + filePath: path.resolve( + rootDir, + 'node_modules/side-effects-false-glob/sub/index.json', + ), + sideEffects: undefined, + }, + ); + }); + }); + it('should not resolve a node module for URL dependencies', async function () { let resolved = await resolver.resolve({ env: BROWSER_ENV,