Skip to content

Commit

Permalink
Fix #1488: circular dependency issue in dist-raw (#1489)
Browse files Browse the repository at this point in the history
* Fix #1488

* add regression test
  • Loading branch information
cspotcode committed Oct 10, 2021
1 parent 8a9ae84 commit b255b0e
Show file tree
Hide file tree
Showing 5 changed files with 50 additions and 1 deletion.
2 changes: 1 addition & 1 deletion dist-raw/node-package-json-reader.js
Expand Up @@ -5,6 +5,7 @@ const { SafeMap } = require('./node-primordials');
const { internalModuleReadJSON } = require('./node-internal-fs');
const { pathToFileURL } = require('url');
const { toNamespacedPath } = require('path');
// const { getOptionValue } = require('./node-options');

const cache = new SafeMap();

Expand All @@ -23,7 +24,6 @@ function read(jsonPath) {
toNamespacedPath(jsonPath)
);
const result = { string, containsKeys };
const { getOptionValue } = require('./node-options');
if (string !== undefined) {
if (manifest === undefined) {
// manifest = getOptionValue('--experimental-policy') ?
Expand Down
39 changes: 39 additions & 0 deletions src/test/regression.spec.ts
@@ -0,0 +1,39 @@
// Misc regression tests go here if they do not have a better home

import * as exp from 'expect';
import { join } from 'path';
import { createExec, createExecTester } from './exec-helpers';
import {
CMD_TS_NODE_WITHOUT_PROJECT_FLAG,
contextTsNodeUnderTest,
TEST_DIR,
} from './helpers';
import { test as _test } from './testlib';

const test = _test.context(contextTsNodeUnderTest);
const exec = createExecTester({
exec: createExec({
cwd: TEST_DIR,
}),
});

test('#1488 regression test', async () => {
// Scenario that caused the bug:
// `allowJs` turned on
// `skipIgnore` turned on so that ts-node tries to compile itself (not ideal but theoretically we should be ok with this)
// Attempt to `require()` a `.js` file
// `assertScriptCanLoadAsCJS` is triggered within `require()`
// `./package.json` needs to be fetched into cache via `assertScriptCanLoadAsCJS` which caused a recursive `require()` call
// Circular dependency warning is emitted by node

const { stdout, stderr } = await exec({
exec: createExec({
cwd: join(TEST_DIR, '1488'),
}),
cmd: `${CMD_TS_NODE_WITHOUT_PROJECT_FLAG} ./index.js`,
});

// Assert that we do *not* get `Warning: Accessing non-existent property 'getOptionValue' of module exports inside circular dependency`
exp(stdout).toBe('foo\n'); // prove that it ran
exp(stderr).toBe(''); // prove that no warnings
});
1 change: 1 addition & 0 deletions tests/1488/index.js
@@ -0,0 +1 @@
console.log('foo');
1 change: 1 addition & 0 deletions tests/1488/package.json
@@ -0,0 +1 @@
{}
8 changes: 8 additions & 0 deletions tests/1488/tsconfig.json
@@ -0,0 +1,8 @@
{
"compilerOptions": {
"allowJs": true
},
"ts-node": {
"skipIgnore": true
}
}

0 comments on commit b255b0e

Please sign in to comment.