Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix #1229 and #1235: [BREAKING] always throw ERR_REQUIRE_ESM when attempting to execute ESM as CJS, even when ESM loader is not loaded #1232

Merged
merged 5 commits into from
Feb 22, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
25 changes: 18 additions & 7 deletions src/index.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -835,8 +835,8 @@ test.suite('ts-node', (test) => {
})
})

test('throws ERR_REQUIRE_ESM when attempting to require() an ESM script while ESM loader is enabled', async () => {
const { err, stdout, stderr } = await exec(`${cmd} ./index.js`, { cwd: join(TEST_DIR, './esm-err-require-esm') })
test('throws ERR_REQUIRE_ESM when attempting to require() an ESM script when ESM loader is enabled', async () => {
const { err, stderr } = await exec(`${cmd} ./index.js`, { cwd: join(TEST_DIR, './esm-err-require-esm') })
expect(err).to.not.equal(null)
expect(stderr).to.contain('Error [ERR_REQUIRE_ESM]: Must use import to load ES Module:')
})
Expand Down Expand Up @@ -874,10 +874,21 @@ test.suite('ts-node', (test) => {
})
}

test('executes ESM as CJS, ignoring package.json "types" field (for backwards compatibility; should be changed in next major release to throw ERR_REQUIRE_ESM)', async () => {
const { err, stdout } = await exec(`${BIN_PATH} ./esm-err-require-esm/index.js`)
expect(err).to.equal(null)
expect(stdout).to.equal('CommonJS\n')
})
if (semver.gte(process.version, '12.0.0')) {
test('throws ERR_REQUIRE_ESM when attempting to require() an ESM script when ESM loader is *not* enabled and node version is >= 12', async () => {
// Node versions >= 12 support package.json "type" field and so will throw an error when attempting to load ESM as CJS
const { err, stderr } = await exec(`${BIN_PATH} ./index.js`, { cwd: join(TEST_DIR, './esm-err-require-esm') })
expect(err).to.not.equal(null)
expect(stderr).to.contain('Error [ERR_REQUIRE_ESM]: Must use import to load ES Module:')
})
} else {
test('Loads as CommonJS when attempting to require() an ESM script when ESM loader is *not* enabled and node version is < 12', async () => {
// Node versions less than 12 do not support package.json "type" field and so will load ESM as CommonJS
const { err, stdout } = await exec(`${BIN_PATH} ./index.js`, { cwd: join(TEST_DIR, './esm-err-require-esm') })
expect(err).to.equal(null)
expect(stdout).to.contain('CommonJS')
})
}

})
})
18 changes: 7 additions & 11 deletions src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -19,18 +19,16 @@ export { createRepl, CreateReplOptions, ReplService } from './repl'
*/
const engineSupportsPackageTypeField = parseInt(process.versions.node.split('.')[0], 10) >= 12

// Loaded conditionally so we don't need to support older node versions
let assertScriptCanLoadAsCJSImpl: ((filename: string) => void) | undefined

/**
* Assert that script can be loaded as CommonJS when we attempt to require it.
* If it should be loaded as ESM, throw ERR_REQUIRE_ESM like node does.
*
* Loaded conditionally so we don't need to support older node versions
*/
function assertScriptCanLoadAsCJS (filename: string) {
if (!engineSupportsPackageTypeField) return
if (!assertScriptCanLoadAsCJSImpl) assertScriptCanLoadAsCJSImpl = require('../dist-raw/node-cjs-loader-utils').assertScriptCanLoadAsCJSImpl
assertScriptCanLoadAsCJSImpl!(filename)
}
const assertScriptCanLoadAsCJS: (filename: string) => void =
engineSupportsPackageTypeField
? require('../dist-raw/node-cjs-loader-utils').assertScriptCanLoadAsCJSImpl
: () => {/* noop */}

/**
* Registered `ts-node` instance information.
Expand Down Expand Up @@ -1115,9 +1113,7 @@ function registerExtension (
require.extensions[ext] = function (m: any, filename) { // tslint:disable-line
if (service.ignored(filename)) return old(m, filename)

if (service.options.experimentalEsmLoader) {
assertScriptCanLoadAsCJS(filename)
}
assertScriptCanLoadAsCJS(filename)

const _compile = m._compile

Expand Down
11 changes: 6 additions & 5 deletions src/testlib.ts
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ export interface TestInterface<Context> /*extends Omit<AvaTestInterface<Context>
/** Declare a concurrent test that uses one or more macros. The macro is responsible for generating a unique test title. */
serial<T extends any[]> (macros: OneOrMoreMacros<T, Context>, ...rest: T): void

macro<Args extends any[]> (cb: (...args: Args) => [(title: string | undefined) => string, (t: TestInterface<Context>) => Promise<void>]): (test: TestInterface<Context>, ...args: Args) => Promise<void> & {
macro<Args extends any[]> (cb: (...args: Args) => ([(title: string | undefined) => string, (t: ExecutionContext<Context>) => Promise<void>] | ((t: ExecutionContext<Context>) => Promise<void>))): (test: ExecutionContext<Context>, ...args: Args) => Promise<void> & {
title (givenTitle: string | undefined, ...args: Args): string;
}

Expand Down Expand Up @@ -139,14 +139,15 @@ function createTestInterface<Context> (opts: {
mustDoSerial = true
beforeEachFunctions.push(once(cb))
}
test.macro = function<Args extends any[]>(cb: (...args: Args) => [(title: string | undefined) => string, (t: ExecutionContext<Context>) => Promise<void>]) {
test.macro = function<Args extends any[]>(cb: (...args: Args) => [(title: string | undefined) => string, (t: ExecutionContext<Context>) => Promise<void>] | ((t: ExecutionContext<Context>) => Promise<void>)) {
function macro (testInterface: ExecutionContext<Context>, ...args: Args) {
const [, macroFunction] = cb(...args)
const ret = cb(...args)
const macroFunction = Array.isArray(ret) ? ret[1] : ret
return macroFunction(testInterface)
}
macro.title = function (givenTitle: string | undefined, ...args: Args) {
const [macroTitleFunction ] = cb(...args)
return macroTitleFunction(givenTitle)
const ret = cb(...args)
return Array.isArray(ret) ? ret[0](givenTitle) : givenTitle
}
return macro
}
Expand Down