Skip to content

Commit

Permalink
fix(bundling): explicitly set types for exports entries in package.js…
Browse files Browse the repository at this point in the history
…on (#27152)

We currently rely on the TS behavior of matching `d.ts` files based on
the `.js` file names. e.g. `foo.js` matches `foo.d.ts`. However, it
isn't working for all tools so we should explicitly set it.

Most modern packages are still setting it even though it is not
technically needed. e.g.
[Nuxt](https://unpkg.com/browse/[email protected]/package.json)

Note: If both ESM and CJS are present, then prefer `*.esm.d.ts` files
since the generated types are in ESM format.

## Current Behavior
`exports` entries are missing `types` field

## Expected Behavior
`exports` entries have `types` field set

## Related Issue(s)
<!-- Please link the issue being fixed so it gets closed when this is
merged. -->

Fixes #18835
  • Loading branch information
jaysoo authored Jul 26, 2024
1 parent 7b9ee39 commit 2d2c0b5
Show file tree
Hide file tree
Showing 4 changed files with 45 additions and 14 deletions.
4 changes: 4 additions & 0 deletions e2e/rollup/src/rollup-legacy.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,7 @@ describe('Rollup Plugin', () => {
module: './index.esm.js',
import: './index.cjs.mjs',
default: './index.cjs.js',
types: './index.esm.d.ts',
},
'./package.json': './package.json',
});
Expand Down Expand Up @@ -111,16 +112,19 @@ describe('Rollup Plugin', () => {
module: './index.esm.js',
import: './index.cjs.mjs',
default: './index.cjs.js',
types: './index.esm.d.ts',
},
'./bar': {
module: './bar.esm.js',
import: './bar.cjs.mjs',
default: './bar.cjs.js',
types: './bar.esm.d.ts',
},
'./foo': {
module: './foo.esm.js',
import: './foo.cjs.mjs',
default: './foo.cjs.js',
types: './foo.esm.d.ts',
},
});
});
Expand Down
4 changes: 4 additions & 0 deletions e2e/rollup/src/rollup.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,7 @@ describe('Rollup Plugin', () => {
module: './index.esm.js',
import: './index.cjs.mjs',
default: './index.cjs.js',
types: './index.esm.d.ts',
},
'./package.json': './package.json',
});
Expand Down Expand Up @@ -142,16 +143,19 @@ describe('Rollup Plugin', () => {
module: './index.esm.js',
import: './index.cjs.mjs',
default: './index.cjs.js',
types: './index.esm.d.ts',
},
'./bar': {
module: './bar.esm.js',
import: './bar.cjs.mjs',
default: './bar.cjs.js',
types: './bar.esm.d.ts',
},
'./foo': {
module: './foo.esm.js',
import: './foo.cjs.mjs',
default: './foo.cjs.js',
types: './foo.esm.d.ts',
},
});
});
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,10 @@ describe('updatePackageJson', () => {
expect(utils.writeJsonFile).toHaveBeenCalledWith(expect.anything(), {
exports: {
'./package.json': './package.json',
'.': './index.esm.js',
'.': {
import: './index.esm.js',
types: './index.esm.d.ts',
},
},
main: './index.esm.js',
module: './index.esm.js',
Expand Down Expand Up @@ -85,11 +88,12 @@ describe('updatePackageJson', () => {
module: './index.esm.js',
import: './index.cjs.mjs',
default: './index.cjs.js',
types: './index.esm.d.ts',
},
},
main: './index.cjs.js',
module: './index.esm.js',
types: './index.cjs.d.ts',
types: './index.esm.d.ts',
});

spy.mockRestore();
Expand All @@ -106,17 +110,25 @@ describe('updatePackageJson', () => {
},
{
exports: {
'./foo': './foo.esm.js',
'./foo': {
import: './some/custom/path/foo.esm.js',
types: './some/custom/path/foo.d.ts',
},
},
} as unknown as PackageJson
);

expect(utils.writeJsonFile).toHaveBeenCalledWith(expect.anything(), {
exports: {
'./package.json': './package.json',
'.': './index.esm.js',

'./foo': './foo.esm.js',
'.': {
import: './index.esm.js',
types: './index.esm.d.ts',
},
'./foo': {
import: './some/custom/path/foo.esm.js',
types: './some/custom/path/foo.d.ts',
},
},
main: './index.esm.js',
module: './index.esm.js',
Expand Down Expand Up @@ -184,7 +196,7 @@ describe('updatePackageJson', () => {
expect(utils.writeJsonFile).toHaveBeenCalledWith(expect.anything(), {
main: './index.cjs.js',
module: './index.esm.js',
types: './index.cjs.d.ts',
types: './index.esm.d.ts',
});

spy.mockRestore();
Expand Down
25 changes: 18 additions & 7 deletions packages/rollup/src/plugins/package-json/update-package-json.ts
Original file line number Diff line number Diff line change
Expand Up @@ -40,12 +40,14 @@ export function updatePackageJson(

if (options.generateExportsField) {
for (const [exportEntry, filePath] of Object.entries(esmExports)) {
packageJson.exports[exportEntry] = hasCjsFormat
? // If CJS format is used, make sure `import` (from Node) points to same instance of the package.
// Otherwise, packages that are required to be singletons (like React, RxJS, etc.) will break.
// Reserve `module` entry for bundlers to accommodate tree-shaking.
{ [hasCjsFormat ? 'module' : 'import']: filePath }
: filePath;
packageJson.exports[exportEntry] =
// If CJS format is used, make sure `import` (from Node) points to same instance of the package.
// Otherwise, packages that are required to be singletons (like React, RxJS, etc.) will break.
// Reserve `module` entry for bundlers to accommodate tree-shaking.
{
[hasCjsFormat ? 'module' : 'import']: filePath,
types: filePath.replace(/\.js$/, '.d.ts'),
};
}
}
}
Expand All @@ -72,6 +74,11 @@ export function updatePackageJson(
const fauxEsmFilePath = filePath.replace(/\.cjs\.js$/, '.cjs.mjs');
packageJson.exports[exportEntry]['import'] ??= fauxEsmFilePath;
packageJson.exports[exportEntry]['default'] ??= filePath;
// Set `types` field only if it's not already set as the preferred ESM Format.
packageJson.exports[exportEntry]['types'] ??= filePath.replace(
/\.js$/,
'.d.ts'
);
// Re-export from relative CJS file, and Node will synthetically export it as ESM.
// Make sure both ESM and CJS point to same instance of the package because libs like React, RxJS, etc. requires it.
// Also need a special .cjs.default.js file that re-exports the `default` from CJS, or else
Expand Down Expand Up @@ -102,7 +109,11 @@ export function updatePackageJson(
}
}

packageJson.types ??= packageJson.main.replace(/\.js$/, '.d.ts');
if (packageJson.module) {
packageJson.types ??= packageJson.module.replace(/\.js$/, '.d.ts');
} else {
packageJson.types ??= packageJson.main.replace(/\.js$/, '.d.ts');
}

writeJsonFile(
join(workspaceRoot, options.outputPath, 'package.json'),
Expand Down

0 comments on commit 2d2c0b5

Please sign in to comment.