fix(node-resolve): handle "package.json" being in path (#927)

tjenkinson · shellscape · web-flow · commit 7d34204f06a9 · 2021-07-15T19:25:16.000-04:00
* fix(node-resolve): handle "package.json" being in path

* apply fix in a few more places too

* chore: fix linting

* chore(repo): use origin/main as a comparator for filtering tests

Co-authored-by: shellscape &lt;andrew@shellscape.org&gt;
diff --git a/.eslintignore b/.eslintignore
@@ -4,3 +4,6 @@
 **/test/**/output
 packages/commonjs/test/fixtures
 packages/typescript/test/fixtures/syntax-error
+
+# temporary workaround for eslint bug where package.json is a directory
+packages/node-resolve/test/fixtures/package-json-in-path
diff --git a/.github/workflows/node-windows.yml b/.github/workflows/node-windows.yml
@@ -27,6 +27,9 @@ jobs:
       - name: Checkout Commit
         uses: actions/checkout@v1
 
+      - name: Checkout Master
+        run: git branch -f master origin/master
+
       - name: Setup Node
         uses: actions/setup-node@v1
         with:
@@ -39,4 +42,4 @@ jobs:
         run: pnpm install --ignore-scripts
 
       - name: run tests
-        run: pnpm ci:test --filter ...[${{ github.sha }}]
+        run: pnpm ci:test --filter "...[origin/master]"
diff --git a/.github/workflows/validate.yml b/.github/workflows/validate.yml
@@ -30,6 +30,9 @@ jobs:
         with:
           node-version: ${{ matrix.node }}
 
+      - name: Checkout Master
+        run: git branch -f master origin/master
+
       - name: Install pnpm
         run: npm install pnpm -g
 
@@ -56,4 +59,4 @@ jobs:
         run: pnpm lint:js
 
       - name: Run Tests
-        run: pnpm ci:coverage --filter ...[${{ github.sha }}]
+        run: pnpm ci:coverage --filter "...[origin/master]"
diff --git a/packages/node-resolve/src/fs.js b/packages/node-resolve/src/fs.js
@@ -7,10 +7,11 @@ export const readFile = promisify(fs.readFile);
 export const realpath = promisify(fs.realpath);
 export { realpathSync } from 'fs';
 export const stat = promisify(fs.stat);
-export async function exists(filePath) {
+
+export async function fileExists(filePath) {
   try {
-    await access(filePath);
-    return true;
+    const res = await stat(filePath);
+    return res.isFile();
   } catch {
     return false;
   }
diff --git a/packages/node-resolve/src/index.js b/packages/node-resolve/src/index.js
@@ -6,7 +6,7 @@ import deepMerge from 'deepmerge';
 import isModule from 'is-module';
 
 import { isDirCached, isFileCached, readCachedFile } from './cache';
-import { exists, readFile, realpath } from './fs';
+import { fileExists, readFile, realpath } from './fs';
 import resolveImportSpecifiers from './resolveImportSpecifiers';
 import { getMainFields, getPackageName, normalizeInput } from './util';
 import handleDeprecatedOptions from './deprecated-options';
@@ -207,8 +207,8 @@ export function nodeResolve(opts = {}) {
     }
 
     if (hasPackageEntry && !preserveSymlinks) {
-      const fileExists = await exists(location);
-      if (fileExists) {
+      const exists = await fileExists(location);
+      if (exists) {
         location = await realpath(location);
       }
     }
@@ -228,7 +228,7 @@ export function nodeResolve(opts = {}) {
       }
     }
 
-    if (options.modulesOnly && (await exists(location))) {
+    if (options.modulesOnly && (await fileExists(location))) {
       const code = await readFile(location, 'utf-8');
       if (isModule(code)) {
         return {
diff --git a/packages/node-resolve/src/package/utils.js b/packages/node-resolve/src/package/utils.js
@@ -1,9 +1,8 @@
 /* eslint-disable no-await-in-loop */
 import path from 'path';
 import fs from 'fs';
-import { promisify } from 'util';
 
-const fileExists = promisify(fs.exists);
+import { fileExists } from '../fs';
 
 function isModuleDir(current, moduleDirs) {
   return moduleDirs.some((dir) => current.endsWith(dir));
diff --git a/packages/node-resolve/src/resolveImportSpecifiers.js b/packages/node-resolve/src/resolveImportSpecifiers.js
@@ -2,10 +2,12 @@ import fs from 'fs';
 import { promisify } from 'util';
 import { fileURLToPath, pathToFileURL } from 'url';
 
+import { dirname } from 'path';
+
 import resolve from 'resolve';
 
 import { getPackageInfo, getPackageName } from './util';
-import { exists, realpath } from './fs';
+import { fileExists, realpath } from './fs';
 import { isDirCached, isFileCached, readCachedFile } from './cache';
 import resolvePackageExports from './package/resolvePackageExports';
 import resolvePackageImports from './package/resolvePackageImports';
@@ -26,7 +28,7 @@ async function getPackageJson(importer, pkgName, resolveOptions, moduleDirectori
   try {
     const pkgJsonPath = await resolveImportPath(`${pkgName}/package.json`, resolveOptions);
     const pkgJson = JSON.parse(await readFile(pkgJsonPath, 'utf-8'));
-    return { pkgJsonPath, pkgJson };
+    return { pkgJsonPath, pkgJson, pkgPath: dirname(pkgJsonPath) };
   } catch (_) {
     return null;
   }
@@ -114,12 +116,11 @@ async function resolveId({
     const result = await getPackageJson(importer, pkgName, resolveOptions, moduleDirectories);
 
     if (result && result.pkgJson.exports) {
-      const { pkgJson, pkgJsonPath } = result;
+      const { pkgJson, pkgJsonPath, pkgPath } = result;
       try {
         const subpath =
           pkgName === importSpecifier ? '.' : `.${importSpecifier.substring(pkgName.length)}`;
-        const pkgDr = pkgJsonPath.replace('package.json', '');
-        const pkgURL = pathToFileURL(pkgDr);
+        const pkgURL = pathToFileURL(`${pkgPath}/`);
 
         const context = {
           importer,
@@ -157,7 +158,7 @@ async function resolveId({
   }
 
   if (!preserveSymlinks) {
-    if (await exists(location)) {
+    if (await fileExists(location)) {
       location = await realpath(location);
     }
   }
diff --git a/packages/node-resolve/test/fixtures/package-json-in-path/package.json/main.js b/packages/node-resolve/test/fixtures/package-json-in-path/package.json/main.js
@@ -0,0 +1,3 @@
+import { main } from "dependency/dist/something.js";
+
+export default main();
diff --git a/packages/node-resolve/test/fixtures/package-json-in-path/package.json/node_modules/dependency/dist-esm/something.js b/packages/node-resolve/test/fixtures/package-json-in-path/package.json/node_modules/dependency/dist-esm/something.js
diff --git a/packages/node-resolve/test/fixtures/package-json-in-path/package.json/node_modules/dependency/package.json b/packages/node-resolve/test/fixtures/package-json-in-path/package.json/node_modules/dependency/package.json
diff --git a/packages/node-resolve/test/test.js b/packages/node-resolve/test/test.js
@@ -124,6 +124,20 @@ test('supports JS extensions in TS when referring to TS imports', async (t) => {
   t.is(module.exports, 'It works!');
 });
 
+test('handles package.json being a directory earlier in the path', async (t) => {
+  const bundle = await rollup({
+    input: 'package-json-in-path/package.json/main.js',
+    onwarn: () => t.fail('No warnings were expected'),
+    plugins: [
+      nodeResolve({
+        extensions: ['.js']
+      })
+    ]
+  });
+  const { module } = await testBundle(t, bundle);
+  t.is(module.exports, 'It works!');
+});
+
 test('ignores IDs with null character', async (t) => {
   const result = await nodeResolve().resolveId('\0someid', 'test.js');
   t.is(result, null);
diff --git a/packages/typescript/test/fixtures/project-references/animals/dog.ts b/packages/typescript/test/fixtures/project-references/animals/dog.ts
@@ -1,6 +1,7 @@
+import Animal from '.';
+
 import { makeRandomName } from '../core/utilities';
 
-import Animal from '.';
 
 export interface Dog extends Animal {
   name: string;

Original file line number	Diff line number	Diff line change
`@@ -6,7 +6,7 @@ import deepMerge from 'deepmerge';`
`6`	`6`	`import isModule from 'is-module';`
`7`	`7`
`8`	`8`	`import { isDirCached, isFileCached, readCachedFile } from './cache';`
`9`		`-import { exists, readFile, realpath } from './fs';`
	`9`	`+import { fileExists, readFile, realpath } from './fs';`
`10`	`10`	`import resolveImportSpecifiers from './resolveImportSpecifiers';`
`11`	`11`	`import { getMainFields, getPackageName, normalizeInput } from './util';`
`12`	`12`	`import handleDeprecatedOptions from './deprecated-options';`
`@@ -207,8 +207,8 @@ export function nodeResolve(opts = {}) {`
`207`	`207`	`}`
`208`	`208`
`209`	`209`	`if (hasPackageEntry && !preserveSymlinks) {`
`210`		`- const fileExists = await exists(location);`
`211`		`- if (fileExists) {`
	`210`	`+ const exists = await fileExists(location);`
	`211`	`+ if (exists) {`
`212`	`212`	`location = await realpath(location);`
`213`	`213`	`}`
`214`	`214`	`}`
`@@ -228,7 +228,7 @@ export function nodeResolve(opts = {}) {`
`228`	`228`	`}`
`229`	`229`	`}`
`230`	`230`
`231`		`- if (options.modulesOnly && (await exists(location))) {`
	`231`	`+ if (options.modulesOnly && (await fileExists(location))) {`
`232`	`232`	`const code = await readFile(location, 'utf-8');`
`233`	`233`	`if (isModule(code)) {`
`234`	`234`	`return {`
Original file line number	Diff line number	Diff line change
`@@ -0,0 +1,3 @@`
	`1`	`+import { main } from "dependency/dist/something.js";`
	`2`	`+`
	`3`	`+export default main();`