From 63f2a1864dc39c7a1860be884b87af81cac3d59d Mon Sep 17 00:00:00 2001 From: cumul Date: Mon, 21 Mar 2022 06:08:43 +0900 Subject: [PATCH] Make NodeResolver check realpath before resolving with `source` entry (#7846) --- packages/utils/node-resolver-core/.gitignore | 1 + .../node-resolver-core/src/NodeResolver.js | 12 +- .../node_modules/source-pnpm/dist.js | 0 .../node_modules/source-pnpm/package.json | 5 + .../node_modules/source-pnpm/source.js | 0 .../utils/node-resolver-core/test/resolver.js | 246 ++++++++---------- 6 files changed, 126 insertions(+), 138 deletions(-) create mode 100644 packages/utils/node-resolver-core/test/fixture/node_modules/.pnpm/source-pnpm@1.0.0/node_modules/source-pnpm/dist.js create mode 100644 packages/utils/node-resolver-core/test/fixture/node_modules/.pnpm/source-pnpm@1.0.0/node_modules/source-pnpm/package.json create mode 100644 packages/utils/node-resolver-core/test/fixture/node_modules/.pnpm/source-pnpm@1.0.0/node_modules/source-pnpm/source.js diff --git a/packages/utils/node-resolver-core/.gitignore b/packages/utils/node-resolver-core/.gitignore index 33dfa7267da..7ed92ab6e96 100644 --- a/packages/utils/node-resolver-core/.gitignore +++ b/packages/utils/node-resolver-core/.gitignore @@ -1 +1,2 @@ !/test/fixture/node_modules +!/test/fixture/node_modules/.pnpm/*/node_modules diff --git a/packages/utils/node-resolver-core/src/NodeResolver.js b/packages/utils/node-resolver-core/src/NodeResolver.js index 0f400fd2fda..f7802facf3e 100644 --- a/packages/utils/node-resolver-core/src/NodeResolver.js +++ b/packages/utils/node-resolver-core/src/NodeResolver.js @@ -906,11 +906,17 @@ export default class NodeResolver { pkg.pkgfile = file; pkg.pkgdir = dir; - // If the package has a `source` field, check if it is behind a symlink. - // If so, we treat the module as source code rather than a pre-compiled module. + // If the package has a `source` field, make sure + // - the package is behind symlinks + // - and the realpath to the packages does not includes `node_modules`. + // Since such package is likely a pre-compiled module + // installed with package managers, rather than including a source code. if (pkg.source) { let realpath = await this.fs.realpath(file); - if (realpath === file) { + if ( + realpath === file || + realpath.includes(`${path.sep}node_modules${path.sep}`) + ) { delete pkg.source; } } diff --git a/packages/utils/node-resolver-core/test/fixture/node_modules/.pnpm/source-pnpm@1.0.0/node_modules/source-pnpm/dist.js b/packages/utils/node-resolver-core/test/fixture/node_modules/.pnpm/source-pnpm@1.0.0/node_modules/source-pnpm/dist.js new file mode 100644 index 00000000000..e69de29bb2d diff --git a/packages/utils/node-resolver-core/test/fixture/node_modules/.pnpm/source-pnpm@1.0.0/node_modules/source-pnpm/package.json b/packages/utils/node-resolver-core/test/fixture/node_modules/.pnpm/source-pnpm@1.0.0/node_modules/source-pnpm/package.json new file mode 100644 index 00000000000..5829a9d15ee --- /dev/null +++ b/packages/utils/node-resolver-core/test/fixture/node_modules/.pnpm/source-pnpm@1.0.0/node_modules/source-pnpm/package.json @@ -0,0 +1,5 @@ +{ + "name": "source-pnpm", + "main": "dist.js", + "source": "source.js" +} diff --git a/packages/utils/node-resolver-core/test/fixture/node_modules/.pnpm/source-pnpm@1.0.0/node_modules/source-pnpm/source.js b/packages/utils/node-resolver-core/test/fixture/node_modules/.pnpm/source-pnpm@1.0.0/node_modules/source-pnpm/source.js new file mode 100644 index 00000000000..e69de29bb2d diff --git a/packages/utils/node-resolver-core/test/resolver.js b/packages/utils/node-resolver-core/test/resolver.js index d2ddb27ccdd..0f1c818429a 100644 --- a/packages/utils/node-resolver-core/test/resolver.js +++ b/packages/utils/node-resolver-core/test/resolver.js @@ -47,6 +47,13 @@ describe('resolver', function () { path.join(rootDir, 'packages/source'), path.join(rootDir, 'node_modules/source'), ); + await outputFS.symlink( + path.join( + rootDir, + 'node_modules/.pnpm/source-pnpm@1.0.0/node_modules/source-pnpm', + ), + path.join(rootDir, 'node_modules/source-pnpm'), + ); await outputFS.symlink( path.join(rootDir, 'packages/source-alias'), path.join(rootDir, 'node_modules/source-alias'), @@ -2074,152 +2081,121 @@ describe('resolver', function () { }); describe('source field', function () { - it('should use the source field when symlinked', async function () { - let resolved = await resolver.resolve({ - env: BROWSER_ENV, - filename: 'source', - specifierType: 'esm', - parent: path.join(rootDir, 'foo.js'), - }); - assert.deepEqual(resolved, { - filePath: path.join(rootDir, 'packages', 'source', 'source.js'), - sideEffects: undefined, - query: undefined, - invalidateOnFileCreate: [ - { - fileName: 'package.json', - aboveFilePath: path.join(rootDir, 'index'), - }, - { - fileName: 'package.json', - aboveFilePath: path.join(rootDir, 'foo.js'), - }, - { - fileName: 'node_modules/source', - aboveFilePath: path.join(rootDir, 'foo.js'), - }, - ], - invalidateOnFileChange: [ - path.join(rootDir, 'package.json'), - path.join(rootDir, 'node_modules', 'source', 'package.json'), - ], + describe('package behind symlinks', function () { + it('should use the source field, when its realpath is not under `node_modules`', async function () { + let resolved = await resolver.resolve({ + env: BROWSER_ENV, + filename: 'source', + specifierType: 'esm', + parent: path.join(rootDir, 'foo.js'), + }); + assert.deepEqual(resolved, { + filePath: path.join(rootDir, 'packages', 'source', 'source.js'), + sideEffects: undefined, + query: undefined, + invalidateOnFileCreate: [ + { + fileName: 'package.json', + aboveFilePath: path.join(rootDir, 'index'), + }, + { + fileName: 'package.json', + aboveFilePath: path.join(rootDir, 'foo.js'), + }, + { + fileName: 'node_modules/source', + aboveFilePath: path.join(rootDir, 'foo.js'), + }, + ], + invalidateOnFileChange: [ + path.join(rootDir, 'package.json'), + path.join(rootDir, 'node_modules', 'source', 'package.json'), + ], + }); }); - }); - it('should not use the source field when not symlinked', async function () { - let resolved = await resolver.resolve({ - env: BROWSER_ENV, - filename: 'source-not-symlinked', - specifierType: 'esm', - parent: path.join(rootDir, 'foo.js'), - }); - assert.deepEqual(resolved, { - filePath: path.join( - rootDir, - 'node_modules', - 'source-not-symlinked', - 'dist.js', - ), - sideEffects: undefined, - query: undefined, - invalidateOnFileCreate: [ - { - fileName: 'package.json', - aboveFilePath: path.join(rootDir, 'index'), - }, - { - fileName: 'package.json', - aboveFilePath: path.join(rootDir, 'foo.js'), - }, - { - fileName: 'node_modules/source-not-symlinked', - aboveFilePath: path.join(rootDir, 'foo.js'), - }, - ], - invalidateOnFileChange: [ - path.join(rootDir, 'package.json'), - path.join( + it('should not use the source field, when its realpath is under `node_modules`', async function () { + let resolved = await resolver.resolve({ + env: BROWSER_ENV, + filename: 'source-pnpm', + specifierType: 'esm', + parent: path.join(rootDir, 'foo.js'), + }); + assert.deepEqual(resolved, { + filePath: path.join( rootDir, 'node_modules', - 'source-not-symlinked', - 'package.json', + '.pnpm', + 'source-pnpm@1.0.0', + 'node_modules', + 'source-pnpm', + 'dist.js', ), - ], - }); - }); - - it('should use the source field as an alias when symlinked', async function () { - let resolved = await resolver.resolve({ - env: BROWSER_ENV, - filename: 'source-alias/dist', - specifierType: 'esm', - parent: path.join(rootDir, 'foo.js'), - }); - assert.deepEqual(resolved, { - filePath: path.join(rootDir, 'packages', 'source-alias', 'source.js'), - sideEffects: undefined, - query: undefined, - invalidateOnFileCreate: [ - { - fileName: 'package.json', - aboveFilePath: path.join(rootDir, 'index'), - }, - { - fileName: 'package.json', - aboveFilePath: path.join(rootDir, 'foo.js'), - }, - { - fileName: 'node_modules/source-alias', - aboveFilePath: path.join(rootDir, 'foo.js'), - }, - ], - invalidateOnFileChange: [ - path.join(rootDir, 'package.json'), - path.join(rootDir, 'node_modules', 'source-alias', 'package.json'), - ], + sideEffects: undefined, + query: undefined, + invalidateOnFileCreate: [ + { + fileName: 'package.json', + aboveFilePath: path.join(rootDir, 'index'), + }, + { + fileName: 'package.json', + aboveFilePath: path.join(rootDir, 'foo.js'), + }, + { + fileName: 'node_modules/source-pnpm', + aboveFilePath: path.join(rootDir, 'foo.js'), + }, + ], + invalidateOnFileChange: [ + path.join(rootDir, 'package.json'), + path.join(rootDir, 'node_modules', 'source-pnpm', 'package.json'), + ], + }); }); }); - it('should use the source field as a glob alias when symlinked', async function () { - let resolved = await resolver.resolve({ - env: BROWSER_ENV, - filename: 'source-alias-glob', - specifierType: 'esm', - parent: path.join(rootDir, 'foo.js'), - }); - assert.deepEqual(resolved, { - filePath: path.join( - rootDir, - 'packages', - 'source-alias-glob', - 'src', - 'test.js', - ), - sideEffects: undefined, - query: undefined, - invalidateOnFileCreate: [ - { - fileName: 'package.json', - aboveFilePath: path.join(rootDir, 'index'), - }, - { - fileName: 'package.json', - aboveFilePath: path.join(rootDir, 'foo.js'), - }, - { - fileName: 'node_modules/source-alias-glob', - aboveFilePath: path.join(rootDir, 'foo.js'), - }, - ], - invalidateOnFileChange: [ - path.join(rootDir, 'package.json'), - path.join( + describe('package not behind symlinks', function () { + it('should not use the source field', async function () { + let resolved = await resolver.resolve({ + env: BROWSER_ENV, + filename: 'source-not-symlinked', + specifierType: 'esm', + parent: path.join(rootDir, 'foo.js'), + }); + assert.deepEqual(resolved, { + filePath: path.join( rootDir, 'node_modules', - 'source-alias-glob', - 'package.json', + 'source-not-symlinked', + 'dist.js', ), - ], + sideEffects: undefined, + query: undefined, + invalidateOnFileCreate: [ + { + fileName: 'package.json', + aboveFilePath: path.join(rootDir, 'index'), + }, + { + fileName: 'package.json', + aboveFilePath: path.join(rootDir, 'foo.js'), + }, + { + fileName: 'node_modules/source-not-symlinked', + aboveFilePath: path.join(rootDir, 'foo.js'), + }, + ], + invalidateOnFileChange: [ + path.join(rootDir, 'package.json'), + path.join( + rootDir, + 'node_modules', + 'source-not-symlinked', + 'package.json', + ), + ], + }); }); }); });