From 8ddd697a07f32a018f24d4a85920779b6c08815c Mon Sep 17 00:00:00 2001 From: Ashkan Date: Wed, 11 Jun 2025 16:06:49 +0200 Subject: [PATCH] fix(bundling): correctly handle .cjs.js .mjs.js in rollup for type definitions (#29366) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit …le extensions for type definitions. Updated the Rollup plugin's logic for generating type definition files to ensure compatibility with additional file extensions, including .cjs.js and .mjs.js. This change improves the handling of entry points and ensures that corresponding .d.ts files are correctly named and emitted in all supported scenarios. Added a comprehensive test case to validate the new behavior. closed #29308 ## Current Behavior ## Expected Behavior ## Related Issue(s) Fixes # --------- Co-authored-by: Colum Ferry --- ...-publishable-libraries-ts-solution.test.ts | 2 +- .../src/release-publishable-libraries.test.ts | 2 +- e2e/rollup/src/rollup-legacy.test.ts | 12 ++-- e2e/rollup/src/rollup.test.ts | 12 ++-- .../plugins/rollup/type-definitions.spec.ts | 71 +++++++++++++++++++ .../js/src/plugins/rollup/type-definitions.ts | 9 ++- .../package-json/update-package-json.spec.ts | 28 ++++---- .../package-json/update-package-json.ts | 7 +- 8 files changed, 111 insertions(+), 32 deletions(-) create mode 100644 packages/js/src/plugins/rollup/type-definitions.spec.ts diff --git a/e2e/release/src/release-publishable-libraries-ts-solution.test.ts b/e2e/release/src/release-publishable-libraries-ts-solution.test.ts index c63fe882ac..c79f63fe90 100644 --- a/e2e/release/src/release-publishable-libraries-ts-solution.test.ts +++ b/e2e/release/src/release-publishable-libraries-ts-solution.test.ts @@ -168,8 +168,8 @@ describe('release publishable libraries in workspace with ts solution setup', () 📦 @proj/{project-name}@0.0.3 === Tarball Contents === XXB README.md + XXB dist/index.d.ts XXB dist/index.esm.css - XXB dist/index.esm.d.ts XXB dist/index.esm.js XXX.XXX kb dist/README.md XXB dist/src/index.d.ts diff --git a/e2e/release/src/release-publishable-libraries.test.ts b/e2e/release/src/release-publishable-libraries.test.ts index 888af9994c..dacfb67098 100644 --- a/e2e/release/src/release-publishable-libraries.test.ts +++ b/e2e/release/src/release-publishable-libraries.test.ts @@ -170,8 +170,8 @@ describe('release publishable libraries', () => { 📦 @proj/{project-name}@0.0.3 === Tarball Contents === XXX.XXX kb README.md + XXB index.d.ts XXB index.esm.css - XXB index.esm.d.ts XXB index.esm.js XXXB package.json XXB src/index.d.ts diff --git a/e2e/rollup/src/rollup-legacy.test.ts b/e2e/rollup/src/rollup-legacy.test.ts index d93e98580e..b7fec5ec9f 100644 --- a/e2e/rollup/src/rollup-legacy.test.ts +++ b/e2e/rollup/src/rollup-legacy.test.ts @@ -41,13 +41,13 @@ describe('Rollup Plugin', () => { ); rmDist(); runCLI(`build ${myPkg} --format=cjs,esm --generateExportsField`); - checkFilesExist(`dist/libs/${myPkg}/index.cjs.d.ts`); + checkFilesExist(`dist/libs/${myPkg}/index.d.ts`); expect(readJson(`dist/libs/${myPkg}/package.json`).exports).toEqual({ '.': { module: './index.esm.js', import: './index.cjs.mjs', default: './index.cjs.js', - types: './index.esm.d.ts', + types: './index.d.ts', }, './package.json': './package.json', }); @@ -106,7 +106,7 @@ describe('Rollup Plugin', () => { checkFilesExist(`dist/libs/${myPkg}/index.esm.js`); checkFilesExist(`dist/libs/${myPkg}/index.cjs.js`); - checkFilesExist(`dist/libs/${myPkg}/index.cjs.d.ts`); + checkFilesExist(`dist/libs/${myPkg}/index.d.ts`); checkFilesExist(`dist/libs/${myPkg}/foo.esm.js`); checkFilesExist(`dist/libs/${myPkg}/foo.cjs.js`); checkFilesExist(`dist/libs/${myPkg}/bar.esm.js`); @@ -117,19 +117,19 @@ describe('Rollup Plugin', () => { module: './index.esm.js', import: './index.cjs.mjs', default: './index.cjs.js', - types: './index.esm.d.ts', + types: './index.d.ts', }, './bar': { module: './bar.esm.js', import: './bar.cjs.mjs', default: './bar.cjs.js', - types: './bar.esm.d.ts', + types: './bar.d.ts', }, './foo': { module: './foo.esm.js', import: './foo.cjs.mjs', default: './foo.cjs.js', - types: './foo.esm.d.ts', + types: './foo.d.ts', }, }); }); diff --git a/e2e/rollup/src/rollup.test.ts b/e2e/rollup/src/rollup.test.ts index bce8c2ca4c..5411baae18 100644 --- a/e2e/rollup/src/rollup.test.ts +++ b/e2e/rollup/src/rollup.test.ts @@ -43,13 +43,13 @@ describe('Rollup Plugin', () => { ); rmDist(); runCLI(`build ${myPkg}`); - checkFilesExist(`dist/libs/${myPkg}/index.cjs.d.ts`); + checkFilesExist(`dist/libs/${myPkg}/index.d.ts`); expect(readJson(`dist/libs/${myPkg}/package.json`).exports).toEqual({ '.': { module: './index.esm.js', import: './index.cjs.mjs', default: './index.cjs.js', - types: './index.esm.d.ts', + types: './index.d.ts', }, './package.json': './package.json', }); @@ -139,7 +139,7 @@ describe('Rollup Plugin', () => { checkFilesExist(`dist/libs/${myPkg}/index.esm.js.map`); checkFilesExist(`dist/libs/${myPkg}/index.cjs.js`); checkFilesExist(`dist/libs/${myPkg}/index.cjs.js.map`); - checkFilesExist(`dist/libs/${myPkg}/index.cjs.d.ts`); + checkFilesExist(`dist/libs/${myPkg}/index.d.ts`); checkFilesExist(`dist/libs/${myPkg}/foo.esm.js`); checkFilesExist(`dist/libs/${myPkg}/foo.esm.js.map`); checkFilesExist(`dist/libs/${myPkg}/foo.cjs.js`); @@ -154,19 +154,19 @@ describe('Rollup Plugin', () => { module: './index.esm.js', import: './index.cjs.mjs', default: './index.cjs.js', - types: './index.esm.d.ts', + types: './index.d.ts', }, './bar': { module: './bar.esm.js', import: './bar.cjs.mjs', default: './bar.cjs.js', - types: './bar.esm.d.ts', + types: './bar.d.ts', }, './foo': { module: './foo.esm.js', import: './foo.cjs.mjs', default: './foo.cjs.js', - types: './foo.esm.d.ts', + types: './foo.d.ts', }, }); }); diff --git a/packages/js/src/plugins/rollup/type-definitions.spec.ts b/packages/js/src/plugins/rollup/type-definitions.spec.ts new file mode 100644 index 0000000000..c135071da4 --- /dev/null +++ b/packages/js/src/plugins/rollup/type-definitions.spec.ts @@ -0,0 +1,71 @@ +import { typeDefinitions } from './type-definitions'; +describe('typeDefinitions', () => { + it('should emit correct .d.ts filenames for various file formats', () => { + const mockBundle = { + 'index.js': { + type: 'chunk', + isEntry: true, + fileName: 'index.js', + facadeModuleId: '/project/src/index.ts', + exports: ['default', 'namedExport1', 'namedExport2'], + }, + 'index1.js': { + type: 'chunk', + isEntry: true, + fileName: 'index.cjs', + facadeModuleId: '/project/src/index.ts', + exports: ['default', 'namedExport1', 'namedExport2'], + }, + 'index2.js': { + type: 'chunk', + isEntry: true, + fileName: 'index.mjs', + facadeModuleId: '/project/src/index.ts', + exports: ['default', 'namedExport1', 'namedExport2'], + }, + 'index3.js': { + type: 'chunk', + isEntry: true, + fileName: 'index.cjs.js', + facadeModuleId: '/project/src/index.ts', + exports: ['default', 'namedExport1', 'namedExport2'], + }, + 'index4.js': { + type: 'chunk', + isEntry: true, + fileName: 'index.mjs.js', + facadeModuleId: '/project/src/index.ts', + exports: ['default', 'namedExport1', 'namedExport2'], + }, + }; + + const mockOpts = {}; // Can be left empty for this scenario + + const mockEmitFile = jest.fn(); + + const plugin = typeDefinitions({ projectRoot: '/project' }); + + // Simulate the `this` context of a Rollup plugin + const mockContext = { + emitFile: mockEmitFile, + }; + + // Run the plugin's `generateBundle` function + (async function testPlugin() { + await plugin.generateBundle.call(mockContext, mockOpts, mockBundle); + + // Verify the correct .d.ts filenames are generated for different file formats + const expectedFileNames = [ + 'index.d.ts', // from index.js + 'index.d.ts', // from index.cjs + 'index.d.ts', // from index.mjs + 'index.d.ts', // from index.cjs.js + 'index.d.ts', // from index.mjs.js + ]; + + mockEmitFile.mock.calls.forEach(([{ fileName }], index) => { + expect(fileName).toBe(expectedFileNames[index]); + }); + })(); + }); +}); diff --git a/packages/js/src/plugins/rollup/type-definitions.ts b/packages/js/src/plugins/rollup/type-definitions.ts index f82ddf505b..a2a2a604f6 100644 --- a/packages/js/src/plugins/rollup/type-definitions.ts +++ b/packages/js/src/plugins/rollup/type-definitions.ts @@ -38,7 +38,14 @@ export function typeDefinitions(options: { projectRoot: string }) { /\.[cm]?[jt]sx?$/, '' ); - const dtsFileName = file.fileName.replace(/\.[cm]?js$/, '.d.ts'); + + // Replace various JavaScript file extensions (e.g., .js, .cjs, .mjs, .cjs.js, .mjs.js) with .d.ts for generating type definition file names. + // This regex matches the pattern used in packages/rollup/src/plugins/package-json/update-package-json.ts + const dtsFileName = file.fileName.replace( + /(\.cjs|\.mjs|\.esm\.js|\.cjs\.js|\.mjs\.js|\.js)$/, + '.d.ts' + ); + const relativeSourceDtsName = JSON.stringify('./' + entrySourceDtsName); const dtsFileSource = hasDefaultExport ? stripIndents` diff --git a/packages/rollup/src/plugins/package-json/update-package-json.spec.ts b/packages/rollup/src/plugins/package-json/update-package-json.spec.ts index c10f92a926..c617498aab 100644 --- a/packages/rollup/src/plugins/package-json/update-package-json.spec.ts +++ b/packages/rollup/src/plugins/package-json/update-package-json.spec.ts @@ -32,13 +32,13 @@ describe('updatePackageJson', () => { './package.json': './package.json', '.': { import: './index.esm.js', - types: './index.esm.d.ts', + types: './index.d.ts', }, }, main: './index.esm.js', module: './index.esm.js', type: 'module', - types: './index.esm.d.ts', + types: './index.d.ts', }); spy.mockRestore(); @@ -63,7 +63,7 @@ describe('updatePackageJson', () => { }, main: './index.cjs.js', type: 'commonjs', - types: './index.cjs.d.ts', + types: './index.d.ts', }); spy.mockRestore(); @@ -88,12 +88,12 @@ describe('updatePackageJson', () => { module: './index.esm.js', import: './index.cjs.mjs', default: './index.cjs.js', - types: './index.esm.d.ts', + types: './index.d.ts', }, }, main: './index.cjs.js', module: './index.esm.js', - types: './index.esm.d.ts', + types: './index.d.ts', }); spy.mockRestore(); @@ -123,7 +123,7 @@ describe('updatePackageJson', () => { './package.json': './package.json', '.': { import: './index.esm.js', - types: './index.esm.d.ts', + types: './index.d.ts', }, './foo': { import: './some/custom/path/foo.esm.js', @@ -133,7 +133,7 @@ describe('updatePackageJson', () => { main: './index.esm.js', module: './index.esm.js', type: 'module', - types: './index.esm.d.ts', + types: './index.d.ts', }); spy.mockRestore(); @@ -156,7 +156,7 @@ describe('updatePackageJson', () => { main: './index.esm.js', module: './index.esm.js', type: 'module', - types: './index.esm.d.ts', + types: './index.d.ts', }); spy.mockRestore(); @@ -176,7 +176,7 @@ describe('updatePackageJson', () => { expect(utils.writeJsonFile).toHaveBeenCalledWith(expect.anything(), { main: './index.cjs.js', type: 'commonjs', - types: './index.cjs.d.ts', + types: './index.d.ts', }); spy.mockRestore(); @@ -196,7 +196,7 @@ describe('updatePackageJson', () => { expect(utils.writeJsonFile).toHaveBeenCalledWith(expect.anything(), { main: './index.cjs.js', module: './index.esm.js', - types: './index.esm.d.ts', + types: './index.d.ts', }); spy.mockRestore(); @@ -221,7 +221,7 @@ describe('updatePackageJson', () => { main: './index.esm.js', module: './index.esm.js', type: 'module', - types: './index.esm.d.ts', + types: './index.d.ts', exports: { './foo': './foo.esm.js', }, @@ -251,7 +251,7 @@ describe('updatePackageJson', () => { expect(utils.writeJsonFile).toHaveBeenCalledWith(expect.anything(), { main: './index.esm.js', module: './index.esm.js', - types: './index.esm.d.ts', + types: './index.d.ts', exports: { './foo': './foo.esm.js', }, @@ -279,7 +279,7 @@ describe('updatePackageJson', () => { main: './index.esm.js', module: './index.esm.js', type: 'module', - types: './index.esm.d.ts', + types: './index.d.ts', exports: { './foo': './foo.esm.js', }, @@ -308,7 +308,7 @@ describe('updatePackageJson', () => { main: './index.esm.js', module: './index.esm.js', type: 'module', - types: './index.esm.d.ts', + types: './index.d.ts', exports: { './foo': './foo.esm.js', }, diff --git a/packages/rollup/src/plugins/package-json/update-package-json.ts b/packages/rollup/src/plugins/package-json/update-package-json.ts index ce5cf85295..bf20c9fcfc 100644 --- a/packages/rollup/src/plugins/package-json/update-package-json.ts +++ b/packages/rollup/src/plugins/package-json/update-package-json.ts @@ -16,6 +16,7 @@ export function updatePackageJson( }, packageJson: PackageJson ) { + const jsFileRegex = /(\.cjs|\.mjs|\.esm\.js|\.cjs\.js|\.mjs\.js|\.js)$/; const hasEsmFormat = options.format.includes('esm'); const hasCjsFormat = options.format.includes('cjs'); @@ -46,7 +47,7 @@ export function updatePackageJson( // Reserve `module` entry for bundlers to accommodate tree-shaking. { [hasCjsFormat ? 'module' : 'import']: filePath, - types: filePath.replace(/\.js$/, '.d.ts'), + types: filePath.replace(jsFileRegex, '.d.ts'), }; } } @@ -110,9 +111,9 @@ export function updatePackageJson( } if (packageJson.module) { - packageJson.types ??= packageJson.module.replace(/\.js$/, '.d.ts'); + packageJson.types ??= packageJson.module.replace(jsFileRegex, '.d.ts'); } else { - packageJson.types ??= packageJson.main.replace(/\.js$/, '.d.ts'); + packageJson.types ??= packageJson.main.replace(jsFileRegex, '.d.ts'); } writeJsonFile(