Skip to content

Commit

Permalink
fix(typescript)!: correctly resolve filenames of declaration files fo…
Browse files Browse the repository at this point in the history
…r `output.file` (#1728)

* test(typescript): add test case for invalid declarationDir with output.file

* fix(typescript): correctly resolve output filename of declaration files when output.file is used

* fix(typescript): validate that declarationDir is inside bundle output directory when using output.file

* test(typescript): check for correct error for invalid declarationDir when using output.file

---------

Co-authored-by: eu ler <27113373+u018f@users.noreply.github.com>
  • Loading branch information
lameuler and lameuler authored Sep 22, 2024
1 parent 62fac85 commit a85b649
Show file tree
Hide file tree
Showing 3 changed files with 46 additions and 20 deletions.
16 changes: 2 additions & 14 deletions packages/typescript/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -178,20 +178,8 @@ export default function typescript(options: RollupTypescriptOptions = {}): Plugi
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}`);
// the bundle output directory used by rollup when outputOptions.file is used instead of outputOptions.dir
baseDir = path.dirname(outputOptions.file);
}
if (!baseDir) return;

Expand Down
22 changes: 16 additions & 6 deletions packages/typescript/src/options/validate.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { relative } from 'path';
import { relative, dirname } from 'path';

import type { OutputOptions, PluginContext } from 'rollup';

Expand Down Expand Up @@ -51,14 +51,24 @@ export function validatePaths(
);
}

let outputDir: string | undefined = outputOptions.dir;
if (outputOptions.file) {
outputDir = dirname(outputOptions.file);
}
for (const dirProperty of DIRECTORY_PROPS) {
if (compilerOptions[dirProperty] && outputOptions.dir) {
if (compilerOptions[dirProperty] && outputDir) {
// Checks if the given path lies within Rollup output dir
const fromRollupDirToTs = relative(outputOptions.dir, compilerOptions[dirProperty]!);
const fromRollupDirToTs = relative(outputDir, compilerOptions[dirProperty]!);
if (fromRollupDirToTs.startsWith('..')) {
context.error(
`@rollup/plugin-typescript: Path of Typescript compiler option '${dirProperty}' must be located inside Rollup 'dir' option.`
);
if (outputOptions.dir) {
context.error(
`@rollup/plugin-typescript: Path of Typescript compiler option '${dirProperty}' must be located inside Rollup 'dir' option.`
);
} else {
context.error(
`@rollup/plugin-typescript: Path of Typescript compiler option '${dirProperty}' must be located inside the same directory as the Rollup 'file' option.`
);
}
}
}
}
Expand Down
28 changes: 28 additions & 0 deletions packages/typescript/test/test.js
Original file line number Diff line number Diff line change
Expand Up @@ -101,6 +101,34 @@ test.serial('ensures declarationDir is located in Rollup output dir', async (t)
);
});

test.serial(
'ensures declarationDir is located in Rollup output directory when output.file is used',
async (t) => {
const bundle = await rollup({
input: 'fixtures/basic/main.ts',
plugins: [
typescript({
tsconfig: 'fixtures/basic/tsconfig.json',
declarationDir: 'fixtures/basic/other/',
declaration: true
})
],
onwarn
});

// this should throw an error just like the equivalent setup using output.dir above
const wrongDirError = await t.throwsAsync(() =>
getCode(bundle, { format: 'es', file: 'fixtures/basic/dist/index.js' }, true)
);
t.true(
wrongDirError.message.includes(
`Path of Typescript compiler option 'declarationDir' must be located inside the same directory as the Rollup 'file' option`
),
`Unexpected error message: ${wrongDirError.message}`
);
}
);

test.serial('ensures multiple outputs can be built', async (t) => {
// In a rollup.config.js we would pass an array
// The rollup method that's exported as a library won't do that so we must make two calls
Expand Down

0 comments on commit a85b649

Please sign in to comment.