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

* 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 <andrew@shellscape.org>

.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

.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]"

.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]"

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;
   }

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 {

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));

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);
     }
   }

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();

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);

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;