From 4afed0cf495898b08e8540ba272741d82f540597 Mon Sep 17 00:00:00 2001 From: eu ler <27113373+u018f@users.noreply.github.com> Date: Mon, 3 Jun 2024 10:00:41 +0800 Subject: [PATCH 1/4] test(typescript): add test case for invalid declarationDir with output.file --- packages/typescript/test/test.js | 23 +++++++++++++++++++++++ 1 file changed, 23 insertions(+) diff --git a/packages/typescript/test/test.js b/packages/typescript/test/test.js index 127f98e19..29a59ccc8 100644 --- a/packages/typescript/test/test.js +++ b/packages/typescript/test/test.js @@ -101,6 +101,29 @@ 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 + await t.throwsAsync(() => + getCode(bundle, { format: 'es', file: 'fixtures/basic/dist/index.js' }, true) + ); + // TODO add check for specific error 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 From f95d278c7f2e83bc1905858cbdc203f8846bc193 Mon Sep 17 00:00:00 2001 From: eu ler <27113373+u018f@users.noreply.github.com> Date: Mon, 3 Jun 2024 10:07:42 +0800 Subject: [PATCH 2/4] fix(typescript): correctly resolve output filename of declaration files when output.file is used --- packages/typescript/src/index.ts | 16 ++-------------- 1 file changed, 2 insertions(+), 14 deletions(-) diff --git a/packages/typescript/src/index.ts b/packages/typescript/src/index.ts index 6b0eb2652..339a8ba40 100644 --- a/packages/typescript/src/index.ts +++ b/packages/typescript/src/index.ts @@ -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; From 34dfc6705fbc8fb3166809ddc896f8d48d7473de Mon Sep 17 00:00:00 2001 From: eu ler <27113373+u018f@users.noreply.github.com> Date: Mon, 3 Jun 2024 10:13:10 +0800 Subject: [PATCH 3/4] fix(typescript): validate that declarationDir is inside bundle output directory when using output.file --- packages/typescript/src/options/validate.ts | 22 +++++++++++++++------ 1 file changed, 16 insertions(+), 6 deletions(-) diff --git a/packages/typescript/src/options/validate.ts b/packages/typescript/src/options/validate.ts index 7dbc3367c..d0f22e3de 100644 --- a/packages/typescript/src/options/validate.ts +++ b/packages/typescript/src/options/validate.ts @@ -1,4 +1,4 @@ -import { relative } from 'path'; +import { relative, dirname } from 'path'; import type { OutputOptions, PluginContext } from 'rollup'; @@ -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.` + ); + } } } } From 0abc35ff50f3095d1e6553032208b6e18abf01c8 Mon Sep 17 00:00:00 2001 From: eu ler <27113373+u018f@users.noreply.github.com> Date: Mon, 3 Jun 2024 10:15:51 +0800 Subject: [PATCH 4/4] test(typescript): check for correct error for invalid declarationDir when using output.file --- packages/typescript/test/test.js | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/packages/typescript/test/test.js b/packages/typescript/test/test.js index 29a59ccc8..d5ac91086 100644 --- a/packages/typescript/test/test.js +++ b/packages/typescript/test/test.js @@ -117,10 +117,15 @@ test.serial( }); // this should throw an error just like the equivalent setup using output.dir above - await t.throwsAsync(() => + const wrongDirError = await t.throwsAsync(() => getCode(bundle, { format: 'es', file: 'fixtures/basic/dist/index.js' }, true) ); - // TODO add check for specific error message + 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}` + ); } );