Skip to content

Commit

Permalink
Fix side effects glob matching (#7288)
Browse files Browse the repository at this point in the history
  • Loading branch information
mischnic committed Nov 12, 2021
1 parent ab0f5e1 commit f651829
Show file tree
Hide file tree
Showing 9 changed files with 117 additions and 7 deletions.
22 changes: 15 additions & 7 deletions packages/utils/node-resolver-core/src/NodeResolver.js
Expand Up @@ -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 =>
Expand Down

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.

93 changes: 93 additions & 0 deletions packages/utils/node-resolver-core/test/resolver.js
Expand Up @@ -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,
Expand Down

0 comments on commit f651829

Please sign in to comment.