Skip to content

Commit b255b0e

Browse files
authored
Fix #1488: circular dependency issue in dist-raw (#1489)
* Fix #1488 * add regression test
1 parent 8a9ae84 commit b255b0e

File tree

5 files changed

+50
-1
lines changed

5 files changed

+50
-1
lines changed

dist-raw/node-package-json-reader.js

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@ const { SafeMap } = require('./node-primordials');
55
const { internalModuleReadJSON } = require('./node-internal-fs');
66
const { pathToFileURL } = require('url');
77
const { toNamespacedPath } = require('path');
8+
// const { getOptionValue } = require('./node-options');
89

910
const cache = new SafeMap();
1011

@@ -23,7 +24,6 @@ function read(jsonPath) {
2324
toNamespacedPath(jsonPath)
2425
);
2526
const result = { string, containsKeys };
26-
const { getOptionValue } = require('./node-options');
2727
if (string !== undefined) {
2828
if (manifest === undefined) {
2929
// manifest = getOptionValue('--experimental-policy') ?

src/test/regression.spec.ts

Lines changed: 39 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,39 @@
1+
// Misc regression tests go here if they do not have a better home
2+
3+
import * as exp from 'expect';
4+
import { join } from 'path';
5+
import { createExec, createExecTester } from './exec-helpers';
6+
import {
7+
CMD_TS_NODE_WITHOUT_PROJECT_FLAG,
8+
contextTsNodeUnderTest,
9+
TEST_DIR,
10+
} from './helpers';
11+
import { test as _test } from './testlib';
12+
13+
const test = _test.context(contextTsNodeUnderTest);
14+
const exec = createExecTester({
15+
exec: createExec({
16+
cwd: TEST_DIR,
17+
}),
18+
});
19+
20+
test('#1488 regression test', async () => {
21+
// Scenario that caused the bug:
22+
// `allowJs` turned on
23+
// `skipIgnore` turned on so that ts-node tries to compile itself (not ideal but theoretically we should be ok with this)
24+
// Attempt to `require()` a `.js` file
25+
// `assertScriptCanLoadAsCJS` is triggered within `require()`
26+
// `./package.json` needs to be fetched into cache via `assertScriptCanLoadAsCJS` which caused a recursive `require()` call
27+
// Circular dependency warning is emitted by node
28+
29+
const { stdout, stderr } = await exec({
30+
exec: createExec({
31+
cwd: join(TEST_DIR, '1488'),
32+
}),
33+
cmd: `${CMD_TS_NODE_WITHOUT_PROJECT_FLAG} ./index.js`,
34+
});
35+
36+
// Assert that we do *not* get `Warning: Accessing non-existent property 'getOptionValue' of module exports inside circular dependency`
37+
exp(stdout).toBe('foo\n'); // prove that it ran
38+
exp(stderr).toBe(''); // prove that no warnings
39+
});

tests/1488/index.js

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
console.log('foo');

tests/1488/package.json

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
{}

tests/1488/tsconfig.json

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,8 @@
1+
{
2+
"compilerOptions": {
3+
"allowJs": true
4+
},
5+
"ts-node": {
6+
"skipIgnore": true
7+
}
8+
}

0 commit comments

Comments
 (0)