From edcbadf5eccf3ce3d8cc2210e3813121924e94ef Mon Sep 17 00:00:00 2001 From: Jack Hsu Date: Thu, 27 Jul 2023 08:39:04 -0400 Subject: [PATCH] feat(bundling): generate matching d.ts files for rollup --- e2e/rollup/src/rollup.test.ts | 19 ++++-- .../rollup/lib/update-package-json.spec.ts | 60 +++++++----------- .../rollup/lib/update-package-json.ts | 28 +++------ .../executors/rollup/lib/validate-types.ts | 20 ------ .../src/executors/rollup/rollup.impl.spec.ts | 8 +-- .../src/executors/rollup/rollup.impl.ts | 39 ++++++------ .../rollup/src/executors/rollup/schema.d.ts | 2 +- .../rollup/src/plugins/type-definitions.ts | 62 +++++++++++++++++++ 8 files changed, 136 insertions(+), 102 deletions(-) delete mode 100644 packages/rollup/src/executors/rollup/lib/validate-types.ts create mode 100644 packages/rollup/src/plugins/type-definitions.ts diff --git a/e2e/rollup/src/rollup.test.ts b/e2e/rollup/src/rollup.test.ts index 704923b7d3d4ec..e858862e2d7f25 100644 --- a/e2e/rollup/src/rollup.test.ts +++ b/e2e/rollup/src/rollup.test.ts @@ -1,6 +1,8 @@ import { + checkFilesExist, cleanupProject, newProject, + readJson, rmDist, runCLI, runCommand, @@ -23,8 +25,17 @@ describe('Rollup Plugin', () => { `generate @nx/rollup:configuration ${myPkg} --target=node --tsConfig=libs/${myPkg}/tsconfig.lib.json --main=libs/${myPkg}/src/index.ts` ); rmDist(); - runCLI(`build ${myPkg}`); - let output = runCommand(`node dist/libs/${myPkg}/index.cjs`); + runCLI(`build ${myPkg} --format=cjs,esm --generateExportsField`); + checkFilesExist(`dist/libs/${myPkg}/index.cjs.d.ts`); + expect(readJson(`dist/libs/${myPkg}/package.json`).exports).toMatchObject({ + exports: { + '.': { + import: './index.esm.js', + default: './index.cjs.js', + }, + }, + }); + let output = runCommand(`node dist/libs/${myPkg}/index.cjs.js`); expect(output).toMatch(/Hello/); updateProjectConfig(myPkg, (config) => { @@ -38,7 +49,7 @@ describe('Rollup Plugin', () => { ); rmDist(); runCLI(`build ${myPkg}`); - output = runCommand(`node dist/libs/${myPkg}/index.cjs`); + output = runCommand(`node dist/libs/${myPkg}/index.cjs.js`); expect(output).toMatch(/Hello/); updateProjectConfig(myPkg, (config) => { @@ -52,7 +63,7 @@ describe('Rollup Plugin', () => { ); rmDist(); runCLI(`build ${myPkg}`); - output = runCommand(`node dist/libs/${myPkg}/index.cjs`); + output = runCommand(`node dist/libs/${myPkg}/index.cjs.js`); expect(output).toMatch(/Hello/); }, 500000); diff --git a/packages/rollup/src/executors/rollup/lib/update-package-json.spec.ts b/packages/rollup/src/executors/rollup/lib/update-package-json.spec.ts index e783760c55b88b..3a0eb410eb5fa9 100644 --- a/packages/rollup/src/executors/rollup/lib/update-package-json.spec.ts +++ b/packages/rollup/src/executors/rollup/lib/update-package-json.spec.ts @@ -42,14 +42,12 @@ describe('updatePackageJson', () => { expect(utils.writeJsonFile).toHaveBeenCalledWith(expect.anything(), { exports: { '.': { - types: './index.d.ts', - import: './index.js', + import: './index.esm.js', }, }, - main: './index.js', - module: './index.js', + main: './index.esm.js', + module: './index.esm.js', type: 'module', - types: './index.d.ts', }); spy.mockRestore(); @@ -73,13 +71,11 @@ describe('updatePackageJson', () => { expect(utils.writeJsonFile).toHaveBeenCalledWith(expect.anything(), { exports: { '.': { - types: './index.d.ts', - require: './index.cjs', + default: './index.cjs.js', }, }, - main: './index.cjs', + main: './index.cjs.js', type: 'commonjs', - types: './index.d.ts', }); spy.mockRestore(); @@ -103,15 +99,12 @@ describe('updatePackageJson', () => { expect(utils.writeJsonFile).toHaveBeenCalledWith(expect.anything(), { exports: { '.': { - types: './index.d.ts', - import: './index.js', - require: './index.cjs', + import: './index.esm.js', + default: './index.cjs.js', }, }, - main: './index.cjs', - module: './index.js', - type: 'module', - types: './index.d.ts', + main: './index.cjs.js', + module: './index.esm.js', }); spy.mockRestore(); @@ -132,7 +125,7 @@ describe('updatePackageJson', () => { { exports: { './foo': { - import: './foo.js', + import: './foo.esm.js', }, }, } as unknown as PackageJson @@ -141,17 +134,15 @@ describe('updatePackageJson', () => { expect(utils.writeJsonFile).toHaveBeenCalledWith(expect.anything(), { exports: { '.': { - types: './index.d.ts', - import: './index.js', + import: './index.esm.js', }, './foo': { - import: './foo.js', + import: './foo.esm.js', }, }, - main: './index.js', - module: './index.js', + main: './index.esm.js', + module: './index.esm.js', type: 'module', - types: './index.d.ts', }); spy.mockRestore(); @@ -174,10 +165,9 @@ describe('updatePackageJson', () => { ); expect(utils.writeJsonFile).toHaveBeenCalledWith(expect.anything(), { - main: './index.js', - module: './index.js', + main: './index.esm.js', + module: './index.esm.js', type: 'module', - types: './index.d.ts', }); spy.mockRestore(); @@ -198,9 +188,8 @@ describe('updatePackageJson', () => { ); expect(utils.writeJsonFile).toHaveBeenCalledWith(expect.anything(), { - main: './index.cjs', + main: './index.cjs.js', type: 'commonjs', - types: './index.d.ts', }); spy.mockRestore(); @@ -221,10 +210,8 @@ describe('updatePackageJson', () => { ); expect(utils.writeJsonFile).toHaveBeenCalledWith(expect.anything(), { - main: './index.cjs', - module: './index.js', - type: 'module', - types: './index.d.ts', + main: './index.cjs.js', + module: './index.esm.js', }); spy.mockRestore(); @@ -244,20 +231,19 @@ describe('updatePackageJson', () => { { exports: { './foo': { - import: './foo.js', + import: './foo.esm.js', }, }, } as unknown as PackageJson ); expect(utils.writeJsonFile).toHaveBeenCalledWith(expect.anything(), { - main: './index.js', - module: './index.js', + main: './index.esm.js', + module: './index.esm.js', type: 'module', - types: './index.d.ts', exports: { './foo': { - import: './foo.js', + import: './foo.esm.js', }, }, }); diff --git a/packages/rollup/src/executors/rollup/lib/update-package-json.ts b/packages/rollup/src/executors/rollup/lib/update-package-json.ts index 79843aa988085a..e610a13bdae534 100644 --- a/packages/rollup/src/executors/rollup/lib/update-package-json.ts +++ b/packages/rollup/src/executors/rollup/lib/update-package-json.ts @@ -9,6 +9,7 @@ import { writeJsonFile } from 'nx/src/utils/fileutils'; import { PackageJson } from 'nx/src/utils/package-json'; import { NormalizedRollupExecutorOptions } from './normalize'; +// TODO(jack): Use updatePackageJson from @nx/js instead. export function updatePackageJson( options: NormalizedRollupExecutorOptions, context: ExecutorContext, @@ -19,40 +20,31 @@ export function updatePackageJson( const hasEsmFormat = options.format.includes('esm'); const hasCjsFormat = options.format.includes('cjs'); - const types = `./${relative(options.projectRoot, options.main).replace( - /\.[jt]sx?$/, - '.d.ts' - )}`; const exports = { - // TS 4.5+ - '.': { - types, - }, + '.': {}, }; if (hasEsmFormat) { // `module` field is used by bundlers like rollup and webpack to detect ESM. // May not be required in the future if type is already "module". - packageJson.module = './index.js'; - exports['.']['import'] = './index.js'; + packageJson.module = './index.esm.js'; + exports['.']['import'] = './index.esm.js'; if (!hasCjsFormat) { - packageJson.main = './index.js'; + packageJson.main = './index.esm.js'; } } if (hasCjsFormat) { - packageJson.main = './index.cjs'; - exports['.']['require'] = './index.cjs'; + packageJson.main = './index.cjs.js'; + exports['.']['default'] = './index.cjs.js'; } - if (!options.skipTypeField) { + + // Dual format should not specify `type` field, the `exports` field resolves ESM vs CJS. + if (!options.skipTypeField && options.format.length === 1) { packageJson.type = options.format.includes('esm') ? 'module' : 'commonjs'; } - // Support for older TS versions < 4.5 - packageJson.types = types; - - // TODO(jack): remove this for Nx 16 if (options.generateExportsField && typeof packageJson.exports !== 'string') { packageJson.exports = { ...packageJson.exports, diff --git a/packages/rollup/src/executors/rollup/lib/validate-types.ts b/packages/rollup/src/executors/rollup/lib/validate-types.ts deleted file mode 100644 index 59628f2f316e70..00000000000000 --- a/packages/rollup/src/executors/rollup/lib/validate-types.ts +++ /dev/null @@ -1,20 +0,0 @@ -import { printDiagnostics, runTypeCheck } from '@nx/js'; -import { join } from 'path'; - -export async function validateTypes(opts: { - workspaceRoot: string; - projectRoot: string; - tsconfig: string; -}): Promise { - const result = await runTypeCheck({ - workspaceRoot: opts.workspaceRoot, - tsConfigPath: join(opts.workspaceRoot, opts.tsconfig), - mode: 'noEmit', - }); - - await printDiagnostics(result.errors, result.warnings); - - if (result.errors.length > 0) { - throw new Error('Found type errors. See above.'); - } -} diff --git a/packages/rollup/src/executors/rollup/rollup.impl.spec.ts b/packages/rollup/src/executors/rollup/rollup.impl.spec.ts index 168af2cbbf89b5..2bef343b0dd65e 100644 --- a/packages/rollup/src/executors/rollup/rollup.impl.spec.ts +++ b/packages/rollup/src/executors/rollup/rollup.impl.spec.ts @@ -52,15 +52,15 @@ describe('rollupExecutor', () => { dir: '/root/dist/ui', format: 'esm', name: 'Example', - chunkFileNames: '[name].js', - entryFileNames: '[name].js', + chunkFileNames: '[name].esm.js', + entryFileNames: '[name].esm.js', }, { dir: '/root/dist/ui', format: 'cjs', name: 'Example', - chunkFileNames: '[name].cjs', - entryFileNames: '[name].cjs', + chunkFileNames: '[name].cjs.js', + entryFileNames: '[name].cjs.js', }, ]); }); diff --git a/packages/rollup/src/executors/rollup/rollup.impl.ts b/packages/rollup/src/executors/rollup/rollup.impl.ts index a554d383405cae..5c25369097a1a9 100644 --- a/packages/rollup/src/executors/rollup/rollup.impl.ts +++ b/packages/rollup/src/executors/rollup/rollup.impl.ts @@ -26,8 +26,8 @@ import { import { analyze } from './lib/analyze-plugin'; import { deleteOutputDir } from '../../utils/fs'; import { swc } from './lib/swc-plugin'; -import { validateTypes } from './lib/validate-types'; import { updatePackageJson } from './lib/update-package-json'; +import { typeDefinitions } from '../../plugins/type-definitions'; export type RollupExecutorEvent = { success: boolean; @@ -85,18 +85,6 @@ export async function* rollupExecutor( const outfile = resolveOutfile(context, options); - if (options.compiler === 'swc') { - try { - await validateTypes({ - workspaceRoot: context.root, - projectRoot: options.projectRoot, - tsconfig: options.tsConfig, - }); - } catch { - return { success: false }; - } - } - if (options.watch) { const watcher = rollup.watch(rollupOptions); return yield* eachValueFrom( @@ -204,6 +192,10 @@ export function createRollupOptions( } return options.format.map((format, idx) => { + // Either we're generating only one format, so we should bundle types + // OR we are generating dual formats, so only bundle types for CJS. + const shouldBundleTypes = options.format.length === 1 || format === 'cjs'; + const plugins = [ copy({ targets: convertCopyAssetsToRollupOptions( @@ -213,7 +205,7 @@ export function createRollupOptions( }), image(), json(), - (useTsc || useBabel) && + (useTsc || shouldBundleTypes) && require('rollup-plugin-typescript2')({ check: !options.skipTypeCheck, tsconfig: options.tsConfig, @@ -225,6 +217,12 @@ export function createRollupOptions( ), }, }), + shouldBundleTypes && + typeDefinitions({ + main: options.main, + compiler: options.compiler, + projectRoot: options.projectRoot, + }), peerDepsExternal({ packageJsonPath: options.project, }), @@ -295,8 +293,8 @@ export function createRollupOptions( format, dir: `${options.outputPath}`, name: names(context.projectName).className, - entryFileNames: `[name].${format === 'esm' ? 'js' : 'cjs'}`, - chunkFileNames: `[name].${format === 'esm' ? 'js' : 'cjs'}`, + entryFileNames: `[name].${format}.js`, + chunkFileNames: `[name].${format}.js`, }, external: (id: string) => { return externalPackages.some( @@ -327,6 +325,9 @@ function createTsCompilerOptions( if (config.options.module === ts.ModuleKind.CommonJS) { compilerOptions['module'] = 'ESNext'; } + if (options.compiler === 'swc') { + compilerOptions['emitDeclarationOnly'] = true; + } return compilerOptions; } @@ -347,7 +348,9 @@ function convertCopyAssetsToRollupOptions( : undefined; } -function readCompatibleFormats(config: ts.ParsedCommandLine) { +function readCompatibleFormats( + config: ts.ParsedCommandLine +): ('cjs' | 'esm')[] { switch (config.options.module) { case ts.ModuleKind.CommonJS: case ts.ModuleKind.UMD: @@ -364,7 +367,7 @@ function resolveOutfile( ) { if (!options.format?.includes('cjs')) return undefined; const { name } = parse(options.outputFileName ?? options.main); - return resolve(context.root, options.outputPath, `${name}.cjs`); + return resolve(context.root, options.outputPath, `${name}.cjs.js`); } export default rollupExecutor; diff --git a/packages/rollup/src/executors/rollup/schema.d.ts b/packages/rollup/src/executors/rollup/schema.d.ts index 9a08e09638f6c7..5fbe33a46f9d9b 100644 --- a/packages/rollup/src/executors/rollup/schema.d.ts +++ b/packages/rollup/src/executors/rollup/schema.d.ts @@ -37,7 +37,7 @@ export interface RollupExecutorOptions { */ buildableProjectDepsInPackageJsonType?: 'dependencies' | 'peerDependencies'; deleteOutputPath?: boolean; - format?: string[]; + format?: ('cjs' | 'esm')[]; compiler?: 'babel' | 'tsc' | 'swc'; javascriptEnabled?: boolean; generateExportsField?: boolean; diff --git a/packages/rollup/src/plugins/type-definitions.ts b/packages/rollup/src/plugins/type-definitions.ts new file mode 100644 index 00000000000000..34340e6ecf17b2 --- /dev/null +++ b/packages/rollup/src/plugins/type-definitions.ts @@ -0,0 +1,62 @@ +import type { OutputBundle, OutputOptions } from 'rollup'; +import { relative } from 'path'; +import { stripIndents } from '@nx/devkit'; + +/* + * This plugin takes all entry-points from the generated bundle and creates a + * bundled version of corresponding d.ts files. + * + * For example, `src/index.ts` generates two corresponding files: + * - `dist/xyz/index.js` + * - `dist/xyz/src/index.d.ts` + * + * We want a third file: `dist/index.d.ts` that re-exports from `src/index.d.ts`. + * That way, when TSC or IDEs look for types, it will find them in the right place. + */ +export function typeDefinitions(options: { + projectRoot: string; + main: string; + compiler: 'babel' | 'swc' | 'tsc'; +}) { + return { + name: 'dts-bundle', + async generateBundle( + _opts: OutputOptions, + bundle: OutputBundle + ): Promise { + for (const [name, file] of Object.entries(bundle)) { + if ( + file.type === 'asset' || + !file.isEntry || + file.facadeModuleId == null + ) { + continue; + } + + const hasDefaultExport = file.exports.includes('default'); + const entrySourceFileName = relative( + options.projectRoot, + file.facadeModuleId + ); + const entrySourceDtsName = entrySourceFileName.replace( + /\.[cm]?[jt]sx?$/, + '' + ); + const dtsFileName = file.fileName.replace(/\.[cm]?js$/, '.d.ts'); + const relativeSourceDtsName = JSON.stringify('./' + entrySourceDtsName); + const dtsFileSource = hasDefaultExport + ? stripIndents` + export * from ${relativeSourceDtsName}; + export { default } from ${relativeSourceDtsName}; + ` + : `export * from ${relativeSourceDtsName};\n`; + + this.emitFile({ + type: 'asset', + fileName: dtsFileName, + source: dtsFileSource, + }); + } + }, + }; +}