feat(bundling): generate matching d.ts files for rollup

nrwl · Jul 27, 2023 · 87ac4a6 · 87ac4a6
1 parent 2dc6656
commit 87ac4a6
Show file tree

Hide file tree

Showing 11 changed files with 146 additions and 114 deletions.
diff --git a/e2e/react-core/src/react-package.test.ts b/e2e/react-core/src/react-package.test.ts
@@ -119,9 +119,9 @@ describe('Build React libraries and apps', () => {
       runCLI(`build ${childLib}`);
       runCLI(`build ${childLib2}`);
 
-      checkFilesExist(`dist/libs/${childLib}/index.js`);
+      checkFilesExist(`dist/libs/${childLib}/index.esm.js`);
 
-      checkFilesExist(`dist/libs/${childLib2}/index.js`);
+      checkFilesExist(`dist/libs/${childLib2}/index.esm.js`);
 
       checkFilesExist(`dist/libs/${childLib}/assets/hello.txt`);
       checkFilesExist(`dist/libs/${childLib2}/README.md`);
@@ -131,7 +131,7 @@ describe('Build React libraries and apps', () => {
        */
       runCLI(`build ${parentLib} --updateBuildableProjectDepsInPackageJson`);
 
-      checkFilesExist(`dist/libs/${parentLib}/index.js`);
+      checkFilesExist(`dist/libs/${parentLib}/index.esm.js`);
 
       const jsonFile = readJson(`dist/libs/${parentLib}/package.json`);
       expect(jsonFile.peerDependencies).toEqual(
@@ -148,14 +148,14 @@ describe('Build React libraries and apps', () => {
 
       runCLI(`build ${parentLib} --skip-nx-cache`);
 
-      checkFilesExist(`dist/libs/${parentLib}/index.js`);
-      checkFilesExist(`dist/libs/${childLib}/index.js`);
-      checkFilesExist(`dist/libs/${childLib2}/index.js`);
+      checkFilesExist(`dist/libs/${parentLib}/index.esm.js`);
+      checkFilesExist(`dist/libs/${childLib}/index.esm.js`);
+      checkFilesExist(`dist/libs/${childLib2}/index.esm.js`);
 
-      expect(readFile(`dist/libs/${childLib}/index.js`)).not.toContain(
+      expect(readFile(`dist/libs/${childLib}/index.esm.js`)).not.toContain(
         'react/jsx-dev-runtime'
       );
-      expect(readFile(`dist/libs/${childLib}/index.js`)).toContain(
+      expect(readFile(`dist/libs/${childLib}/index.esm.js`)).toContain(
         'react/jsx-runtime'
       );
     });
@@ -173,13 +173,13 @@ export async function h() { return 'c'; }
       runCLI(`build ${childLib} --format cjs,esm`);
 
       checkFilesExist(`dist/libs/${childLib}/index.cjs`);
-      checkFilesExist(`dist/libs/${childLib}/index.js`);
+      checkFilesExist(`dist/libs/${childLib}/index.esm.js`);
 
       const cjsPackageSize = getSize(
         tmpProjPath(`dist/libs/${childLib}/index.cjs`)
       );
       const esmPackageSize = getSize(
-        tmpProjPath(`dist/libs/${childLib}/index.js`)
+        tmpProjPath(`dist/libs/${childLib}/index.esm.js`)
       );
 
       // This is a loose requirement that ESM should be smaller than CJS output.
@@ -228,7 +228,7 @@ export async function h() { return 'c'; }
       // What we're testing
       runCLI(`build ${myLib}`);
       // Assertion
-      const content = readFile(`dist/libs/${myLib}/index.js`);
+      const content = readFile(`dist/libs/${myLib}/index.esm.js`);
 
       /**
        * Then check if the result contains this "promise" polyfill?

diff --git 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/e2e/web/src/web.test.ts b/e2e/web/src/web.test.ts
@@ -125,7 +125,7 @@ describe('Web Components Applications', () => {
     checkFilesExist(`dist/apps/_should_not_remove.txt`);
 
     // Asset that React runtime is imported
-    expect(readFile(`dist/libs/${libName}/index.js`)).toMatch(
+    expect(readFile(`dist/libs/${libName}/index.esm.js`)).toMatch(
       /react\/jsx-runtime/
     );
 

diff --git a/packages/js/.eslintrc.json b/packages/js/.eslintrc.json
@@ -42,6 +42,7 @@
               "typescript",
               "eslint",
               "verdaccio", // Optional, used for local registry
+              "rollup", // just for types (not in actual dist)
               // require.resolve is used for these packages
               "source-map-support",
               "@babel/core",

diff --git a/packages/js/src/plugins/rollup/type-definitions.ts b/packages/js/src/plugins/rollup/type-definitions.ts
@@ -0,0 +1,60 @@
+import type { OutputBundle } from 'rollup';
+import { relative } from 'path';
+import { stripIndents } from '@nx/devkit';
+
+//NOTE: This is here so we can share between `@nx/rollup` and `@nx/vite`.
+
+/*
+ * 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;
+}) {
+  return {
+    name: 'dts-bundle',
+    async generateBundle(_opts: unknown, bundle: OutputBundle): Promise<void> {
+      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,
+        });
+      }
+    },
+  };
+}
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
@@ -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
@@ -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,