From 65e9a6b203f6fc1bc3dfebe73171c70afe1dc25d Mon Sep 17 00:00:00 2001 From: Jack Hsu Date: Tue, 14 Jan 2025 16:13:32 -0500 Subject: [PATCH] fix(js): non-buildable js libs specify type=module (#29620) In the new TS solution setup, non-buildable libraries should still`type: module`. This can lead to problem since ESM is used but the type will default to CJS. This PR also updates `nx sync` for TS references such that transitive deps are not sycned by default, unless `NX_ENABLE_TS_SYNC_TRANSITIVE_DEPENDENCIES=true` env var is setup. Previously, the transitive deps are synced unless the env var disables it. There isn't a good reason to enable it by default, and it is much cleaner to not sync by default. This means that if we have libs `a`, `b`, and `c`, where `a -> b` and `b -> c` dependency edges are formed, then running: ``` nx sync ``` Will update `a/tsconfig.json` to contain refs to `b` but not `c`. Whereas: ``` NX_ENABLE_TS_SYNC_TRANSITIVE_DEPENDENCIES=true nx sync ``` Will update `a/tsconfig.json` to contain refs to both `b` and `c`. ## Current Behavior `type` is missing ## Expected Behavior `type: module` is set in `package.json` ## Related Issue(s) Fixes # --- .../js/src/generators/library/library.spec.ts | 32 +++++ packages/js/src/generators/library/library.ts | 39 +++--- .../typescript-sync/typescript-sync.spec.ts | 120 ++---------------- .../typescript-sync/typescript-sync.ts | 2 +- 4 files changed, 62 insertions(+), 131 deletions(-) diff --git a/packages/js/src/generators/library/library.spec.ts b/packages/js/src/generators/library/library.spec.ts index 4d6ef0f40f..f1ba983c7c 100644 --- a/packages/js/src/generators/library/library.spec.ts +++ b/packages/js/src/generators/library/library.spec.ts @@ -1708,6 +1708,7 @@ describe('lib', () => { "main": "./src/index.ts", "name": "@proj/my-ts-lib", "private": true, + "type": "module", "types": "./src/index.ts", "version": "0.0.1", } @@ -1722,6 +1723,7 @@ describe('lib', () => { "main": "./src/index.js", "name": "@proj/my-js-lib", "private": true, + "type": "module", "types": "./src/index.js", "version": "0.0.1", } @@ -1870,5 +1872,35 @@ describe('lib', () => { tree.read('my-ts-lib/src/lib/my-ts-lib.spec.ts', 'utf-8') ).toContain(`import { myTsLib } from './my-ts-lib.js';`); }); + + it('should generate non-buildable setup', async () => { + await libraryGenerator(tree, { + ...defaultOptions, + directory: 'my-lib', + bundler: 'none', + unitTestRunner: 'none', + linter: 'none', + }); + + expect(readJson(tree, 'my-lib/package.json')).toMatchInlineSnapshot(` + { + "dependencies": {}, + "exports": { + ".": { + "default": "./src/index.ts", + "import": "./src/index.ts", + "types": "./src/index.ts", + }, + "./package.json": "./package.json", + }, + "main": "./src/index.ts", + "name": "@proj/my-lib", + "private": true, + "type": "module", + "types": "./src/index.ts", + "version": "0.0.1", + } + `); + }); }); }); diff --git a/packages/js/src/generators/library/library.ts b/packages/js/src/generators/library/library.ts index 9136578708..94714ae4c3 100644 --- a/packages/js/src/generators/library/library.ts +++ b/packages/js/src/generators/library/library.ts @@ -499,10 +499,7 @@ function createFiles(tree: Tree, options: NormalizedLibraryGeneratorOptions) { createProjectTsConfigs(tree, options); let fileNameImport = options.fileName; - if ( - options.bundler === 'vite' || - (options.isUsingTsSolutionConfig && options.bundler !== 'none') - ) { + if (options.bundler === 'vite' || options.isUsingTsSolutionConfig) { const tsConfig = readTsConfigFromTree( tree, join(options.projectRoot, 'tsconfig.lib.json') @@ -613,10 +610,9 @@ function createFiles(tree: Tree, options: NormalizedLibraryGeneratorOptions) { ...determineEntryFields(options), }; - if ( - options.isUsingTsSolutionConfig && - !['none', 'rollup', 'vite'].includes(options.bundler) - ) { + if (options.bundler === 'none') { + updatedPackageJson.type = 'module'; + } else if (options.bundler !== 'vite' && options.bundler !== 'rollup') { return getUpdatedPackageJsonContent(updatedPackageJson, { main: join(options.projectRoot, 'src/index.ts'), outputPath: joinPathFragments(options.projectRoot, 'dist'), @@ -646,19 +642,20 @@ function createFiles(tree: Tree, options: NormalizedLibraryGeneratorOptions) { packageJson.files = ['dist', '!**/*.tsbuildinfo']; } - if ( - options.isUsingTsSolutionConfig && - !['none', 'rollup', 'vite'].includes(options.bundler) - ) { - packageJson = getUpdatedPackageJsonContent(packageJson, { - main: join(options.projectRoot, 'src/index.ts'), - outputPath: joinPathFragments(options.projectRoot, 'dist'), - projectRoot: options.projectRoot, - rootDir: join(options.projectRoot, 'src'), - generateExportsField: true, - packageJsonPath, - format: ['esm'], - }); + if (options.isUsingTsSolutionConfig) { + if (options.bundler === 'none') { + packageJson.type = 'module'; + } else if (options.bundler !== 'vite' && options.bundler !== 'rollup') { + packageJson = getUpdatedPackageJsonContent(packageJson, { + main: join(options.projectRoot, 'src/index.ts'), + outputPath: joinPathFragments(options.projectRoot, 'dist'), + projectRoot: options.projectRoot, + rootDir: join(options.projectRoot, 'src'), + generateExportsField: true, + packageJsonPath, + format: ['esm'], + }); + } } writeJson(tree, packageJsonPath, packageJson); diff --git a/packages/js/src/generators/typescript-sync/typescript-sync.spec.ts b/packages/js/src/generators/typescript-sync/typescript-sync.spec.ts index 97ea3b33f5..343522cba9 100644 --- a/packages/js/src/generators/typescript-sync/typescript-sync.spec.ts +++ b/packages/js/src/generators/typescript-sync/typescript-sync.spec.ts @@ -159,16 +159,13 @@ describe('syncGenerator()', () => { ); updateJson(tree, 'packages/c/tsconfig.json', (json) => ({ ...json, - references: [{ path: '../b' }, { path: '../a' }], + references: [{ path: '../b' }], })); writeJson(tree, 'packages/c/tsconfig.lib.json', { compilerOptions: { composite: true, }, - references: [ - { path: '../b/tsconfig.lib.json' }, - { path: '../a/tsconfig.lib.json' }, - ], + references: [{ path: '../b/tsconfig.lib.json' }], }); updateJson(tree, 'packages/d/tsconfig.json', (json) => ({ ...json, @@ -185,17 +182,13 @@ describe('syncGenerator()', () => { }); updateJson(tree, 'packages/e/tsconfig.json', (json) => ({ ...json, - references: [{ path: '../b' }, { path: '../d' }, { path: '../a' }], + references: [{ path: '../d' }], })); writeJson(tree, 'packages/e/tsconfig.lib.json', { compilerOptions: { composite: true, }, - references: [ - { path: '../b/tsconfig.lib.json' }, - { path: '../d/tsconfig.lib.json' }, - { path: '../a/tsconfig.lib.json' }, - ], + references: [{ path: '../d/tsconfig.lib.json' }], }); const changesBeforeSyncing = tree .listChanges() @@ -500,7 +493,8 @@ describe('syncGenerator()', () => { `); }); - it('should collect transitive dependencies and sync project references to tsconfig.json files', async () => { + it('should collect transitive dependencies and sync project references to tsconfig.json files when NX_ENABLE_TS_SYNC_TRANSITIVE_DEPENDENCIES=true', async () => { + process.env.NX_ENABLE_TS_SYNC_TRANSITIVE_DEPENDENCIES = 'true'; // c => b => a // d => b => a // => a @@ -558,6 +552,8 @@ describe('syncGenerator()', () => { }, ] `); + + delete process.env.NX_ENABLE_TS_SYNC_TRANSITIVE_DEPENDENCIES; }); it('should leave comments outside of references untouched in the tsconfig.json when patching', async () => { @@ -835,9 +831,6 @@ describe('syncGenerator()', () => { expect(readJson(tree, 'packages/c/tsconfig.json').references) .toMatchInlineSnapshot(` [ - { - "path": "../a", - }, { "path": "../b", }, @@ -846,9 +839,6 @@ describe('syncGenerator()', () => { expect(readJson(tree, 'packages/c/tsconfig.cjs.json').references) .toMatchInlineSnapshot(` [ - { - "path": "../a/tsconfig.lib.json", - }, { "path": "../b/tsconfig.build.json", }, @@ -857,9 +847,6 @@ describe('syncGenerator()', () => { expect(readJson(tree, 'packages/c/tsconfig.esm.json').references) .toMatchInlineSnapshot(` [ - { - "path": "../a/tsconfig.lib.json", - }, { "path": "../b/tsconfig.build.json", }, @@ -892,12 +879,6 @@ describe('syncGenerator()', () => { expect(readJson(tree, 'packages/e/tsconfig.json').references) .toMatchInlineSnapshot(` [ - { - "path": "../a", - }, - { - "path": "../b", - }, { "path": "../c", }, @@ -906,12 +887,6 @@ describe('syncGenerator()', () => { expect(readJson(tree, 'packages/e/tsconfig.cjs.json').references) .toMatchInlineSnapshot(` [ - { - "path": "../a/tsconfig.lib.json", - }, - { - "path": "../b/tsconfig.build.json", - }, { "path": "../c/tsconfig.cjs.json", }, @@ -920,12 +895,6 @@ describe('syncGenerator()', () => { expect(readJson(tree, 'packages/e/tsconfig.esm.json').references) .toMatchInlineSnapshot(` [ - { - "path": "../a/tsconfig.lib.json", - }, - { - "path": "../b/tsconfig.build.json", - }, { "path": "../c/tsconfig.esm.json", }, @@ -935,12 +904,6 @@ describe('syncGenerator()', () => { expect(readJson(tree, 'packages/f/tsconfig.json').references) .toMatchInlineSnapshot(` [ - { - "path": "../a", - }, - { - "path": "../b", - }, { "path": "../c", }, @@ -950,12 +913,6 @@ describe('syncGenerator()', () => { expect(readJson(tree, 'packages/f/tsconfig.runtime.json').references) .toMatchInlineSnapshot(` [ - { - "path": "../a/tsconfig.lib.json", - }, - { - "path": "../b/tsconfig.build.json", - }, { "path": "../c/tsconfig.cjs.json", }, @@ -972,8 +929,9 @@ describe('syncGenerator()', () => { ${'tsconfig.esm.json'} ${'tsconfig.runtime.json'} `( - 'should collect transitive dependencies and sync project references to $runtimeTsConfigFileName files', + 'should collect transitive dependencies and sync project references to $runtimeTsConfigFileName files when NX_ENABLE_TS_SYNC_TRANSITIVE_DEPENDENCIES=true', async ({ runtimeTsConfigFileName }) => { + process.env.NX_ENABLE_TS_SYNC_TRANSITIVE_DEPENDENCIES = 'true'; writeJson(tree, `packages/a/${runtimeTsConfigFileName}`, { compilerOptions: { composite: true, @@ -1092,6 +1050,7 @@ describe('syncGenerator()', () => { }, ] `); + delete process.env.NX_ENABLE_TS_SYNC_TRANSITIVE_DEPENDENCIES; } ); @@ -1327,9 +1286,6 @@ describe('syncGenerator()', () => { expect(readJson(tree, 'packages/c/tsconfig.json').references) .toMatchInlineSnapshot(` [ - { - "path": "../a", - }, { "path": "../b", }, @@ -1338,9 +1294,6 @@ describe('syncGenerator()', () => { expect(readJson(tree, 'packages/c/tsconfig.custom-cjs.json').references) .toMatchInlineSnapshot(` [ - { - "path": "../a/tsconfig.custom.json", - }, { "path": "../b/tsconfig.custom-build.json", }, @@ -1349,9 +1302,6 @@ describe('syncGenerator()', () => { expect(readJson(tree, 'packages/c/tsconfig.custom-esm.json').references) .toMatchInlineSnapshot(` [ - { - "path": "../a/tsconfig.custom.json", - }, { "path": "../b/tsconfig.custom-build.json", }, @@ -1385,12 +1335,6 @@ describe('syncGenerator()', () => { expect(readJson(tree, 'packages/e/tsconfig.json').references) .toMatchInlineSnapshot(` [ - { - "path": "../a", - }, - { - "path": "../b", - }, { "path": "../c", }, @@ -1399,12 +1343,6 @@ describe('syncGenerator()', () => { expect(readJson(tree, 'packages/e/tsconfig.custom-cjs.json').references) .toMatchInlineSnapshot(` [ - { - "path": "../a/tsconfig.custom.json", - }, - { - "path": "../b/tsconfig.custom-build.json", - }, { "path": "../c/tsconfig.custom-cjs.json", }, @@ -1413,12 +1351,6 @@ describe('syncGenerator()', () => { expect(readJson(tree, 'packages/e/tsconfig.custom-esm.json').references) .toMatchInlineSnapshot(` [ - { - "path": "../a/tsconfig.custom.json", - }, - { - "path": "../b/tsconfig.custom-build.json", - }, { "path": "../c/tsconfig.custom-esm.json", }, @@ -1428,12 +1360,6 @@ describe('syncGenerator()', () => { expect(readJson(tree, 'packages/f/tsconfig.json').references) .toMatchInlineSnapshot(` [ - { - "path": "../a", - }, - { - "path": "../b", - }, { "path": "../c", }, @@ -1444,12 +1370,6 @@ describe('syncGenerator()', () => { readJson(tree, 'packages/f/tsconfig.custom-runtime.json').references ).toMatchInlineSnapshot(` [ - { - "path": "../a/tsconfig.custom.json", - }, - { - "path": "../b/tsconfig.custom-build.json", - }, { "path": "../c/tsconfig.custom-cjs.json", }, @@ -1512,9 +1432,6 @@ describe('syncGenerator()', () => { expect(readJson(tree, 'packages/c/tsconfig.json').references) .toMatchInlineSnapshot(` [ - { - "path": "../a", - }, { "path": "../b", }, @@ -1523,9 +1440,6 @@ describe('syncGenerator()', () => { expect(readJson(tree, 'packages/c/tsconfig.custom.json').references) .toMatchInlineSnapshot(` [ - { - "path": "../a/tsconfig.custom.json", - }, { "path": "../b/tsconfig.custom.json", }, @@ -1556,12 +1470,6 @@ describe('syncGenerator()', () => { expect(readJson(tree, 'packages/e/tsconfig.json').references) .toMatchInlineSnapshot(` [ - { - "path": "../a", - }, - { - "path": "../b", - }, { "path": "../d", }, @@ -1570,12 +1478,6 @@ describe('syncGenerator()', () => { expect(readJson(tree, 'packages/e/tsconfig.custom.json').references) .toMatchInlineSnapshot(` [ - { - "path": "../a/tsconfig.custom.json", - }, - { - "path": "../b/tsconfig.custom.json", - }, { "path": "../d/tsconfig.custom.json", }, diff --git a/packages/js/src/generators/typescript-sync/typescript-sync.ts b/packages/js/src/generators/typescript-sync/typescript-sync.ts index b3fbd9a5eb..b26880437f 100644 --- a/packages/js/src/generators/typescript-sync/typescript-sync.ts +++ b/packages/js/src/generators/typescript-sync/typescript-sync.ts @@ -505,7 +505,7 @@ function collectProjectDependencies( collectedDependencies.get(projectName).push(targetProjectNode); } - if (process.env.NX_DISABLE_TS_SYNC_TRANSITIVE_DEPENDENCIES === 'true') { + if (process.env.NX_ENABLE_TS_SYNC_TRANSITIVE_DEPENDENCIES !== 'true') { continue; }