From dddbc46c8a1864a6788d52eb050f8f1c808fd9c0 Mon Sep 17 00:00:00 2001 From: KCM Date: Sun, 30 Jul 2023 17:04:22 -0500 Subject: [PATCH] feat: correct module systems by file extension. (#5) microsoft/TypeScript#51990 microsoft/TypeScript#27957 microsoft/TypeScript#50985 --- README.md | 6 +- package-lock.json | 6 +- package.json | 4 +- src/duel.js | 65 +++++++++++++++++-- test/__fixtures__/cjsProject/package.json | 18 +++++ .../{project => cjsProject}/src/cjs.cts | 0 .../{project => cjsProject}/src/esm.mts | 0 .../cjsProject/src/folder/another.mts | 9 +++ .../src/folder/module.ts | 2 +- test/__fixtures__/cjsProject/src/index.ts | 39 +++++++++++ test/__fixtures__/cjsProject/tsconfig.json | 12 ++++ .../{project => esmProject}/package.json | 0 test/__fixtures__/esmProject/script.js | 3 + test/__fixtures__/esmProject/src/cjs.cts | 39 +++++++++++ test/__fixtures__/esmProject/src/esm.mts | 9 +++ .../esmProject/src/folder/another.mts | 9 +++ .../esmProject/src/folder/module.ts | 20 ++++++ .../{project => esmProject}/src/index.ts | 0 .../{project => esmProject}/tsconfig.json | 2 +- .../tsconfig.noOutDir.json | 0 .../{project => esmProject}/tsconfig.not.json | 0 test/integration.js | 64 ++++++++++++++---- 22 files changed, 276 insertions(+), 31 deletions(-) create mode 100644 test/__fixtures__/cjsProject/package.json rename test/__fixtures__/{project => cjsProject}/src/cjs.cts (100%) rename test/__fixtures__/{project => cjsProject}/src/esm.mts (100%) create mode 100644 test/__fixtures__/cjsProject/src/folder/another.mts rename test/__fixtures__/{project => cjsProject}/src/folder/module.ts (93%) create mode 100644 test/__fixtures__/cjsProject/src/index.ts create mode 100644 test/__fixtures__/cjsProject/tsconfig.json rename test/__fixtures__/{project => esmProject}/package.json (100%) create mode 100644 test/__fixtures__/esmProject/script.js create mode 100644 test/__fixtures__/esmProject/src/cjs.cts create mode 100644 test/__fixtures__/esmProject/src/esm.mts create mode 100644 test/__fixtures__/esmProject/src/folder/another.mts create mode 100644 test/__fixtures__/esmProject/src/folder/module.ts rename test/__fixtures__/{project => esmProject}/src/index.ts (100%) rename test/__fixtures__/{project => esmProject}/tsconfig.json (88%) rename test/__fixtures__/{project => esmProject}/tsconfig.noOutDir.json (100%) rename test/__fixtures__/{project => esmProject}/tsconfig.not.json (100%) diff --git a/README.md b/README.md index 27e1897..dc85487 100644 --- a/README.md +++ b/README.md @@ -6,7 +6,7 @@ Node.js tool for creating a TypeScript dual package. -Early stages of development. Inspired by https://github.com/microsoft/TypeScript/issues/49462. +Inspired by https://github.com/microsoft/TypeScript/issues/49462. ## Requirements @@ -79,8 +79,8 @@ Options: ## Gotchas -* Unfortunately, TypeScript doesn't really build [dual packages](https://nodejs.org/api/packages.html#dual-commonjses-module-packages) very well in regards to preserving module system by file extension. For instance, it will **always** create CJS exports when `--module commonjs` is used, _even on files with an `.mts` extension_, which is contrary to [how Node determines module systems](https://nodejs.org/api/packages.html#determining-module-system). The `tsc` compiler is fundamentally broken in this regard. One reference issue is https://github.com/microsoft/TypeScript/issues/54573. If you use `.mts` extensions to enforce an ESM module system, this will break in the corresponding dual CJS build. There is no way to fix this until TypeScript fixes their compiler. +These are definitely edge cases, and would only really come up if your project mixes file extensions. For example, if you have `.ts` files combined with `.mts`, and/or `.cts`. For most project, things should just work as expected. -* If targeting a dual CJS build, and you are using [top level `await`](https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Operators/await#top_level_await), you will most likely encounter the compilation error `error TS1378: Top-level 'await' expressions are only allowed when the 'module' option is set to 'es2022', 'esnext', 'system', 'node16', or 'nodenext', and the 'target' option is set to 'es2017' or higher.` during the CJS build. This is because `duel` creates a temporary `tsconfig.json` from your original and necessarily overwrites the `--module` and `--moduleResolution` based on the provided `--target-ext`. There is no workaround other than to **not** use top level await if you want a dual build. +* Unfortunately, TypeScript doesn't really build [dual packages](https://nodejs.org/api/packages.html#dual-commonjses-module-packages) very well in regards to preserving module system by file extension. For instance, there doesn't appear to be a way to convert an arbitrary `.ts` file into another module system, _while also preserving the module system of `.mts` and `.cts` files_. In my opinion, the `tsc` compiler is fundamentally broken in this regard, and at best is enforcing usage patterns it shouldn't. If you want to see one of my extended rants on this, check out this [comment](https://github.com/microsoft/TypeScript/pull/50985#issuecomment-1656991606). This is only mentioned for transparency, `duel` will correct for this and produce files with the module system you would expect based on the files extension, so that it works with [how Node.js determines module systems](https://nodejs.org/api/packages.html#determining-module-system). * If doing an `import type` across module systems, i.e. from `.mts` into `.cts`, or vice versa, you might encounter the compilation error ``error TS1452: 'resolution-mode' assertions are only supported when `moduleResolution` is `node16` or `nodenext`.``. This is a [known issue](https://github.com/microsoft/TypeScript/issues/49055) and TypeScript currently suggests installing the nightly build, i.e. `npm i typescript@next`. diff --git a/package-lock.json b/package-lock.json index 5a8c555..48e10aa 100644 --- a/package-lock.json +++ b/package-lock.json @@ -1,12 +1,12 @@ { "name": "@knighted/duel", - "version": "1.0.0-alpha.3", + "version": "1.0.0-alpha.4", "lockfileVersion": 3, "requires": true, "packages": { "": { "name": "@knighted/duel", - "version": "1.0.0-alpha.3", + "version": "1.0.0-alpha.4", "license": "MIT", "dependencies": { "@knighted/specifier": "^1.0.0-alpha.5", @@ -27,7 +27,7 @@ "node": ">=16.19.0" }, "peerDependencies": { - "typescript": "^5.0.0" + "typescript": ">=4.0.0" } }, "node_modules/@aashutoshrathi/word-wrap": { diff --git a/package.json b/package.json index d226a1f..92f5c3f 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "@knighted/duel", - "version": "1.0.0-alpha.3", + "version": "1.0.0-alpha.4", "description": "TypeScript dual packages.", "type": "module", "main": "dist", @@ -44,7 +44,7 @@ "url": "https://github.com/knightedcodemonkey/duel/issues" }, "peerDependencies": { - "typescript": "^5.0.0" + "typescript": ">=4.0.0" }, "devDependencies": { "babel-dual-package": "^1.0.0-rc.5", diff --git a/src/duel.js b/src/duel.js index c777398..fce7fc5 100755 --- a/src/duel.js +++ b/src/duel.js @@ -1,9 +1,9 @@ #!/usr/bin/env node import { argv, cwd } from 'node:process' -import { join } from 'node:path' +import { join, relative } from 'node:path' import { spawnSync } from 'node:child_process' -import { writeFile, rm } from 'node:fs/promises' +import { writeFile, copyFile, rm } from 'node:fs/promises' import { randomBytes } from 'node:crypto' import { performance } from 'node:perf_hooks' @@ -13,7 +13,6 @@ import { specifier } from '@knighted/specifier' import { init } from './init.js' import { getRealPathAsFileUrl, logError, log } from './util.js' -// TypeScript is defined as a peer dependency. const tsc = join(cwd(), 'node_modules', '.bin', 'tsc') const runBuild = project => { const { status, error } = spawnSync(tsc, ['-p', project], { stdio: 'inherit' }) @@ -42,7 +41,7 @@ const duel = async args => { const ctx = await init(args) if (ctx) { - const { projectDir, tsconfig, targetExt, configPath } = ctx + const { projectDir, tsconfig, targetExt, configPath, absoluteOutDir } = ctx const startTime = performance.now() log('Starting primary build...\n') @@ -55,13 +54,15 @@ const duel = async args => { const { outDir } = tsconfig.compilerOptions const dualConfigPath = join(projectDir, `tsconfig.${hex}.json`) const dualOutDir = isCjsBuild ? join(outDir, 'cjs') : join(outDir, 'mjs') + // Using structuredClone() would require node >= 17.0.0 const tsconfigDual = { ...tsconfig, compilerOptions: { ...tsconfig.compilerOptions, outDir: dualOutDir, - module: isCjsBuild ? 'CommonJS' : 'NodeNext', - moduleResolution: isCjsBuild ? 'Node' : 'NodeNext', + module: isCjsBuild ? 'CommonJS' : 'ESNext', + // Best way to make this work given how tsc works + moduleResolution: 'Node', }, } @@ -71,7 +72,8 @@ const duel = async args => { await rm(dualConfigPath, { force: true }) if (success) { - const filenames = await glob(`${join(projectDir, dualOutDir)}/**/*{.js,.d.ts}`, { + const absoluteDualOutDir = join(projectDir, dualOutDir) + const filenames = await glob(`${absoluteDualOutDir}/**/*{.js,.d.ts}`, { ignore: 'node_modules/**', }) @@ -95,6 +97,55 @@ const duel = async args => { await rm(filename, { force: true }) } + /** + * This is a fix for tsc compiler which doesn't seem to support + * converting an arbitrary `.ts` file, into another module system, + * while also preserving the module systems of `.mts` and `.cts` files. + * + * Hopefully it can be removed when TS updates their supported options, + * or at least how the combination of `--module` and `--moduleResolution` + * currently work. + * + * @see https://github.com/microsoft/TypeScript/pull/50985#issuecomment-1656991606 + */ + if (isCjsBuild) { + const mjsFiles = await glob(`${absoluteOutDir}/**/*.mjs`, { + ignore: ['node_modules/**', `${absoluteDualOutDir}/**`], + }) + + for (const filename of mjsFiles) { + const relativeFn = relative(absoluteOutDir, filename) + + await copyFile(filename, join(absoluteDualOutDir, relativeFn)) + } + } else { + const cjsFiles = await glob(`${absoluteOutDir}/**/*.cjs`, { + ignore: ['node_modules/**', `${absoluteDualOutDir}/**`], + }) + + for (const filename of cjsFiles) { + const relativeFn = relative(absoluteOutDir, filename) + + await copyFile(filename, join(absoluteDualOutDir, relativeFn)) + } + + /** + * Now copy the good .mjs files from the dual out dir + * to the original out dir, but build the file path + * from the original out dir to distinguish from the + * dual build .mjs files. + */ + const mjsFiles = await glob(`${absoluteOutDir}/**/*.mjs`, { + ignore: ['node_modules/**', `${absoluteDualOutDir}/**`], + }) + + for (const filename of mjsFiles) { + const relativeFn = relative(absoluteOutDir, filename) + + await copyFile(join(absoluteDualOutDir, relativeFn), filename) + } + } + log( `Successfully created a dual ${targetExt .replace('.', '') diff --git a/test/__fixtures__/cjsProject/package.json b/test/__fixtures__/cjsProject/package.json new file mode 100644 index 0000000..a5c8421 --- /dev/null +++ b/test/__fixtures__/cjsProject/package.json @@ -0,0 +1,18 @@ +{ + "version": "0.0.0", + "type": "commonjs", + "exports": { + ".": { + "import": { + "types": "./dist/mjs/index.d.mts", + "default": "./dist/index.mjs" + }, + "require": { + "types": "./dist/index.d.cts", + "default": "./dist/index.cjs" + }, + "default": "./dist/index.cjs" + }, + "./package.json": "./package.json" + } +} diff --git a/test/__fixtures__/project/src/cjs.cts b/test/__fixtures__/cjsProject/src/cjs.cts similarity index 100% rename from test/__fixtures__/project/src/cjs.cts rename to test/__fixtures__/cjsProject/src/cjs.cts diff --git a/test/__fixtures__/project/src/esm.mts b/test/__fixtures__/cjsProject/src/esm.mts similarity index 100% rename from test/__fixtures__/project/src/esm.mts rename to test/__fixtures__/cjsProject/src/esm.mts diff --git a/test/__fixtures__/cjsProject/src/folder/another.mts b/test/__fixtures__/cjsProject/src/folder/another.mts new file mode 100644 index 0000000..4d9a7bf --- /dev/null +++ b/test/__fixtures__/cjsProject/src/folder/another.mts @@ -0,0 +1,9 @@ +interface ESM { + esm: boolean; +} + +export const esm: ESM = { + esm: true +} + +export type { ESM } diff --git a/test/__fixtures__/project/src/folder/module.ts b/test/__fixtures__/cjsProject/src/folder/module.ts similarity index 93% rename from test/__fixtures__/project/src/folder/module.ts rename to test/__fixtures__/cjsProject/src/folder/module.ts index 1947289..ef2ff4f 100644 --- a/test/__fixtures__/project/src/folder/module.ts +++ b/test/__fixtures__/cjsProject/src/folder/module.ts @@ -16,5 +16,5 @@ const mod: Mod = { } } -export { mod } +export { mod, cjs } export type { Mod } diff --git a/test/__fixtures__/cjsProject/src/index.ts b/test/__fixtures__/cjsProject/src/index.ts new file mode 100644 index 0000000..e2efc10 --- /dev/null +++ b/test/__fixtures__/cjsProject/src/index.ts @@ -0,0 +1,39 @@ +import { mod } from "./folder/module.js" +import { cjs } from './cjs.cjs' + +import type { Mod } from "./folder/module.js" +import type { CJS } from "./cjs.cjs" + +interface User { + name: string; + id: number; + mod: Mod; + esm: any; + cjs: CJS; +} + +class UserAccount { + name: string; + id: number; + mod: Mod; + esm: any; + cjs: CJS; + + constructor(name: string, id: number, mod: Mod, esm: any, cjs: CJS) { + this.name = name; + this.id = id; + this.mod = mod; + this.esm = esm; + this.cjs = cjs; + } +} + +const getUser = async () => { + const { esm } = await import('./esm.mjs') + + return new UserAccount("Murphy", 1, mod, esm, cjs) +} + +export type { User } + +export { getUser } diff --git a/test/__fixtures__/cjsProject/tsconfig.json b/test/__fixtures__/cjsProject/tsconfig.json new file mode 100644 index 0000000..babc20e --- /dev/null +++ b/test/__fixtures__/cjsProject/tsconfig.json @@ -0,0 +1,12 @@ +{ + "compilerOptions": { + "target": "ESNext", + "module": "CommonJS", + "moduleResolution": "Node", + "declaration": true, + "strict": false, + "outDir": "dist", + "lib": ["ES2015"] + }, + "include": ["src"] +} diff --git a/test/__fixtures__/project/package.json b/test/__fixtures__/esmProject/package.json similarity index 100% rename from test/__fixtures__/project/package.json rename to test/__fixtures__/esmProject/package.json diff --git a/test/__fixtures__/esmProject/script.js b/test/__fixtures__/esmProject/script.js new file mode 100644 index 0000000..8286baa --- /dev/null +++ b/test/__fixtures__/esmProject/script.js @@ -0,0 +1,3 @@ +//import * as cjs from './dist/cjs/cjs.cjs' + +require('./dist/cjs/cjs.cjs') diff --git a/test/__fixtures__/esmProject/src/cjs.cts b/test/__fixtures__/esmProject/src/cjs.cts new file mode 100644 index 0000000..a3d3256 --- /dev/null +++ b/test/__fixtures__/esmProject/src/cjs.cts @@ -0,0 +1,39 @@ +/* +import type { ESM } from './esm.mjs' assert { 'resolution-mode': 'import' }; + +interface CJS { + cjs: boolean, + esm: ESM; +} + +const func = async () => { + const { esm } = await import('./esm.mjs') + + const cjs: CJS = { + cjs: true, + esm + } + + return cjs +} + +export { func } + +export type { CJS } +*/ + +import MagicString from "magic-string" + +interface CJS { + cjs: boolean; + magic: MagicString; +} + +const cjs: CJS = { + cjs: true, + magic: new MagicString('magic') +} + +export { cjs } + +export type { CJS } diff --git a/test/__fixtures__/esmProject/src/esm.mts b/test/__fixtures__/esmProject/src/esm.mts new file mode 100644 index 0000000..4d9a7bf --- /dev/null +++ b/test/__fixtures__/esmProject/src/esm.mts @@ -0,0 +1,9 @@ +interface ESM { + esm: boolean; +} + +export const esm: ESM = { + esm: true +} + +export type { ESM } diff --git a/test/__fixtures__/esmProject/src/folder/another.mts b/test/__fixtures__/esmProject/src/folder/another.mts new file mode 100644 index 0000000..4d9a7bf --- /dev/null +++ b/test/__fixtures__/esmProject/src/folder/another.mts @@ -0,0 +1,9 @@ +interface ESM { + esm: boolean; +} + +export const esm: ESM = { + esm: true +} + +export type { ESM } diff --git a/test/__fixtures__/esmProject/src/folder/module.ts b/test/__fixtures__/esmProject/src/folder/module.ts new file mode 100644 index 0000000..ef2ff4f --- /dev/null +++ b/test/__fixtures__/esmProject/src/folder/module.ts @@ -0,0 +1,20 @@ +import MagicString from "magic-string"; +import { cjs } from "../cjs.cjs"; + +import type { CJS } from "../cjs.cjs"; + +interface Mod { + prop: string; + cjs: CJS +} + +const mod: Mod = { + prop: 'foobar', + cjs: { + cjs: true, + magic: new MagicString('module') + } +} + +export { mod, cjs } +export type { Mod } diff --git a/test/__fixtures__/project/src/index.ts b/test/__fixtures__/esmProject/src/index.ts similarity index 100% rename from test/__fixtures__/project/src/index.ts rename to test/__fixtures__/esmProject/src/index.ts diff --git a/test/__fixtures__/project/tsconfig.json b/test/__fixtures__/esmProject/tsconfig.json similarity index 88% rename from test/__fixtures__/project/tsconfig.json rename to test/__fixtures__/esmProject/tsconfig.json index 3895bac..ce581a7 100644 --- a/test/__fixtures__/project/tsconfig.json +++ b/test/__fixtures__/esmProject/tsconfig.json @@ -8,5 +8,5 @@ "outDir": "dist", "lib": ["ES2015"] }, - "include": ["src/*.ts"] + "include": ["src"] } diff --git a/test/__fixtures__/project/tsconfig.noOutDir.json b/test/__fixtures__/esmProject/tsconfig.noOutDir.json similarity index 100% rename from test/__fixtures__/project/tsconfig.noOutDir.json rename to test/__fixtures__/esmProject/tsconfig.noOutDir.json diff --git a/test/__fixtures__/project/tsconfig.not.json b/test/__fixtures__/esmProject/tsconfig.not.json similarity index 100% rename from test/__fixtures__/project/tsconfig.not.json rename to test/__fixtures__/esmProject/tsconfig.not.json diff --git a/test/integration.js b/test/integration.js index 59a4727..5ef15ee 100644 --- a/test/integration.js +++ b/test/integration.js @@ -2,14 +2,15 @@ import { describe, it, before } from 'node:test' import assert from 'node:assert/strict' import { fileURLToPath } from 'node:url' import { dirname, resolve } from 'node:path' -import { rm } from 'node:fs/promises' +import { rm, readFile } from 'node:fs/promises' import { existsSync } from 'node:fs' import { duel } from '../src/duel.js' const __filename = fileURLToPath(import.meta.url) const __dirname = dirname(__filename) -const dist = resolve(__dirname, '__fixtures__/project/dist') +const esmDist = resolve(__dirname, '__fixtures__/esmProject/dist') +const cjsDist = resolve(__dirname, '__fixtures__/cjsProject/dist') const errDist = resolve(__dirname, '__fixtures__/compileErrors/dist') const rmDist = async distPath => { await rm(distPath, { recursive: true, force: true }) @@ -17,7 +18,8 @@ const rmDist = async distPath => { describe('duel', () => { before(async () => { - await rmDist(dist) + await rmDist(esmDist) + await rmDist(cjsDist) await rmDist(errDist) }) @@ -39,7 +41,7 @@ describe('duel', () => { const spy = t.mock.method(global.console, 'log') /** * Should error due to the cwd of this processs not being - * within test/__fixtures__/project. + * within test/__fixtures__/esmProject. */ await duel() assert.ok(spy.mock.calls[0].arguments[1].endsWith('is not a file or directory.')) @@ -55,14 +57,14 @@ describe('duel', () => { it('reports errors when --project is not valid json', async t => { const spy = t.mock.method(global.console, 'log') - await duel(['-p', 'test/__fixtures__/project/tsconfig.not.json']) + await duel(['-p', 'test/__fixtures__/esmProject/tsconfig.not.json']) assert.ok(spy.mock.calls[0].arguments[1].endsWith('not parsable as JSON.')) }) it('reports errors when the config in --project has no outDir', async t => { const spy = t.mock.method(global.console, 'log') - await duel(['-p', 'test/__fixtures__/project/tsconfig.noOutDir.json']) + await duel(['-p', 'test/__fixtures__/esmProject/tsconfig.noOutDir.json']) assert.equal( spy.mock.calls[0].arguments[1], 'You must define an `outDir` in your project config.', @@ -80,21 +82,55 @@ describe('duel', () => { const spy = t.mock.method(global.console, 'log') t.after(async () => { - await rmDist(dist) + await rmDist(esmDist) }) - await duel(['--project', 'test/__fixtures__/project', '--target-extension', '.cjs']) + await duel([ + '--project', + 'test/__fixtures__/esmProject', + '--target-extension', + '.cjs', + ]) // Third call because of logging for starting each build. assert.ok( spy.mock.calls[2].arguments[0].startsWith('Successfully created a dual CJS build'), ) // Check that the expected files and extensions are there - assert.ok(existsSync(resolve(dist, 'index.js'))) - assert.ok(existsSync(resolve(dist, 'index.d.ts'))) - assert.ok(existsSync(resolve(dist, 'cjs.cjs'))) - assert.ok(existsSync(resolve(dist, 'cjs/index.cjs'))) - assert.ok(existsSync(resolve(dist, 'cjs/index.d.cts'))) - assert.ok(existsSync(resolve(dist, 'cjs/esm.mjs'))) + assert.ok(existsSync(resolve(esmDist, 'index.js'))) + assert.ok(existsSync(resolve(esmDist, 'index.d.ts'))) + assert.ok(existsSync(resolve(esmDist, 'cjs.cjs'))) + assert.ok(existsSync(resolve(esmDist, 'cjs/index.cjs'))) + assert.ok(existsSync(resolve(esmDist, 'cjs/index.d.cts'))) + assert.ok(existsSync(resolve(esmDist, 'cjs/esm.mjs'))) + + // Check that there are no `exports.esm` statements in the .mjs file + const mjs = (await readFile(resolve(esmDist, 'cjs/esm.mjs'))).toString() + + assert.ok(mjs.indexOf('exports.esm') === -1) + }) + + it('can be passed .mjs as a --target-extension', async t => { + const spy = t.mock.method(global.console, 'log') + + t.after(async () => { + await rmDist(cjsDist) + }) + await duel(['-p', 'test/__fixtures__/cjsProject/tsconfig.json', '-x', '.mjs']) + + assert.ok( + spy.mock.calls[2].arguments[0].startsWith('Successfully created a dual MJS build'), + ) + assert.ok(existsSync(resolve(cjsDist, 'index.js'))) + assert.ok(existsSync(resolve(cjsDist, 'index.d.ts'))) + assert.ok(existsSync(resolve(cjsDist, 'mjs/index.mjs'))) + + // Check that the files are using the correct module system + const mjs = (await readFile(resolve(cjsDist, 'esm.mjs'))).toString() + const cjs = (await readFile(resolve(cjsDist, 'mjs/cjs.cjs'))).toString() + + assert.ok(mjs.indexOf('exports.esm') === -1) + assert.ok(mjs.indexOf('export const esm') > -1) + assert.ok(cjs.indexOf('exports.cjs') > -1) }) it('reports compilation errors during a build', async t => {