From 2bee85ebfcff47d1307f04b6cccf14d548618e6e Mon Sep 17 00:00:00 2001 From: Andrew Bradley Date: Sun, 3 May 2020 02:48:30 -0400 Subject: [PATCH 01/14] revert addition of package.json "exports" because it is a breaking change and requires more thought --- package.json | 4 ---- 1 file changed, 4 deletions(-) diff --git a/package.json b/package.json index abfc19510..7e0e42061 100644 --- a/package.json +++ b/package.json @@ -3,10 +3,6 @@ "version": "8.10.0", "description": "TypeScript execution environment and REPL for node.js, with source map support", "main": "dist/index.js", - "exports": { - ".": "./dist/index.js", - "./esm": "./esm.mjs" - }, "types": "dist/index.d.ts", "bin": { "ts-node": "dist/bin.js", From c2ab1c3f1753cf6fc8d4de36196c395bd4977e7b Mon Sep 17 00:00:00 2001 From: Andrew Bradley Date: Sat, 9 May 2020 02:32:06 -0400 Subject: [PATCH 02/14] Implement ERR_REQUIRE_ESM error when attempting to require() an ESM file --- dist-raw/node-cjs-loader-utils.js | 106 ++++++++++++++++++++++++++++++ src/index.ts | 18 +++++ 2 files changed, 124 insertions(+) create mode 100644 dist-raw/node-cjs-loader-utils.js diff --git a/dist-raw/node-cjs-loader-utils.js b/dist-raw/node-cjs-loader-utils.js new file mode 100644 index 000000000..d95b5074e --- /dev/null +++ b/dist-raw/node-cjs-loader-utils.js @@ -0,0 +1,106 @@ +// copied from https://github.com/nodejs/node/blob/master/lib/internal/modules/cjs/loader.js + +module.exports.assertScriptIsNotEsm = assertScriptIsNotEsm; + +const packageJsonCache = new Map(); + +function readPackageScope(checkPath) { + const rootSeparatorIndex = checkPath.indexOf(path.sep); + let separatorIndex; + while ( + (separatorIndex = checkPath.lastIndexOf(path.sep)) > rootSeparatorIndex + ) { + checkPath = checkPath.slice(0, separatorIndex); + if (checkPath.endsWith(path.sep + 'node_modules')) + return false; + const pjson = readPackage(checkPath); + if (pjson) return { + path: checkPath, + data: pjson + }; + } + return false; +} + +function readPackage(requestPath) { + const jsonPath = path.resolve(requestPath, 'package.json'); + + const existing = packageJsonCache.get(jsonPath); + if (existing !== undefined) return existing; + + const json = internalModuleReadJSON(path.toNamespacedPath(jsonPath)); + if (json === undefined) { + packageJsonCache.set(jsonPath, false); + return false; + } + + if (manifest) { + const jsonURL = pathToFileURL(jsonPath); + manifest.assertIntegrity(jsonURL, json); + } + + try { + const parsed = JSONParse(json); + const filtered = { + name: parsed.name, + main: parsed.main, + exports: parsed.exports, + type: parsed.type + }; + packageJsonCache.set(jsonPath, filtered); + return filtered; + } catch (e) { + e.path = jsonPath; + e.message = 'Error parsing ' + jsonPath + ': ' + e.message; + throw e; + } +} + +// copied from Module._extensions['.js'] +function assertScriptIsNotEsm(filename) { + const pkg = readPackageScope(filename); + // Function require shouldn't be used in ES modules. + if (pkg && pkg.data && pkg.data.type === 'module') { + const parentPath = module.parent && module.parent.filename; + const packageJsonPath = path.resolve(pkg.path, 'package.json'); + throw createErrRequireEsm(filename, parentPath, packageJsonPath); + } +} + +function createErrRequireEsm(filename, parentPath, packageJsonPath) { + // Attempt to create an error object that matches node's native error close enough + const code = 'ERR_REQUIRE_ESM' + const err = new Error(getMessage(filename, parentPath, packageJsonPath)) + err.name = `Error [${ code }]` + err.stack + Object.defineProperty(err, 'name', { + value: 'Error', + enumerable: false, + writable: true, + configurable: true + }) + err.code = code + return err + + // copy-pasted from https://github.com/nodejs/node/blob/master/lib/internal/errors.js#L1294-L1311 + function getMessage(filename, parentPath = null, packageJsonPath = null) { + const ext = path.extname(filename) + let msg = `Must use import to load ES Module: ${filename}`; + if (parentPath && packageJsonPath) { + const path = require('path'); + const basename = path.basename(filename) === path.basename(parentPath) ? + filename : path.basename(filename); + msg += + '\nrequire() of ES modules is not supported.\nrequire() of ' + + `${filename} ${parentPath ? `from ${parentPath} ` : ''}` + + `is an ES module file as it is a ${ext} file whose nearest parent ` + + `package.json contains "type": "module" which defines all ${ext} ` + + 'files in that package scope as ES modules.\nInstead ' + + 'change the requiring code to use ' + + 'import(), or remove "type": "module" from ' + + `${packageJsonPath}.\n`; + return msg; + } + return msg; + } +} diff --git a/src/index.ts b/src/index.ts index e37442a90..6f75b1ae5 100644 --- a/src/index.ts +++ b/src/index.ts @@ -5,6 +5,22 @@ import { BaseError } from 'make-error' import * as util from 'util' import * as _ts from 'typescript' +// Feature-detect native ECMAScript modules support +let engineSupportsNativeModules = true +try { + new Function('import("")') +} catch (e) { + engineSupportsNativeModules = false +} + +/** + * Assert that script should not be loaded as ESM, when we attempt to require it. + * If it should, throw the same error as node. + * + * Loaded conditionally so we don't need to support older node versions + */ +let assertScriptIsNotEsm = engineSupportsNativeModules ? require('../dist-raw/node-cjs-loader-utils').assertScriptIsNotEsm : () => {} + /** * Registered `ts-node` instance information. */ @@ -850,6 +866,8 @@ function registerExtension ( require.extensions[ext] = function (m: any, filename) { // tslint:disable-line if (register.ignored(filename)) return old(m, filename) + assertScriptIsNotEsm(filename) + const _compile = m._compile m._compile = function (code: string, fileName: string) { From d82df95111d3b97fb3cd66db8679e5db18af2fc9 Mon Sep 17 00:00:00 2001 From: Andrew Bradley Date: Sat, 9 May 2020 03:40:54 -0400 Subject: [PATCH 03/14] fix --- dist-raw/node-cjs-loader-utils.js | 22 +++++++++++++++++----- src/index.ts | 4 ++-- 2 files changed, 19 insertions(+), 7 deletions(-) diff --git a/dist-raw/node-cjs-loader-utils.js b/dist-raw/node-cjs-loader-utils.js index d95b5074e..460d67ccd 100644 --- a/dist-raw/node-cjs-loader-utils.js +++ b/dist-raw/node-cjs-loader-utils.js @@ -1,4 +1,6 @@ // copied from https://github.com/nodejs/node/blob/master/lib/internal/modules/cjs/loader.js +const path = require('path'); +const fs = require('fs'); module.exports.assertScriptIsNotEsm = assertScriptIsNotEsm; @@ -34,13 +36,14 @@ function readPackage(requestPath) { return false; } - if (manifest) { - const jsonURL = pathToFileURL(jsonPath); - manifest.assertIntegrity(jsonURL, json); - } + // TODO Related to `--experimental-policy`? Disabling for now + // if (manifest) { + // const jsonURL = pathToFileURL(jsonPath); + // manifest.assertIntegrity(jsonURL, json); + // } try { - const parsed = JSONParse(json); + const parsed = JSON.parse(json); const filtered = { name: parsed.name, main: parsed.main, @@ -56,6 +59,15 @@ function readPackage(requestPath) { } } +function internalModuleReadJSON(path) { + try { + return fs.readFileSync(path, 'utf8') + } catch (e) { + if (e.code === 'ENOENT') return undefined + throw e + } +} + // copied from Module._extensions['.js'] function assertScriptIsNotEsm(filename) { const pkg = readPackageScope(filename); diff --git a/src/index.ts b/src/index.ts index 6f75b1ae5..00bd3507f 100644 --- a/src/index.ts +++ b/src/index.ts @@ -8,7 +8,7 @@ import * as _ts from 'typescript' // Feature-detect native ECMAScript modules support let engineSupportsNativeModules = true try { - new Function('import("")') + new Function('import("")') // tslint:disable-line } catch (e) { engineSupportsNativeModules = false } @@ -19,7 +19,7 @@ try { * * Loaded conditionally so we don't need to support older node versions */ -let assertScriptIsNotEsm = engineSupportsNativeModules ? require('../dist-raw/node-cjs-loader-utils').assertScriptIsNotEsm : () => {} +let assertScriptIsNotEsm = engineSupportsNativeModules ? require('../dist-raw/node-cjs-loader-utils').assertScriptIsNotEsm : () => { } // tslint:disable-line /** * Registered `ts-node` instance information. From a10c7a9173d786e24eabdbeb9f18a9c999186422 Mon Sep 17 00:00:00 2001 From: Andrew Bradley Date: Sat, 9 May 2020 18:03:22 -0400 Subject: [PATCH 04/14] test on node 12.15 and 12.16 due to weird ESM handling --- .github/workflows/continuous-integration.yml | 18 ++++++++++++------ 1 file changed, 12 insertions(+), 6 deletions(-) diff --git a/.github/workflows/continuous-integration.yml b/.github/workflows/continuous-integration.yml index 3895aeee2..a98d3aa68 100644 --- a/.github/workflows/continuous-integration.yml +++ b/.github/workflows/continuous-integration.yml @@ -7,27 +7,33 @@ jobs: strategy: fail-fast: false matrix: - flavor: [1, 2, 3, 4, 5, 6, 7] + flavor: [1, 2, 3, 4, 5, 6, 7, 8, 9] include: - flavor: 1 node: 6 typescript: typescript@latest - flavor: 2 - node: 13 + node: 12.15 typescript: typescript@latest - flavor: 3 + node: 12.16 + typescript: typescript@latest + - flavor: 4 + node: 13 + typescript: typescript@latest + - flavor: 5 node: 13 typescript: typescript@2.7 - - flavor: 4 + - flavor: 6 node: 13 typescript: typescript@next - - flavor: 5 + - flavor: 7 node: 14 typescript: typescript@latest - - flavor: 6 + - flavor: 8 node: 14 typescript: typescript@2.7 - - flavor: 7 + - flavor: 9 node: 14 typescript: typescript@next steps: From 724312305cccd86e7cecd0cab6c404e5b7fcac6a Mon Sep 17 00:00:00 2001 From: Andrew Bradley Date: Sat, 9 May 2020 18:04:54 -0400 Subject: [PATCH 05/14] Switch package.json "type" feature detection to a simple version number check --- src/index.ts | 16 +++++++--------- 1 file changed, 7 insertions(+), 9 deletions(-) diff --git a/src/index.ts b/src/index.ts index 00bd3507f..598726e3f 100644 --- a/src/index.ts +++ b/src/index.ts @@ -5,21 +5,19 @@ import { BaseError } from 'make-error' import * as util from 'util' import * as _ts from 'typescript' -// Feature-detect native ECMAScript modules support -let engineSupportsNativeModules = true -try { - new Function('import("")') // tslint:disable-line -} catch (e) { - engineSupportsNativeModules = false -} +/** + * Does this version of node obey the package.json "type" field + * and attempt to load scripts as ESM + */ +let engineSupportsPackageTypeField = parseInt(process.versions.node.split('.')[0]) >= 12 /** * Assert that script should not be loaded as ESM, when we attempt to require it. - * If it should, throw the same error as node. + * If it should be ESM, throw ERR_REQUIRE_ESM error like node does. * * Loaded conditionally so we don't need to support older node versions */ -let assertScriptIsNotEsm = engineSupportsNativeModules ? require('../dist-raw/node-cjs-loader-utils').assertScriptIsNotEsm : () => { } // tslint:disable-line +let assertScriptIsNotEsm = engineSupportsPackageTypeField ? require('../dist-raw/node-cjs-loader-utils').assertScriptIsNotEsm : () => { } // tslint:disable-line /** * Registered `ts-node` instance information. From eb07ff6a335e26e7ac64ab13d690ef1d08c9d0df Mon Sep 17 00:00:00 2001 From: Andrew Bradley Date: Sat, 9 May 2020 18:35:19 -0400 Subject: [PATCH 06/14] Remove dead code --- src/index.ts | 3 --- 1 file changed, 3 deletions(-) diff --git a/src/index.ts b/src/index.ts index 598726e3f..445237198 100644 --- a/src/index.ts +++ b/src/index.ts @@ -780,9 +780,6 @@ export function create (rawOptions: CreateOptions = {}): Register { } } - const cannotCompileViaBothCodepathsErrorMessage = 'Cannot compile the same file via both `require()` and ESM hooks codepaths. ' + - 'This breaks source-map-support, which cannot tell the difference between the two sourcemaps. ' + - 'To avoid this problem, load each .ts file as only ESM or only CommonJS.' // Create a simple TypeScript compiler proxy. function compile (code: string, fileName: string, lineOffset = 0) { const normalizedFileName = normalizeSlashes(fileName) From 567acb64cb7c45b210f2963665b87ad8f55ced8f Mon Sep 17 00:00:00 2001 From: Andrew Bradley Date: Sat, 9 May 2020 18:36:49 -0400 Subject: [PATCH 07/14] Only throw ERR_REQUIRE_ESM if experimental ESM loader is enabled, since it is a breaking change --- dist-raw/node-cjs-loader-utils.js | 26 +++++++++++++------------- src/esm.ts | 5 ++++- src/index.ts | 28 +++++++++++++++++++++------- 3 files changed, 38 insertions(+), 21 deletions(-) diff --git a/dist-raw/node-cjs-loader-utils.js b/dist-raw/node-cjs-loader-utils.js index 460d67ccd..8512bba3f 100644 --- a/dist-raw/node-cjs-loader-utils.js +++ b/dist-raw/node-cjs-loader-utils.js @@ -2,9 +2,18 @@ const path = require('path'); const fs = require('fs'); -module.exports.assertScriptIsNotEsm = assertScriptIsNotEsm; +module.exports.assertScriptCanLoadAsCJSImpl = assertScriptCanLoadAsCJSImpl; -const packageJsonCache = new Map(); +// copied from Module._extensions['.js'] +function assertScriptCanLoadAsCJSImpl(filename) { + const pkg = readPackageScope(filename); + // Function require shouldn't be used in ES modules. + if (pkg && pkg.data && pkg.data.type === 'module') { + const parentPath = module.parent && module.parent.filename; + const packageJsonPath = path.resolve(pkg.path, 'package.json'); + throw createErrRequireEsm(filename, parentPath, packageJsonPath); + } +} function readPackageScope(checkPath) { const rootSeparatorIndex = checkPath.indexOf(path.sep); @@ -24,6 +33,8 @@ function readPackageScope(checkPath) { return false; } +const packageJsonCache = new Map(); + function readPackage(requestPath) { const jsonPath = path.resolve(requestPath, 'package.json'); @@ -68,17 +79,6 @@ function internalModuleReadJSON(path) { } } -// copied from Module._extensions['.js'] -function assertScriptIsNotEsm(filename) { - const pkg = readPackageScope(filename); - // Function require shouldn't be used in ES modules. - if (pkg && pkg.data && pkg.data.type === 'module') { - const parentPath = module.parent && module.parent.filename; - const packageJsonPath = path.resolve(pkg.path, 'package.json'); - throw createErrRequireEsm(filename, parentPath, packageJsonPath); - } -} - function createErrRequireEsm(filename, parentPath, packageJsonPath) { // Attempt to create an error object that matches node's native error close enough const code = 'ERR_REQUIRE_ESM' diff --git a/src/esm.ts b/src/esm.ts index ed1e6003c..9d49a0017 100644 --- a/src/esm.ts +++ b/src/esm.ts @@ -8,7 +8,10 @@ const { createResolve } = require('../dist-raw/node-esm-resolve-implementation') export function registerAndCreateEsmHooks (opts?: RegisterOptions) { // Automatically performs registration just like `-r ts-node/register` - const tsNodeInstance = register(opts) + const tsNodeInstance = register({ + ...opts, + experimentalEsmLoader: true + }) // Custom implementation that considers additional file extensions and automatically adds file extensions const nodeResolveImplementation = createResolve({ diff --git a/src/index.ts b/src/index.ts index 445237198..b6dd9f290 100644 --- a/src/index.ts +++ b/src/index.ts @@ -11,13 +11,18 @@ import * as _ts from 'typescript' */ let engineSupportsPackageTypeField = parseInt(process.versions.node.split('.')[0]) >= 12 +// Loaded conditionally so we don't need to support older node versions +let assertScriptCanLoadAsCJSImpl: ((filename: string) => void) | undefined + /** - * Assert that script should not be loaded as ESM, when we attempt to require it. - * If it should be ESM, throw ERR_REQUIRE_ESM error like node does. - * - * Loaded conditionally so we don't need to support older node versions + * 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. */ -let assertScriptIsNotEsm = engineSupportsPackageTypeField ? require('../dist-raw/node-cjs-loader-utils').assertScriptIsNotEsm : () => { } // tslint:disable-line +function assertScriptCanLoadAsCJS(filename: string) { + if(!engineSupportsPackageTypeField) return + if(!assertScriptCanLoadAsCJSImpl) assertScriptCanLoadAsCJSImpl = require('../dist-raw/node-cjs-loader-utils').assertScriptCanLoadAsCJSImpl + assertScriptCanLoadAsCJSImpl!(filename) +} /** * Registered `ts-node` instance information. @@ -193,6 +198,12 @@ export interface CreateOptions { readFile?: (path: string) => string | undefined fileExists?: (path: string) => boolean transformers?: _ts.CustomTransformers | ((p: _ts.Program) => _ts.CustomTransformers) + /** + * True if require() hooks should interop with experimental ESM loader. + * Enabled explicitly via a flag since it is a breaking change. + * @internal + */ + experimentalEsmLoader?: boolean } /** @@ -261,7 +272,8 @@ export const DEFAULTS: RegisterOptions = { transpileOnly: yn(process.env.TS_NODE_TRANSPILE_ONLY), typeCheck: yn(process.env.TS_NODE_TYPE_CHECK), compilerHost: yn(process.env.TS_NODE_COMPILER_HOST), - logError: yn(process.env.TS_NODE_LOG_ERROR) + logError: yn(process.env.TS_NODE_LOG_ERROR), + experimentalEsmLoader: false } /** @@ -861,7 +873,9 @@ function registerExtension ( require.extensions[ext] = function (m: any, filename) { // tslint:disable-line if (register.ignored(filename)) return old(m, filename) - assertScriptIsNotEsm(filename) + if (register.options.experimentalEsmLoader) { + assertScriptCanLoadAsCJS(filename) + } const _compile = m._compile From f5fab010fd085ecdd47943fcad8cf77c87ba6078 Mon Sep 17 00:00:00 2001 From: Andrew Bradley Date: Sat, 9 May 2020 18:39:04 -0400 Subject: [PATCH 08/14] Fix linter failures --- src/index.ts | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/index.ts b/src/index.ts index b6dd9f290..a5da1e682 100644 --- a/src/index.ts +++ b/src/index.ts @@ -9,7 +9,7 @@ import * as _ts from 'typescript' * Does this version of node obey the package.json "type" field * and attempt to load scripts as ESM */ -let engineSupportsPackageTypeField = parseInt(process.versions.node.split('.')[0]) >= 12 +let 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 @@ -18,9 +18,9 @@ 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. */ -function assertScriptCanLoadAsCJS(filename: string) { - if(!engineSupportsPackageTypeField) return - if(!assertScriptCanLoadAsCJSImpl) assertScriptCanLoadAsCJSImpl = require('../dist-raw/node-cjs-loader-utils').assertScriptCanLoadAsCJSImpl +function assertScriptCanLoadAsCJS (filename: string) { + if (!engineSupportsPackageTypeField) return + if (!assertScriptCanLoadAsCJSImpl) assertScriptCanLoadAsCJSImpl = require('../dist-raw/node-cjs-loader-utils').assertScriptCanLoadAsCJSImpl assertScriptCanLoadAsCJSImpl!(filename) } From d70658bf8715963901ff4adb235779f408d18c68 Mon Sep 17 00:00:00 2001 From: Andrew Bradley Date: Sat, 16 May 2020 08:42:35 -0400 Subject: [PATCH 09/14] Add tests --- src/index.spec.ts | 27 +++++++++++++++---- src/index.ts | 2 +- .../esm-package/loaded-as.ts | 5 ++++ .../esm-package/package.json | 3 +++ tests/esm-err-require-esm/index.js | 1 + 5 files changed, 32 insertions(+), 6 deletions(-) create mode 100644 tests/esm-err-require-esm/esm-package/loaded-as.ts create mode 100644 tests/esm-err-require-esm/esm-package/package.json create mode 100644 tests/esm-err-require-esm/index.js diff --git a/src/index.spec.ts b/src/index.spec.ts index 25346385f..930bb6b61 100644 --- a/src/index.spec.ts +++ b/src/index.spec.ts @@ -678,12 +678,12 @@ describe('ts-node', function () { }) }) - if (semver.gte(process.version, '13.0.0')) { - describe('esm', () => { - this.slow(1000) + describe('esm', () => { + this.slow(1000) - const cmd = `node --loader ../../esm.mjs` + const cmd = `node --loader ts-node/esm.mjs` + if (semver.gte(process.version, '13.0.0')) { it('should compile and execute as ESM', (done) => { exec(`${cmd} index.ts`, { cwd: join(__dirname, '../tests/esm') }, function (err, stdout) { expect(err).to.equal(null) @@ -701,6 +701,23 @@ describe('ts-node', function () { }) }) + it('throws ERR_REQUIRE_ESM when attempting to require() an ESM script while ESM loader is enabled', function (done) { + exec(`${cmd} ./index.js`, { cwd: join(__dirname, '../tests/esm-err-require-esm') }, function (err, stdout, stderr) { + expect(err).to.equal(null) + expect(stdout).to.contain('Error [ERR_REQUIRE_ESM]: Must use import to load ES Module:\n') + + return done() + }) + }) + } + + it('executes ESM as CJS, ignoring package.json "types" field (for backwards compatibility; should be changed in next major release to throw ERR_REQUIRE_ESM)', function (done) { + exec(`${BIN_PATH} ./tests/esm-err-require-esm/index.js`, function (err, stdout) { + expect(err).to.equal(null) + expect(stdout).to.equal('CommonJS\n') + + return done() + }) }) - } + }) }) diff --git a/src/index.ts b/src/index.ts index 7ab8c20e9..3344846cd 100644 --- a/src/index.ts +++ b/src/index.ts @@ -9,7 +9,7 @@ import * as _ts from 'typescript' * Does this version of node obey the package.json "type" field * and attempt to load scripts as ESM */ -let engineSupportsPackageTypeField = parseInt(process.versions.node.split('.')[0], 10) >= 12 +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 diff --git a/tests/esm-err-require-esm/esm-package/loaded-as.ts b/tests/esm-err-require-esm/esm-package/loaded-as.ts new file mode 100644 index 000000000..df054c0bc --- /dev/null +++ b/tests/esm-err-require-esm/esm-package/loaded-as.ts @@ -0,0 +1,5 @@ +// Log if this file is loaded as ESM or CommonJS +if(typeof module !== 'undefined') + console.log('CommonJS') +else + console.log('ESM') diff --git a/tests/esm-err-require-esm/esm-package/package.json b/tests/esm-err-require-esm/esm-package/package.json new file mode 100644 index 000000000..3dbc1ca59 --- /dev/null +++ b/tests/esm-err-require-esm/esm-package/package.json @@ -0,0 +1,3 @@ +{ + "type": "module" +} diff --git a/tests/esm-err-require-esm/index.js b/tests/esm-err-require-esm/index.js new file mode 100644 index 000000000..b2bf5a5fc --- /dev/null +++ b/tests/esm-err-require-esm/index.js @@ -0,0 +1 @@ +require('./esm-package/loaded-as') From b1c35b8c38ab20b9186d9c37765218ae92d0b8be Mon Sep 17 00:00:00 2001 From: Andrew Bradley Date: Sat, 16 May 2020 08:50:33 -0400 Subject: [PATCH 10/14] fix test? --- src/index.spec.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/index.spec.ts b/src/index.spec.ts index 930bb6b61..cc619c07f 100644 --- a/src/index.spec.ts +++ b/src/index.spec.ts @@ -703,7 +703,7 @@ describe('ts-node', function () { }) it('throws ERR_REQUIRE_ESM when attempting to require() an ESM script while ESM loader is enabled', function (done) { exec(`${cmd} ./index.js`, { cwd: join(__dirname, '../tests/esm-err-require-esm') }, function (err, stdout, stderr) { - expect(err).to.equal(null) + expect(err).to.not.equal(null) expect(stdout).to.contain('Error [ERR_REQUIRE_ESM]: Must use import to load ES Module:\n') return done() From 5e8090af9bf0a046ed746c6f39acc33f89df8b35 Mon Sep 17 00:00:00 2001 From: Andrew Bradley Date: Sat, 16 May 2020 09:00:10 -0400 Subject: [PATCH 11/14] Fix tests --- package-lock.json | 33 +++++++++++++++++++++++++++++++++ package.json | 1 + src/index.spec.ts | 4 +++- 3 files changed, 37 insertions(+), 1 deletion(-) diff --git a/package-lock.json b/package-lock.json index 23e6967c2..04e912bb7 100644 --- a/package-lock.json +++ b/package-lock.json @@ -50,12 +50,35 @@ "integrity": "sha1-Lpu4n5rMOrAQjw89xNvc8v/4qZw=", "dev": true }, + "@types/events": { + "version": "3.0.0", + "resolved": "https://registry.npmjs.org/@types/events/-/events-3.0.0.tgz", + "integrity": "sha512-EaObqwIvayI5a8dCzhFrjKzVwKLxjoG9T6Ppd5CEo07LRKfQ8Yokw54r5+Wq7FaBQ+yXRvQAYPrHwya1/UFt9g==", + "dev": true + }, + "@types/glob": { + "version": "7.1.1", + "resolved": "https://registry.npmjs.org/@types/glob/-/glob-7.1.1.tgz", + "integrity": "sha512-1Bh06cbWJUHMC97acuD6UMG29nMt0Aqz1vF3guLfG+kHHJhy3AyohZFFxYk2f7Q1SQIrNwvncxAE0N/9s70F2w==", + "dev": true, + "requires": { + "@types/events": "*", + "@types/minimatch": "*", + "@types/node": "*" + } + }, "@types/json-schema": { "version": "7.0.4", "resolved": "https://registry.npmjs.org/@types/json-schema/-/json-schema-7.0.4.tgz", "integrity": "sha1-OP1z3f2bVaux4bLtV4y1W9e30zk=", "dev": true }, + "@types/minimatch": { + "version": "3.0.3", + "resolved": "https://registry.npmjs.org/@types/minimatch/-/minimatch-3.0.3.tgz", + "integrity": "sha512-tHq6qdbT9U1IRSGf14CL0pUlULksvY9OZ+5eEgl1N7t+OA3tGvNpxJCzuKQlsNgCVwbAs670L1vcVQi8j9HjnA==", + "dev": true + }, "@types/mocha": { "version": "5.2.7", "resolved": "https://registry.npmjs.org/@types/mocha/-/mocha-5.2.7.tgz", @@ -80,6 +103,16 @@ "integrity": "sha512-ft7OuDGUo39e+9LGwUewf2RyEaNBOjWbHUmD5bzjNuSuDabccE/1IuO7iR0dkzLjVUKxTMq69E+FmKfbgBcfbQ==", "dev": true }, + "@types/rimraf": { + "version": "3.0.0", + "resolved": "https://registry.npmjs.org/@types/rimraf/-/rimraf-3.0.0.tgz", + "integrity": "sha512-7WhJ0MdpFgYQPXlF4Dx+DhgvlPCfz/x5mHaeDQAKhcenvQP1KCpLQ18JklAqeGMYSAT2PxLpzd0g2/HE7fj7hQ==", + "dev": true, + "requires": { + "@types/glob": "*", + "@types/node": "*" + } + }, "@types/semver": { "version": "7.1.0", "resolved": "https://registry.npmjs.org/@types/semver/-/semver-7.1.0.tgz", diff --git a/package.json b/package.json index fb6f91962..1841f9958 100644 --- a/package.json +++ b/package.json @@ -65,6 +65,7 @@ "@types/node": "13.13.5", "@types/proxyquire": "^1.3.28", "@types/react": "^16.0.2", + "@types/rimraf": "^3.0.0", "@types/semver": "^7.1.0", "@types/source-map-support": "^0.5.0", "axios": "^0.19.0", diff --git a/src/index.spec.ts b/src/index.spec.ts index cc619c07f..a1bb9b7fb 100644 --- a/src/index.spec.ts +++ b/src/index.spec.ts @@ -7,6 +7,7 @@ import proxyquire = require('proxyquire') import { register, create, VERSION } from './index' import { unlinkSync, existsSync, statSync } from 'fs' import * as promisify from 'util.promisify' +import { sync as rimrafSync } from 'rimraf' const execP = promisify(exec) @@ -20,6 +21,7 @@ const SOURCE_MAP_REGEXP = /\/\/# sourceMappingURL=data:application\/json;charset // Pack and install ts-node locally, necessary to test package "exports" before(async function () { this.timeout(30000) + rimrafSync(join(TEST_DIR, 'node_modules')) await execP(`npm install`, { cwd: TEST_DIR }) const packageLockPath = join(TEST_DIR, 'package-lock.json') existsSync(packageLockPath) && unlinkSync(packageLockPath) @@ -704,7 +706,7 @@ describe('ts-node', function () { it('throws ERR_REQUIRE_ESM when attempting to require() an ESM script while ESM loader is enabled', function (done) { exec(`${cmd} ./index.js`, { cwd: join(__dirname, '../tests/esm-err-require-esm') }, function (err, stdout, stderr) { expect(err).to.not.equal(null) - expect(stdout).to.contain('Error [ERR_REQUIRE_ESM]: Must use import to load ES Module:\n') + expect(stderr).to.contain('Error [ERR_REQUIRE_ESM]: Must use import to load ES Module:') return done() }) From 9f9efd6464b37958562b6506e68aa3390339a699 Mon Sep 17 00:00:00 2001 From: Andrew Bradley Date: Sun, 17 May 2020 07:19:29 -0400 Subject: [PATCH 12/14] add comments to explain copy-pasted cjs loader code from node core --- dist-raw/README.md | 13 +++++++++++++ dist-raw/node-cjs-loader-utils.js | 24 +++++++++++++++++++++--- 2 files changed, 34 insertions(+), 3 deletions(-) create mode 100644 dist-raw/README.md diff --git a/dist-raw/README.md b/dist-raw/README.md new file mode 100644 index 000000000..b7b4c4d7d --- /dev/null +++ b/dist-raw/README.md @@ -0,0 +1,13 @@ +The `dist-raw` directory contains JS sources that are distributed verbatim, not compiled nor typechecked via TS. + +To implement ESM support, we unfortunately must duplicate some of node's built-in functionality that is not +exposed via an API. We have copy-pasted the necessary code from https://github.com/nodejs/node/tree/master/lib +then modified it to suite our needs. + +Formatting may be intentionally bad to keep the diff as small as possible, to make it easier to merge +upstream changes and understand our modifications. For example, when we need to wrap node's source code +in a factory function, we will not indent the function body, to avoid whitespace changes in the diff. + +One obvious problem with this approach: the code has been pulled from one version of node, whereas users of ts-node +run multiple versions of node. +Users running node 12 may see that ts-node behaves like node 14, for example. diff --git a/dist-raw/node-cjs-loader-utils.js b/dist-raw/node-cjs-loader-utils.js index 8512bba3f..0c5fabf9c 100644 --- a/dist-raw/node-cjs-loader-utils.js +++ b/dist-raw/node-cjs-loader-utils.js @@ -1,10 +1,14 @@ -// copied from https://github.com/nodejs/node/blob/master/lib/internal/modules/cjs/loader.js +// Copied from several files in node's source code. +// https://github.com/nodejs/node/blob/2d5d77306f6dff9110c1f77fefab25f973415770/lib/internal/modules/cjs/loader.js +// Each function and variable below must have a comment linking to the source in node's github repo. + const path = require('path'); const fs = require('fs'); module.exports.assertScriptCanLoadAsCJSImpl = assertScriptCanLoadAsCJSImpl; // copied from Module._extensions['.js'] +// https://github.com/nodejs/node/blob/2d5d77306f6dff9110c1f77fefab25f973415770/lib/internal/modules/cjs/loader.js#L1211-L1217 function assertScriptCanLoadAsCJSImpl(filename) { const pkg = readPackageScope(filename); // Function require shouldn't be used in ES modules. @@ -15,6 +19,7 @@ function assertScriptCanLoadAsCJSImpl(filename) { } } +// Copied from https://github.com/nodejs/node/blob/2d5d77306f6dff9110c1f77fefab25f973415770/lib/internal/modules/cjs/loader.js#L285-L301 function readPackageScope(checkPath) { const rootSeparatorIndex = checkPath.indexOf(path.sep); let separatorIndex; @@ -33,8 +38,10 @@ function readPackageScope(checkPath) { return false; } +// Copied from https://github.com/nodejs/node/blob/2d5d77306f6dff9110c1f77fefab25f973415770/lib/internal/modules/cjs/loader.js#L249 const packageJsonCache = new Map(); +// Copied from https://github.com/nodejs/node/blob/2d5d77306f6dff9110c1f77fefab25f973415770/lib/internal/modules/cjs/loader.js#L251-L283 function readPackage(requestPath) { const jsonPath = path.resolve(requestPath, 'package.json'); @@ -70,6 +77,8 @@ function readPackage(requestPath) { } } +// In node's core, this is implemented in C +// https://github.com/nodejs/node/blob/e9f293750760d59243020d0376edf242c9a26b67/src/node_file.cc#L845-L939 function internalModuleReadJSON(path) { try { return fs.readFileSync(path, 'utf8') @@ -79,10 +88,18 @@ function internalModuleReadJSON(path) { } } +// Native ERR_REQUIRE_ESM Error is declared here: +// https://github.com/nodejs/node/blob/2d5d77306f6dff9110c1f77fefab25f973415770/lib/internal/errors.js#L1294-L1313 +// Error class factory is implemented here: +// function E: https://github.com/nodejs/node/blob/2d5d77306f6dff9110c1f77fefab25f973415770/lib/internal/errors.js#L323-L341 +// function makeNodeErrorWithCode: https://github.com/nodejs/node/blob/2d5d77306f6dff9110c1f77fefab25f973415770/lib/internal/errors.js#L251-L278 +// The code below should create an error that matches the native error as closely as possible. +// Third-party libraries which attempt to catch the native ERR_REQUIRE_ESM should recognize our imitation error. function createErrRequireEsm(filename, parentPath, packageJsonPath) { - // Attempt to create an error object that matches node's native error close enough const code = 'ERR_REQUIRE_ESM' const err = new Error(getMessage(filename, parentPath, packageJsonPath)) + // Set `name` to be used in stack trace, generate stack trace with that name baked in, then re-declare the `name` field. + // This trick is copied from node's source. err.name = `Error [${ code }]` err.stack Object.defineProperty(err, 'name', { @@ -94,7 +111,8 @@ function createErrRequireEsm(filename, parentPath, packageJsonPath) { err.code = code return err - // copy-pasted from https://github.com/nodejs/node/blob/master/lib/internal/errors.js#L1294-L1311 + // Copy-pasted from https://github.com/nodejs/node/blob/b533fb3508009e5f567cc776daba8fbf665386a6/lib/internal/errors.js#L1293-L1311 + // so that our error message is identical to the native message. function getMessage(filename, parentPath = null, packageJsonPath = null) { const ext = path.extname(filename) let msg = `Must use import to load ES Module: ${filename}`; From 7a1a55bc69bc35b665795c8a49b20131a3cb5fc0 Mon Sep 17 00:00:00 2001 From: Andrew Bradley Date: Mon, 18 May 2020 02:11:03 -0400 Subject: [PATCH 13/14] fix --- .github/workflows/continuous-integration.yml | 1 + 1 file changed, 1 insertion(+) diff --git a/.github/workflows/continuous-integration.yml b/.github/workflows/continuous-integration.yml index d8ae590d3..4c49393fd 100644 --- a/.github/workflows/continuous-integration.yml +++ b/.github/workflows/continuous-integration.yml @@ -58,6 +58,7 @@ jobs: typescriptFlag: latest - flavor: 4 node: 13 + nodeFlag: 13 typescript: latest typescriptFlag: latest - flavor: 5 From 1a21a7d8dbc2a2543b29d8b86d480237f94d6fa2 Mon Sep 17 00:00:00 2001 From: Andrew Bradley Date: Tue, 19 May 2020 20:21:05 -0400 Subject: [PATCH 14/14] tweak comment --- src/index.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/index.ts b/src/index.ts index 3344846cd..ad8d250f5 100644 --- a/src/index.ts +++ b/src/index.ts @@ -7,7 +7,7 @@ import * as _ts from 'typescript' /** * Does this version of node obey the package.json "type" field - * and attempt to load scripts as ESM + * and throw ERR_REQUIRE_ESM when attempting to require() an ESM modules. */ const engineSupportsPackageTypeField = parseInt(process.versions.node.split('.')[0], 10) >= 12