Skip to content

Commit

Permalink
fix: resolve cjs deps as cjs instead of esm (#362)
Browse files Browse the repository at this point in the history
When `nft` analyzes a file that has both a `require()` and an ESM
`export`, the first call to acorn fails because of the `export`, so it
treats the whole file as ESM, which is probably fine.

The problem is it treats the `require()` as an ESM import which is
wrong. This will cause the resolve dependency entrypoint in the
dependency to get the ESM export instead of the proper CJS export.

If the dependency is an `import`, add it to `imports`. If it's a
`require()`, add it to `deps` and don't worry about whatever `isESM`.

---------

Co-authored-by: Steven <steven@ceriously.com>
  • Loading branch information
Chris Barber and styfle committed Sep 18, 2023
1 parent 712fee2 commit 1eba4d5
Show file tree
Hide file tree
Showing 5 changed files with 28 additions and 3 deletions.
1 change: 1 addition & 0 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,7 @@
"@types/graceful-fs": "^4.1.5",
"@types/micromatch": "^4.0.1",
"@types/node": "^14.14.37",
"@vercel/edge-config": "^0.4.0",
"analytics-node": "^3.4.0-beta.1",
"apollo-server-express": "^2.14.2",
"argon2": "^0.31.1",
Expand Down
2 changes: 1 addition & 1 deletion src/node-file-trace.ts
Original file line number Diff line number Diff line change
Expand Up @@ -328,7 +328,7 @@ export class Job {
else
await this.emitFile(asset, 'asset', path);
}),
...[...deps].map(async dep => this.maybeEmitDep(dep, path, !isESM)),
...[...deps].map(async dep => this.maybeEmitDep(dep, path, !isESM || this.mixedModules)),
...[...imports].map(async dep => this.maybeEmitDep(dep, path, false)),
]);
}
Expand Down
7 changes: 5 additions & 2 deletions test/integration.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ const integrationDir = `${__dirname}${path.sep}integration`;
for (const integrationTest of readdirSync(integrationDir)) {
it(`should correctly trace and correctly execute ${integrationTest}`, async () => {
console.log('Tracing and executing ' + integrationTest);
const mixedModules = integrationTest === 'vercel-edge-config-esm.mjs';
const nftCache = {}
const fails = integrationTest.endsWith('failure.js');
const { fileList, reasons, warnings } = await nodeFileTrace([`${integrationDir}/${integrationTest}`], {
Expand All @@ -21,7 +22,8 @@ for (const integrationTest of readdirSync(integrationDir)) {
base: path.resolve(__dirname, '..'),
processCwd: integrationDir,
// ignore other integration tests
ignore: ['test/integration/**']
ignore: ['test/integration/**'],
mixedModules
});
// warnings.forEach(warning => console.warn(warning));
const randomTmpId = Math.random().toString().slice(2)
Expand Down Expand Up @@ -62,7 +64,8 @@ for (const integrationTest of readdirSync(integrationDir)) {
base: path.resolve(__dirname, '..'),
processCwd: integrationDir,
// ignore other integration tests
ignore: ['test/integration/**']
ignore: ['test/integration/**'],
mixedModules
});
expect([...cachedResult.fileList].sort()).toEqual([...fileList].sort())
}
Expand Down
9 changes: 9 additions & 0 deletions test/integration/vercel-edge-config-esm.mjs
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
import { createRequire } from 'node:module';

const require = createRequire(import.meta.url);

// this will load the CJS export and should not be treated as ESM
const ec = require('@vercel/edge-config');

// this will cause the file to be treated as ESM
export default function foo() {}
12 changes: 12 additions & 0 deletions yarn.lock
Original file line number Diff line number Diff line change
Expand Up @@ -2420,6 +2420,18 @@
dependencies:
"@types/node" "*"

"@vercel/edge-config-fs@0.1.0":
version "0.1.0"
resolved "https://registry.yarnpkg.com/@vercel/edge-config-fs/-/edge-config-fs-0.1.0.tgz#cda8327f611418c1d1cbb28c6bed02d782be928b"
integrity sha512-NRIBwfcS0bUoUbRWlNGetqjvLSwgYH/BqKqDN7vK1g32p7dN96k0712COgaz6VFizAm9b0g6IG6hR6+hc0KCPg==

"@vercel/edge-config@^0.4.0":
version "0.4.0"
resolved "https://registry.yarnpkg.com/@vercel/edge-config/-/edge-config-0.4.0.tgz#e3649125778d450e6702dfa3ed7ad3e93e16d2c4"
integrity sha512-v8AKiW/Tp9S79xapH3gXmOPXkkzJhUNnpyL6OX8xbHJvP7y5x0Oo/v/FUsGHpgay1x8a5vpddTH0Upw04RQcRA==
dependencies:
"@vercel/edge-config-fs" "0.1.0"

"@webcomponents/template@^1.4.0":
version "1.4.4"
resolved "https://registry.yarnpkg.com/@webcomponents/template/-/template-1.4.4.tgz#2ca5a68b65b6e9cd9b17b25a951514f254c00742"
Expand Down

0 comments on commit 1eba4d5

Please sign in to comment.