Resolve realpath in default resolver (#5508)

* Use inputFS.realpathSync get realpath of module

* Add test for symlink structure

* add symlink file

* dynamically create symlinks for test

* Update packages/utils/node-resolver-core/src/NodeResolver.js

Co-authored-by: Amin Yahyaabadi <[email protected]>

* Update NodeResolver.js

* Update packages/utils/node-resolver-core/src/NodeResolver.js

Co-authored-by: Amin Yahyaabadi <[email protected]>

* Update packages/utils/node-resolver-core/src/NodeResolver.js

Co-authored-by: Amin Yahyaabadi <[email protected]>

* use realpath

* Tests use overlayFS

* Update packages/utils/node-resolver-core/src/NodeResolver.js

Co-authored-by: Amin Yahyaabadi <[email protected]>

* Realpath at the end of resolution

* Remove realpaths that are now unnecessary

* Fix sourcemaps tests

* Add tests for symlink resolution

* Remove realpath comment

* prettier

Co-authored-by: dishuostec <[email protected]>
Co-authored-by: Niklas Mischkulnig <[email protected]>
Co-authored-by: Jasper De Moor <[email protected]>
Co-authored-by: Amin Yahyaabadi <[email protected]>
Co-authored-by: dishuostec <[email protected]>
Co-authored-by: Will Binns-Smith <[email protected]>

packages/core/core/src/requests/AssetRequest.js

@@ -56,8 +56,7 @@ function getId(input: AssetRequestInput) {
 }
 
 async function run({input, api, options, farm}: RunInput) {
-  let realpath = await options.inputFS.realpath(input.filePath);
-  api.invalidateOnFileUpdate(realpath);
+  api.invalidateOnFileUpdate(input.filePath);
   let start = Date.now();
   let {optionsRef, ...request} = input;
   let {cachePath} = nullthrows(
@@ -68,7 +67,9 @@ async function run({input, api, options, farm}: RunInput) {
   // These are used to compute the cache key for assets during transformation.
   request.invalidations = api.getInvalidations().filter(invalidation => {
     // Filter out invalidation node for the input file itself.
-    return invalidation.type !== 'file' || invalidation.filePath !== realpath;
+    return (
+      invalidation.type !== 'file' || invalidation.filePath !== input.filePath
+    );
   });
 
   let {assets, configRequests, invalidations} = (await farm.createHandle(

packages/core/core/src/summarizeRequest.js

@@ -13,18 +13,13 @@ export default async function summarizeRequest(
   fs: FileSystem,
   req: {|filePath: FilePath, code?: string|},
 ): Promise<{|content: Blob, hash: string, size: number, isSource: boolean|}> {
-  let [{content, hash, size}, isSource] = await Promise.all([
-    summarizeDiskRequest(fs, req),
-    isFilePathSource(fs, req.filePath),
-  ]);
+  let {content, hash, size} = await summarizeDiskRequest(fs, req);
+  let isSource = isFilePathSource(fs, req.filePath);
   return {content, hash, size, isSource};
 }
 
-async function isFilePathSource(fs: FileSystem, filePath: FilePath) {
-  return (
-    !filePath.includes(NODE_MODULES) ||
-    (await fs.realpath(filePath)) !== filePath
-  );
+function isFilePathSource(fs: FileSystem, filePath: FilePath) {
+  return !filePath.includes(NODE_MODULES);
 }
 
 async function summarizeDiskRequest(

packages/core/integration-tests/test/integration/resolve-symlinked-monorepos/packages/app/index.js

@@ -0,0 +1,5 @@
+import count from 'pkg';
+import init from 'library';
+
+init();
+export default count();

packages/core/integration-tests/test/integration/resolve-symlinked-monorepos/packages/app/package.json

@@ -0,0 +1,3 @@
+{
+  "private": true
+}

packages/core/integration-tests/test/integration/resolve-symlinked-monorepos/packages/library/package.json

@@ -0,0 +1,5 @@
+{
+  "name": "library",
+  "main": "src/index.js",
+  "private": true
+}

packages/core/integration-tests/test/integration/resolve-symlinked-monorepos/packages/library/src/index.js

@@ -0,0 +1,5 @@
+import count from 'pkg';
+
+export default function init() {
+  count();
+}

packages/core/integration-tests/test/integration/resolve-symlinked-monorepos/pnpm-workspace.yaml

@@ -0,0 +1,3 @@
+packages:
+  # all packages in subdirs of packages/
+  - 'packages/*'

packages/core/integration-tests/test/integration/resolve-symlinked-node_modules-structure/index.js

@@ -0,0 +1,3 @@
+import {answer} from 'library';
+
+export default answer;

packages/core/integration-tests/test/integration/resolve-symlinked-node_modules-structure/package.json

@@ -0,0 +1,3 @@
+{
+  "private": true
+}

packages/core/integration-tests/test/resolver.js

@@ -1,6 +1,6 @@
 import assert from 'assert';
 import path from 'path';
-import {bundle, run} from '@parcel/test-utils';
+import {bundle, run, ncp, overlayFS, outputFS} from '@parcel/test-utils';
 
 describe('resolver', function() {
   it('should support resolving tilde in monorepo packages', async function() {
@@ -256,4 +256,83 @@ describe('resolver', function() {
     let output = await run(b);
     assert.deepEqual(output.default, {});
   });
+
+  it('should support symlinked node_modules structure', async function() {
+    const rootDir = path.join(
+      __dirname,
+      'integration/resolve-symlinked-node_modules-structure',
+    );
+
+    await overlayFS.mkdirp(rootDir);
+    await ncp(rootDir, rootDir);
+
+    await outputFS.symlink(
+      path.join(
+        rootDir,
+        'node_modules/.origin/[email protected]/node_modules/library',
+      ),
+      path.join(rootDir, 'node_modules/library'),
+    );
+    await outputFS.symlink(
+      path.join(
+        rootDir,
+        'node_modules/.origin/[email protected]/node_modules/library-dep',
+      ),
+      path.join(
+        rootDir,
+        'node_modules/.origin/[email protected]/node_modules/library-dep',
+      ),
+    );
+
+    let b = await bundle(
+      path.join(
+        __dirname,
+        '/integration/resolve-symlinked-node_modules-structure/index.js',
+      ),
+      {
+        inputFS: overlayFS,
+        outputFS,
+      },
+    );
+
+    let output = await run(b);
+    assert.strictEqual(output.default, 42);
+  });
+
+  it('should support symlinked monorepos structure', async function() {
+    const rootDir = path.join(
+      __dirname,
+      'integration/resolve-symlinked-monorepos',
+    );
+
+    await overlayFS.mkdirp(rootDir);
+    await ncp(rootDir, rootDir);
+
+    await outputFS.symlink(
+      path.join(rootDir, 'packages/library'),
+      path.join(rootDir, 'packages/app/node_modules/library'),
+    );
+    await outputFS.symlink(
+      path.join(rootDir, 'node_modules/.origin/[email protected]/node_modules/pkg'),
+      path.join(rootDir, 'packages/app/node_modules/pkg'),
+    );
+    await outputFS.symlink(
+      path.join(rootDir, 'node_modules/.origin/[email protected]/node_modules/pkg'),
+      path.join(rootDir, 'packages/library/node_modules/pkg'),
+    );
+
+    let b = await bundle(
+      path.join(
+        __dirname,
+        '/integration/resolve-symlinked-monorepos/packages/app/index.js',
+      ),
+      {
+        inputFS: overlayFS,
+        outputFS,
+      },
+    );
+
+    let output = await run(b);
+    assert.strictEqual(output.default, 2);
+  });
 });

packages/core/integration-tests/test/sourcemaps.js

@@ -462,7 +462,7 @@ describe('sourcemaps', function() {
     assert.equal(mapData.sources.length, 2);
     assert.deepEqual(mapData.sources, [
       './index.ts',
-      '../../../../../../node_modules/@parcel/transformer-js/src/esmodule-helpers.js',
+      '../../../../../transformers/js/src/esmodule-helpers.js',
     ]);
 
     let input = await inputFS.readFile(
@@ -505,7 +505,7 @@ describe('sourcemaps', function() {
     assert.deepEqual(mapData.sources, [
       './index.ts',
       './local.ts',
-      '../../../../../../node_modules/@parcel/transformer-js/src/esmodule-helpers.js',
+      '../../../../../transformers/js/src/esmodule-helpers.js',
     ]);
 
     let input = await inputFS.readFile(
@@ -959,7 +959,7 @@ describe('sourcemaps', function() {
     assert.equal(map.file, 'index.js.map');
     assert.deepEqual(map.sources, [
       './index.js',
-      '../../../../../../node_modules/@parcel/transformer-js/src/esmodule-helpers.js',
+      '../../../../../transformers/js/src/esmodule-helpers.js',
     ]);
     assert.equal(map.sourcesContent[0], sourceContent);
   });
@@ -993,7 +993,7 @@ describe('sourcemaps', function() {
     assert.equal(map.file, 'index.js.map');
     assert.deepEqual(map.sources, [
       './index.js',
-      '../../../../../../node_modules/@parcel/transformer-js/src/esmodule-helpers.js',
+      '../../../../../transformers/js/src/esmodule-helpers.js',
     ]);
   });
 

packages/core/utils/src/config.js

@@ -21,7 +21,6 @@ export function resolveConfig(
   filepath: FilePath,
   filenames: Array<FilePath>,
 ): Promise<?FilePath> {
-  // TODO: realpath
   return Promise.resolve(
     fs.findAncestorFile(filenames, path.dirname(filepath)),
   );
@@ -32,7 +31,6 @@ export function resolveConfigSync(
   filepath: FilePath,
   filenames: Array<FilePath>,
 ): ?FilePath {
-  // TODO: realpath
   return fs.findAncestorFile(filenames, path.dirname(filepath));
 }
 

packages/utils/node-resolver-core/src/NodeResolver.js

@@ -136,7 +136,7 @@ export default class NodeResolver {
       } else if (module.filePath) {
         if (module.code != null) {
           return {
-            filePath: module.filePath,
+            filePath: await this.fs.realpath(module.filePath),
             code: module.code,
           };
         }
@@ -151,7 +151,7 @@ export default class NodeResolver {
 
       if (resolved) {
         return {
-          filePath: resolved.path,
+          filePath: await this.fs.realpath(resolved.path),
           sideEffects:
             resolved.pkg && !this.hasSideEffects(resolved.path, resolved.pkg)
               ? false

packages/utils/node-resolver-core/test/resolver.js

@@ -52,6 +52,14 @@ describe('resolver', function() {
       path.join(rootDir, 'packages/source-alias-glob'),
       path.join(rootDir, 'node_modules/source-alias-glob'),
     );
+    await outputFS.symlink(
+      path.join(rootDir, 'bar.js'),
+      path.join(rootDir, 'baz.js'),
+    );
+    await outputFS.symlink(
+      path.join(rootDir, 'nested'),
+      path.join(rootDir, 'symlinked-nested'),
+    );
 
     resolver = new NodeResolver({
       fs: overlayFS,
@@ -693,7 +701,7 @@ describe('resolver', function() {
         parent: path.join(rootDir, 'foo.js'),
       });
       assert.deepEqual(resolved, {
-        filePath: path.join(rootDir, 'node_modules', 'source', 'source.js'),
+        filePath: path.join(rootDir, 'packages', 'source', 'source.js'),
         sideEffects: undefined,
       });
     });
@@ -724,12 +732,7 @@ describe('resolver', function() {
         parent: path.join(rootDir, 'foo.js'),
       });
       assert.deepEqual(resolved, {
-        filePath: path.join(
-          rootDir,
-          'node_modules',
-          'source-alias',
-          'source.js',
-        ),
+        filePath: path.join(rootDir, 'packages', 'source-alias', 'source.js'),
         sideEffects: undefined,
       });
     });
@@ -744,7 +747,7 @@ describe('resolver', function() {
       assert.deepEqual(resolved, {
         filePath: path.join(
           rootDir,
-          'node_modules',
+          'packages',
           'source-alias-glob',
           'src',
           'test.js',
@@ -754,6 +757,31 @@ describe('resolver', function() {
     });
   });
 
+  describe('symlinks', function() {
+    it('should resolve symlinked files to their realpath', async function() {
+      let resolved = await resolver.resolve({
+        env: BROWSER_ENV,
+        filename: './baz.js',
+        isURL: false,
+        parent: path.join(rootDir, 'foo.js'),
+      });
+      assert.equal(nullthrows(resolved).filePath, path.join(rootDir, 'bar.js'));
+    });
+
+    it('should resolve symlinked directories to their realpath', async function() {
+      let resolved = await resolver.resolve({
+        env: BROWSER_ENV,
+        filename: './symlinked-nested',
+        isURL: false,
+        parent: path.join(rootDir, 'foo.js'),
+      });
+      assert.equal(
+        nullthrows(resolved).filePath,
+        path.join(rootDir, 'nested', 'index.js'),
+      );
+    });
+  });
+
   describe('error handling', function() {
     it('should return diagnostics when package.module does not exist', async function() {
       let result = await resolver.resolve({