Fix #1229 and #1235: [BREAKING] always throw ERR_REQUIRE_ESM when att…

…empting to execute ESM as CJS, even when ESM loader is not loaded (#1232)

* always throw ERR_REQUIRE_ESM when attempting to execute ESM as CJS, even when ESM loader is not loaded

* cleanup

* Fix #1235

* fix bug

src/index.spec.ts

@@ -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:')
       })
@@ -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')
+      })
+    }
+
   })
 })

src/index.ts

@@ -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.
@@ -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
 

src/testlib.ts

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