From c82f444f68427b9496bedd19368b907896f72e8a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=EC=A1=B0=20=EC=9C=A0=EC=84=B1?= Date: Sun, 20 Mar 2022 19:12:09 +0900 Subject: [PATCH 1/6] check realpath inlcudes node_modules --- packages/utils/node-resolver-core/src/NodeResolver.js | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/packages/utils/node-resolver-core/src/NodeResolver.js b/packages/utils/node-resolver-core/src/NodeResolver.js index 0f400fd2fda..784d44c71a0 100644 --- a/packages/utils/node-resolver-core/src/NodeResolver.js +++ b/packages/utils/node-resolver-core/src/NodeResolver.js @@ -906,11 +906,14 @@ 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('/node_modules/')) { delete pkg.source; } } From b1cdbd2fda3fc0e320313daba52ebd730dd3ceec Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=EC=A1=B0=20=EC=9C=A0=EC=84=B1?= Date: Sun, 20 Mar 2022 19:12:17 +0900 Subject: [PATCH 2/6] add tests --- packages/utils/node-resolver-core/.gitignore | 1 + .../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 | 241 ++++++++---------- 5 files changed, 112 insertions(+), 135 deletions(-) create mode 100644 packages/utils/node-resolver-core/test/fixture/pnpm-store/node_modules/source-pnpm/dist.js create mode 100644 packages/utils/node-resolver-core/test/fixture/pnpm-store/node_modules/source-pnpm/package.json create mode 100644 packages/utils/node-resolver-core/test/fixture/pnpm-store/node_modules/source-pnpm/source.js diff --git a/packages/utils/node-resolver-core/.gitignore b/packages/utils/node-resolver-core/.gitignore index 33dfa7267da..d30023c8ed8 100644 --- a/packages/utils/node-resolver-core/.gitignore +++ b/packages/utils/node-resolver-core/.gitignore @@ -1 +1,2 @@ !/test/fixture/node_modules +!/test/fixture/pnpm-store/node_modules diff --git a/packages/utils/node-resolver-core/test/fixture/pnpm-store/node_modules/source-pnpm/dist.js b/packages/utils/node-resolver-core/test/fixture/pnpm-store/node_modules/source-pnpm/dist.js new file mode 100644 index 00000000000..e69de29bb2d diff --git a/packages/utils/node-resolver-core/test/fixture/pnpm-store/node_modules/source-pnpm/package.json b/packages/utils/node-resolver-core/test/fixture/pnpm-store/node_modules/source-pnpm/package.json new file mode 100644 index 00000000000..5829a9d15ee --- /dev/null +++ b/packages/utils/node-resolver-core/test/fixture/pnpm-store/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/pnpm-store/node_modules/source-pnpm/source.js b/packages/utils/node-resolver-core/test/fixture/pnpm-store/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..7b12290a81f 100644 --- a/packages/utils/node-resolver-core/test/resolver.js +++ b/packages/utils/node-resolver-core/test/resolver.js @@ -47,6 +47,10 @@ describe('resolver', function () { path.join(rootDir, 'packages/source'), path.join(rootDir, 'node_modules/source'), ); + await outputFS.symlink( + path.join(rootDir, 'pnpm-store/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 +2078,119 @@ 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, + 'pnpm-store', 'node_modules', - 'source-not-symlinked', - 'package.json', + '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', + ), + ], + }); }); }); }); From fc4b7e61b4cf1cc2de684bc019b2f45ee2a8a23f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=EC=A1=B0=20=EC=9C=A0=EC=84=B1?= Date: Sun, 20 Mar 2022 19:46:21 +0900 Subject: [PATCH 3/6] make test case more realistic --- packages/utils/node-resolver-core/.gitignore | 2 +- .../source-pnpm@1.0.0}/node_modules/source-pnpm/dist.js | 0 .../node_modules/source-pnpm/package.json | 0 .../node_modules/source-pnpm/source.js | 0 packages/utils/node-resolver-core/test/resolver.js | 9 +++++++-- 5 files changed, 8 insertions(+), 3 deletions(-) rename packages/utils/node-resolver-core/test/fixture/{pnpm-store => node_modules/.pnpm/source-pnpm@1.0.0}/node_modules/source-pnpm/dist.js (100%) rename packages/utils/node-resolver-core/test/fixture/{pnpm-store => node_modules/.pnpm/source-pnpm@1.0.0}/node_modules/source-pnpm/package.json (100%) rename packages/utils/node-resolver-core/test/fixture/{pnpm-store => node_modules/.pnpm/source-pnpm@1.0.0}/node_modules/source-pnpm/source.js (100%) diff --git a/packages/utils/node-resolver-core/.gitignore b/packages/utils/node-resolver-core/.gitignore index d30023c8ed8..7ed92ab6e96 100644 --- a/packages/utils/node-resolver-core/.gitignore +++ b/packages/utils/node-resolver-core/.gitignore @@ -1,2 +1,2 @@ !/test/fixture/node_modules -!/test/fixture/pnpm-store/node_modules +!/test/fixture/node_modules/.pnpm/*/node_modules diff --git a/packages/utils/node-resolver-core/test/fixture/pnpm-store/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 similarity index 100% rename from packages/utils/node-resolver-core/test/fixture/pnpm-store/node_modules/source-pnpm/dist.js rename to packages/utils/node-resolver-core/test/fixture/node_modules/.pnpm/source-pnpm@1.0.0/node_modules/source-pnpm/dist.js diff --git a/packages/utils/node-resolver-core/test/fixture/pnpm-store/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 similarity index 100% rename from packages/utils/node-resolver-core/test/fixture/pnpm-store/node_modules/source-pnpm/package.json rename to packages/utils/node-resolver-core/test/fixture/node_modules/.pnpm/source-pnpm@1.0.0/node_modules/source-pnpm/package.json diff --git a/packages/utils/node-resolver-core/test/fixture/pnpm-store/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 similarity index 100% rename from packages/utils/node-resolver-core/test/fixture/pnpm-store/node_modules/source-pnpm/source.js rename to 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/test/resolver.js b/packages/utils/node-resolver-core/test/resolver.js index 7b12290a81f..0f1c818429a 100644 --- a/packages/utils/node-resolver-core/test/resolver.js +++ b/packages/utils/node-resolver-core/test/resolver.js @@ -48,7 +48,10 @@ describe('resolver', function () { path.join(rootDir, 'node_modules/source'), ); await outputFS.symlink( - path.join(rootDir, 'pnpm-store/node_modules/source-pnpm'), + 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( @@ -2121,7 +2124,9 @@ describe('resolver', function () { assert.deepEqual(resolved, { filePath: path.join( rootDir, - 'pnpm-store', + 'node_modules', + '.pnpm', + 'source-pnpm@1.0.0', 'node_modules', 'source-pnpm', 'dist.js', From 9c9d379041b938b6a15cbaa9b6a7d577f45cfb89 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=EC=A1=B0=20=EC=9C=A0=EC=84=B1?= Date: Sun, 20 Mar 2022 20:51:28 +0900 Subject: [PATCH 4/6] improve cross-platform support --- packages/utils/node-resolver-core/src/NodeResolver.js | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/packages/utils/node-resolver-core/src/NodeResolver.js b/packages/utils/node-resolver-core/src/NodeResolver.js index 784d44c71a0..b56adcfb9b8 100644 --- a/packages/utils/node-resolver-core/src/NodeResolver.js +++ b/packages/utils/node-resolver-core/src/NodeResolver.js @@ -34,6 +34,7 @@ import _Module from 'module'; import {fileURLToPath} from 'url'; const EMPTY_SHIM = require.resolve('./_empty'); +const NODE_MODULES = `${path.sep}node_modules${path.sep}`; type InternalPackageJSON = PackageJSON & {pkgdir: string, pkgfile: string, ...}; type Options = {| @@ -913,7 +914,7 @@ export default class NodeResolver { // installed with package managers, rather than including a source code. if (pkg.source) { let realpath = await this.fs.realpath(file); - if (realpath === file || realpath.includes('/node_modules/')) { + if (realpath === file || realpath.includes(NODE_MODULES)) { delete pkg.source; } } From 255ba5bdf08a8c8d9d013cd8f24f9d658c10334a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=EC=A1=B0=20=EC=9C=A0=EC=84=B1?= Date: Sun, 20 Mar 2022 20:54:38 +0900 Subject: [PATCH 5/6] use normalizeSeparators() --- packages/utils/node-resolver-core/src/NodeResolver.js | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/packages/utils/node-resolver-core/src/NodeResolver.js b/packages/utils/node-resolver-core/src/NodeResolver.js index b56adcfb9b8..55ec70b3742 100644 --- a/packages/utils/node-resolver-core/src/NodeResolver.js +++ b/packages/utils/node-resolver-core/src/NodeResolver.js @@ -34,7 +34,6 @@ import _Module from 'module'; import {fileURLToPath} from 'url'; const EMPTY_SHIM = require.resolve('./_empty'); -const NODE_MODULES = `${path.sep}node_modules${path.sep}`; type InternalPackageJSON = PackageJSON & {pkgdir: string, pkgfile: string, ...}; type Options = {| @@ -914,7 +913,10 @@ export default class NodeResolver { // installed with package managers, rather than including a source code. if (pkg.source) { let realpath = await this.fs.realpath(file); - if (realpath === file || realpath.includes(NODE_MODULES)) { + if ( + realpath === file || + normalizeSeparators(realpath).includes('/node_modules/') + ) { delete pkg.source; } } From 8b25313eab53ccd00077f52c9454abb1f0ac2e5f Mon Sep 17 00:00:00 2001 From: Devon Govett Date: Sun, 20 Mar 2022 16:03:41 -0400 Subject: [PATCH 6/6] Update packages/utils/node-resolver-core/src/NodeResolver.js --- packages/utils/node-resolver-core/src/NodeResolver.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/utils/node-resolver-core/src/NodeResolver.js b/packages/utils/node-resolver-core/src/NodeResolver.js index 55ec70b3742..f7802facf3e 100644 --- a/packages/utils/node-resolver-core/src/NodeResolver.js +++ b/packages/utils/node-resolver-core/src/NodeResolver.js @@ -915,7 +915,7 @@ export default class NodeResolver { let realpath = await this.fs.realpath(file); if ( realpath === file || - normalizeSeparators(realpath).includes('/node_modules/') + realpath.includes(`${path.sep}node_modules${path.sep}`) ) { delete pkg.source; }