feat(typescript): write declaration files in configured directory for…

… `output.file` (#1378) * fix(typescript): Add missing types for `resolve` Signed-off-by: Ferdinand Thiessen <rpm@fthiessen.de> * feat(utils): Add test function `getFiles` This function returns the file names and content as rollup would write to filesystem. Emulating `handleGenerateWrite` and `writeOutputFile`. Signed-off-by: Ferdinand Thiessen <rpm@fthiessen.de> * fix(typescript): Fix writing declarations when `output.file` is used This fixes writing declarations into the configured `declarationDir` when configured rollup build with `output.file` instead of `output.dir`. Signed-off-by: Ferdinand Thiessen <rpm@fthiessen.de> * docs(typescript): Remove now unneeded documentation of non functional workaround Signed-off-by: Ferdinand Thiessen <rpm@fthiessen.de> * fix: Make `generateBundle` function more self explaining Signed-off-by: Ferdinand Thiessen <rpm@fthiessen.de> --------- Signed-off-by: Ferdinand Thiessen <rpm@fthiessen.de>
rollup · Apr 4, 2023 · 96b0338 · 96b0338
1 parent 9919bf2
commit 96b0338
Show file tree

Hide file tree

Showing 7 changed files with 97 additions and 71 deletions.
diff --git a/packages/typescript/README.md b/packages/typescript/README.md
@@ -300,54 +300,6 @@ export default {
 
 Previous versions of this plugin used Typescript's `transpileModule` API, which is faster but does not perform typechecking and does not support cross-file features like `const enum`s and emit-less types. If you want this behaviour, you can use [@rollup/plugin-sucrase](https://github.com/rollup/plugins/tree/master/packages/sucrase) instead.
 
-### Declaration Output With `output.file`
-
-When instructing Rollup to output a specific file name via the `output.file` Rollup configuration, and TypeScript to output declaration files, users may encounter a situation where the declarations are nested improperly. And additionally when attempting to fix the improper nesting via use of `outDir` or `declarationDir` result in further TypeScript errors.
-
-Consider the following `rollup.config.js` file:
-
-```js
-import typescript from '@rollup/plugin-typescript';
-
-export default {
-  input: 'src/index.ts',
-  output: {
-    file: 'dist/index.mjs'
-  },
-  plugins: [typescript()]
-};
-```
-
-And accompanying `tsconfig.json` file:
-
-```json
-{
-  "include": ["*"],
-  "compilerOptions": {
-    "outDir": "dist",
-    "declaration": true
-  }
-}
-```
-
-This setup will produce `dist/index.mjs` and `dist/src/index.d.ts`. To correctly place the declaration file, add an `exclude` setting in `tsconfig` and modify the `declarationDir` setting in `compilerOptions` to resemble:
-
-```json
-{
-  "include": ["*"],
-  "exclude": ["dist"],
-  "compilerOptions": {
-    "outDir": "dist",
-    "declaration": true,
-    "declarationDir": "."
-  }
-}
-```
-
-This will result in the correct output of `dist/index.mjs` and `dist/index.d.ts`.
-
-_For reference, please see the workaround this section is based on [here](https://github.com/microsoft/bistring/commit/7e57116c812ae2c01f383c234f3b447f733b5d0c)_
-
 ## Meta
 
 [CONTRIBUTING](/.github/CONTRIBUTING.md)

diff --git a/packages/typescript/package.json b/packages/typescript/package.json
@@ -71,6 +71,7 @@
     "@rollup/plugin-buble": "^1.0.0",
     "@rollup/plugin-commonjs": "^23.0.0",
     "@types/node": "^14.18.30",
+    "@types/resolve": "^1.20.2",
     "buble": "^0.20.0",
     "rollup": "^3.2.3",
     "typescript": "^4.8.3"

diff --git a/packages/typescript/src/index.ts b/packages/typescript/src/index.ts
@@ -154,24 +154,30 @@ export default function typescript(options: RollupTypescriptOptions = {}): Plugi
         const output = findTypescriptOutput(ts, parsedOptions, fileName, emittedFiles, tsCache);
         output.declarations.forEach((id) => {
           const code = getEmittedFile(id, emittedFiles, tsCache);
-          let baseDir =
-            outputOptions.dir ||
-            (parsedOptions.options.declaration
-              ? parsedOptions.options.declarationDir || parsedOptions.options.outDir
-              : null);
-          const cwd = normalizePath(process.cwd());
-          if (
-            parsedOptions.options.declaration &&
-            parsedOptions.options.declarationDir &&
-            baseDir?.startsWith(cwd)
-          ) {
-            const declarationDir = baseDir.slice(cwd.length + 1);
-            baseDir = baseDir.slice(0, -1 * declarationDir.length);
+          if (!code || !parsedOptions.options.declaration) {
+            return;
           }
-          if (!baseDir && tsconfig) {
-            baseDir = tsconfig.substring(0, tsconfig.lastIndexOf('/'));
+
+          let baseDir: string | undefined;
+          if (outputOptions.dir) {
+            baseDir = outputOptions.dir;
+          } else if (outputOptions.file) {
+            // find common path of output.file and configured declation output
+            const outputDir = path.dirname(outputOptions.file);
+            const configured = path.resolve(
+              parsedOptions.options.declarationDir ||
+                parsedOptions.options.outDir ||
+                tsconfig ||
+                process.cwd()
+            );
+            const backwards = path
+              .relative(outputDir, configured)
+              .split(path.sep)
+              .filter((v) => v === '..')
+              .join(path.sep);
+            baseDir = path.normalize(`${outputDir}/${backwards}`);
           }
-          if (!code || !baseDir) return;
+          if (!baseDir) return;
 
           this.emitFile({
             type: 'asset',

diff --git a/packages/typescript/test/test.js b/packages/typescript/test/test.js
@@ -6,7 +6,7 @@ const test = require('ava');
 const { rollup, watch } = require('rollup');
 const ts = require('typescript');
 
-const { evaluateBundle, getCode, onwarn } = require('../../../util/test');
+const { evaluateBundle, getCode, getFiles, onwarn } = require('../../../util/test');
 
 const typescript = require('..');
 
@@ -139,39 +139,66 @@ test.serial('supports emitting types also for single file output', async (t) =>
   // as that would have the side effect that the tsconfig's path would be used as fallback path for
   // the here unspecified outputOptions.dir, in which case the original issue wouldn't show.
   process.chdir('fixtures/basic');
+  const outputOpts = { format: 'es', file: 'dist/main.js' };
 
   const warnings = [];
   const bundle = await rollup({
     input: 'main.ts',
+    output: outputOpts,
     plugins: [typescript({ declaration: true, declarationDir: 'dist' })],
     onwarn(warning) {
       warnings.push(warning);
     }
   });
   // generate a single output bundle, in which case, declaration files were not correctly emitted
-  const output = await getCode(bundle, { format: 'es', file: 'dist/main.js' }, true);
+  const output = await getFiles(bundle, outputOpts);
 
   t.deepEqual(
     output.map((out) => out.fileName),
-    ['main.js', 'dist/main.d.ts']
+    ['dist/main.js', 'dist/main.d.ts']
+  );
+  t.is(warnings.length, 0);
+});
+
+test.serial('supports emitting declarations in correct directory for output.file', async (t) => {
+  // Ensure even when no `output.dir` is configured, declarations are emitted to configured `declarationDir`
+  process.chdir('fixtures/basic');
+  const outputOpts = { format: 'es', file: 'dist/main.esm.js' };
+
+  const warnings = [];
+  const bundle = await rollup({
+    input: 'main.ts',
+    output: outputOpts,
+    plugins: [typescript({ declaration: true, declarationDir: 'dist' })],
+    onwarn(warning) {
+      warnings.push(warning);
+    }
+  });
+  const output = await getFiles(bundle, outputOpts);
+
+  t.deepEqual(
+    output.map((out) => out.fileName),
+    ['dist/main.esm.js', 'dist/main.d.ts']
   );
   t.is(warnings.length, 0);
 });
 
 test.serial('relative paths in tsconfig.json are resolved relative to the file', async (t) => {
+  const outputOpts = { format: 'es', dir: 'fixtures/relative-dir/dist' };
   const bundle = await rollup({
     input: 'fixtures/relative-dir/main.ts',
+    output: outputOpts,
     plugins: [typescript({ tsconfig: 'fixtures/relative-dir/tsconfig.json' })],
     onwarn
   });
-  const output = await getCode(bundle, { format: 'es', dir: 'fixtures/relative-dir/dist' }, true);
+  const output = await getFiles(bundle, outputOpts);
 
   t.deepEqual(
     output.map((out) => out.fileName),
-    ['main.js', 'main.d.ts']
+    ['fixtures/relative-dir/dist/main.js', 'fixtures/relative-dir/dist/main.d.ts']
   );
 
-  t.true(output[1].source.includes('declare const answer = 42;'), output[1].source);
+  t.true(output[1].content.includes('declare const answer = 42;'), output[1].content);
 });
 
 test.serial('throws for unsupported module types', async (t) => {

diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml
diff --git a/util/test.d.ts b/util/test.d.ts
@@ -15,6 +15,16 @@ interface GetCode {
 
 export const getCode: GetCode;
 
+export function getFiles(
+  bundle: RollupBuild,
+  outputOptions?: OutputOptions
+): Promise<
+  {
+    fileName: string;
+    content: any;
+  }[]
+>;
+
 export function evaluateBundle(bundle: RollupBuild): Promise<Pick<NodeModule, 'exports'>>;
 
 export function getImports(bundle: RollupBuild): Promise<string[]>;

diff --git a/util/test.js b/util/test.js
@@ -1,3 +1,6 @@
+const path = require('path');
+const process = require('process');
+
 /**
  * @param {import('rollup').RollupBuild} bundle
  * @param {import('rollup').OutputOptions} [outputOptions]
@@ -7,13 +10,37 @@ const getCode = async (bundle, outputOptions, allFiles = false) => {
 
   if (allFiles) {
     return output.map(({ code, fileName, source, map }) => {
-      return { code, fileName, source, map };
+      return {
+        code,
+        fileName,
+        source,
+        map
+      };
     });
   }
   const [{ code }] = output;
   return code;
 };
 
+/**
+ * @param {import('rollup').RollupBuild} bundle
+ * @param {import('rollup').OutputOptions} [outputOptions]
+ */
+const getFiles = async (bundle, outputOptions) => {
+  if (!outputOptions.dir && !outputOptions.file)
+    throw new Error('You must specify "output.file" or "output.dir" for the build.');
+
+  const { output } = await bundle.generate(outputOptions || { format: 'cjs', exports: 'auto' });
+
+  return output.map(({ code, fileName, source }) => {
+    const absPath = path.resolve(outputOptions.dir || path.dirname(outputOptions.file), fileName);
+    return {
+      fileName: path.relative(process.cwd(), absPath).split(path.sep).join('/'),
+      content: code || source
+    };
+  });
+};
+
 const getImports = async (bundle) => {
   if (bundle.imports) {
     return bundle.imports;
@@ -70,6 +97,7 @@ const evaluateBundle = async (bundle) => {
 module.exports = {
   evaluateBundle,
   getCode,
+  getFiles,
   getImports,
   getResolvedModules,
   onwarn,