Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[rollup-plugin-typescript] Re-enable declarationDir outside output file/dir #1772

Open
marekdedic opened this issue Sep 23, 2024 · 17 comments
Open

Comments

@marekdedic
Copy link

Expected Behavior

Enable declarationDir to be outside the rollup output directory, much like it was possible in v11

Actual Behavior

Since v12, I get:

[!] (plugin typescript) RollupError: [plugin typescript] @rollup/plugin-typescript: Path of Typescript compiler option 'declarationDir' must be located inside the same directory as the Rollup 'file' option.

Additional Information

The change I'd like to be reconsidered was introduced in #1728. However, emitting declarations to a different place from the build output can be really useful for things like post-processing with api-extractor - having the intermediary declarations produced by rollup be outside the final output dir makes it much easier to not accidentally include these files in the final build.

@shellscape
Copy link
Collaborator

Thanks for the issue. I'd like to point out that the reproduction you provided is not valid, and that's not entirely cool since we lay out exactly what is.

On the subject matter itself, #1728 was a breaking change in how that worked and released as a major because of that. Moving forward, that's the intended behavior. What you're describing is the job of a post-processing task, which you elude to since you're using something like api-extractor. It would make sense that instead of error-prone output, post-processing moves build files to where they need to be in order to process them. This is extremely common in devops and deployments (much of what I do day to day).

I see a few possible paths forward for this use case:

  • intelligently handle/move build files where they need to be for additional processing
  • exclude appropriate files from final builds/packaging/deployment
  • write a rollup plugin to handle all or some of the above, which will run after plugin-typescript
  • open a PR which provides the capability you're requesting, without reintroducing problems previously associated with the capability prior to fix(typescript)!: correctly resolve filenames of declaration files for output.file #1728

This repo is almost entirely dependent on community contributions, so please understand that this may be something you have to affect to see it realized.

@stemcstudio
Copy link

I'd like to suggest that #1728 has introduced behavior that may not be intended. In order to determine whether this is actually the case, the relationship between the "file" option in the OutputOptions type in the rollup.config.* file and the "outDir" and/or "declarationDir" property in compilerOptions of the tsconfig.json file needs to be clarified. I have not been able to find relevant documentation for the plugin.

The fact that the rollup configuration supports multiple output files suggests to me that there is a conceptual issue with the relationship between these properties because the declarationDir property cannot simultaneously be inside many different directories implied by multiple OutputOptions.

@shellscape
Copy link
Collaborator

your argument may be valid, but in a specific context - yours, and a few others. the full request is it was written was to solve a problem that plagued the plug-in for a very long time and affected a lot of users. to that end, it's an effective solution. it's also a breaking change, which means you don't necessarily have to adopt the new contract.

that said, I'm sure we would all be open to a new pull request which satisfied the problem as it was resolved by #1728, and allowing for the use case which you view as correct.

if you'd like to discuss this until the cows come home, you're more than welcome to. but there's not going to be a one-size-fits-all solution unless the community steps up to make that happen.

@michaelfaith
Copy link
Contributor

@shellscape is there an issue that explains what the original bug was that this change aimed to solve? There isn't one linked in the PR and it's not clear to me from the PR description what the bug was. You're saying it was such a widespread problem that affected so many people, but I've used the plugin for a long time and haven't experienced any issue this solves. So, it'd help to understand the larger context, in order to ensure any follow-up change is consistent with that goal.

@carry0987
Copy link

carry0987 commented Oct 17, 2024

@marekdedic
I’m experiencing a similar issue, but only with version v12.1.1, not in v12.1.0. In my case, I want to place the declaration files in the dist/dts folder, while keeping the UMD and module JS files in the dist folder.

Here are my settings:

tsconfig.json

{
    "compilerOptions": {
        "module": "ESNext",
        "target": "ESNext",
        "moduleResolution": "Node",
        "esModuleInterop": true,
        "importHelpers": true,
        "strict": true,
        "jsx": "react-jsx",
        "jsxImportSource": "preact",
        "declaration": true,
        "declarationDir": "dist/dts/",
        "paths": {
            "@/*": ["./src/*"]
        }
    },
    "include": [
        "src/**/*",
        "test/**/*",
        "rollup.config.ts"
    ],
    "exclude": ["node_modules", "dist", "**/__snapshots__/**/*"]
}

rollup.config.ts

import { RollupOptions } from 'rollup';
import typescript from '@rollup/plugin-typescript';
import terser from '@rollup/plugin-terser';
import replace from '@rollup/plugin-replace';
import tsConfigPaths from 'rollup-plugin-tsconfig-paths';
import nodeResolve from '@rollup/plugin-node-resolve';
import { dts } from 'rollup-plugin-dts';
import postcss from 'rollup-plugin-postcss';
import del from 'rollup-plugin-delete';
import { createRequire } from 'module';
import path from 'path';

const pkg = createRequire(import.meta.url)('./package.json');
const isProduction = process.env.BUILD === 'production';
const sourceFile = 'src/index.ts';

const jsConfig: RollupOptions = {
    input: sourceFile,
    output: [
        {
            file: pkg.exports['.']['umd'],
            format: 'umd',
            name: 'checkBoxjs',
            plugins: isProduction ? [terser()] : []
        }
    ],
    plugins: [
        postcss({
            extract: path.resolve(pkg.exports['./theme/checkBox.min.css']),
            minimize: true,
            sourceMap: false
        }),
        typescript(),
        tsConfigPaths(),
        nodeResolve(),
        replace({
            preventAssignment: true,
            __version__: pkg.version
        })
    ]
};

const esConfig: RollupOptions = {
    input: sourceFile,
    output: [
        {
            file: pkg.exports['.']['import'],
            format: 'es'
        }
    ],
    plugins: [
        postcss({
            inject: false,
            extract: false,
            sourceMap: false
        }),
        typescript(),
        tsConfigPaths(),
        nodeResolve(),
        replace({
            preventAssignment: true,
            __version__: pkg.version
        })
    ]
};

const dtsConfig: RollupOptions = {
    input: sourceFile,
    output: {
        file: pkg.exports['.']['types'],
        format: 'es'
    },
    external: [/\.scss$/u],
    plugins: [
        tsConfigPaths(),
        dts(),
        del({ hook: 'buildEnd', targets: ['dist/dts'] })
    ]
};

export default [jsConfig, esConfig, dtsConfig];

package.json

{
  //...
  "type": "module",
  "main": "dist/checkBox.min.js",
  "module": "dist/checkBox.esm.js",
  "types": "dist/index.d.ts",
  "exports": {
    ".": {
      "umd": "./dist/checkBox.min.js",
      "import": "./dist/checkBox.esm.js",
      "types": "./dist/index.d.ts"
    },
    "./theme/checkBox.min.css": "./dist/theme/checkBox.min.css"
  },
  //...
  "devDependencies": {
    "@carry0987/utils": "^3.7.8",
    "@rollup/plugin-node-resolve": "^15.3.0",
    "@rollup/plugin-replace": "^6.0.1",
    "@rollup/plugin-terser": "^0.4.4",
    "@rollup/plugin-typescript": "^12.1.1",
    "@testing-library/preact": "^3.2.4",
    "happy-dom": "^15.7.4",
    "preact": "^10.24.3",
    "prettier": "^3.3.3",
    "rollup": "^4.24.0",
    "rollup-plugin-delete": "^2.1.0",
    "rollup-plugin-dts": "^6.1.1",
    "rollup-plugin-postcss": "^4.0.2",
    "rollup-plugin-tsconfig-paths": "^1.5.2",
    "sass": "^1.80.1",
    "tslib": "^2.8.0",
    "vitest": "^2.1.3"
  }
}

Do you have any suggestions on how to resolve this issue or specific settings that work between these versions? Let me know if more details are needed for troubleshooting. Thank you!

@marekdedic
Copy link
Author

@carry0987 In the end, I chose the solution of building types in dist/types with rollup, then bundling them with api-extractor and then deleting the dist/types folder. See here for inspiration.

@carry0987
Copy link

@michaelfaith I've add the example in my reply, let me know if more details are needed for troubleshooting.

@carry0987
Copy link

@carry0987 In the end, I chose the solution of building types in dist/types with rollup, then bundling them with api-extractor and then deleting the dist/types folder. See here for inspiration.

Thanks ! But I still want to use pure plugin @ollup/plugin-typescript, since api-extractor is too heavy and complex for my projects.

@marekdedic
Copy link
Author

@carry0987 Hmm, re-reading your first comment, it seems like you want to just do the thing I am doing before running api-extractor?

@carry0987
Copy link

carry0987 commented Oct 18, 2024

@carry0987 Hmm, re-reading your first comment, it seems like you want to just do the thing I am doing before running api-extractor?

Exactly, I just want to create index.d.ts instead of creating multiple declaration files separately for whole project.
Everything work perfectly before v12.1.1, I'm using v12.1.0 currently.

@carry0987
Copy link

carry0987 commented Oct 20, 2024

@marekdedic @michaelfaith @shellscape

Regarding my requirement, which is not to generate default declaration files but rather to produce a single .d.ts file in the dist directory alongside the mjs and cjs files, I am considering removing the declaration and declarationDir configurations from my tsconfig.json. Then, I will use rollup-plugin-dts with index.ts as the source to independently generate index.d.ts in the dist directory. An additional benefit of this approach is that it eliminates the need to use rollup-plugin-delete to remove the dist/dts folder.

@marekdedic I think you can just remove declaration and declarationDir configurations like me, and using rollup-plugin-dts to generate individual declaration file in wherever you want.

@kahagerman
Copy link

kahagerman commented Oct 21, 2024

It seems to me that the implementation of #1783 is flawed.

Versions <12 assumed that declarationDir was relative to outputDir. (./lib + ./types = ./lib/types)

As of 12.1.0 (presumably for 12.0.0 through 12.1.0) the assumption is that declarationDir is relative to the root directory. (./lib + ./types = ./types) this matches the behaviour of tsc, but isn't viable when you need multiple outputs in different directories; I think this is the problem that #1783 is trying to resolve.

#1783 changes this behaviour somehow that I don't fully understand (there's a rough explanation in a comment here); but it always seems to complain no matter what I do.

@michaelfaith
Copy link
Contributor

michaelfaith commented Oct 21, 2024

I think this is the problem that #1783 is trying to resolve.

That PR is solving the issue where output.file in the rollup config is nested in a directory underneath what's defined as outDir in the tsconfig. Not in a completely different peer directory. I can absolutely see how that would be valuable, but it seems the original change that introduced this behavior wanted to prevent that very situation. I don't know enough about the original motivation for that change to speak on it, but my use case was just limited to nested sub-directories under outDir.

@marekdedic
Copy link
Author

marekdedic commented Oct 21, 2024

To add to this, I actually played with the source code a bit to try and implement this and didn't find any reasonable way to go outside the rollup output dir in a rollup plugin.

So my position now is that it'd be nice to be able to have a separate directory for declarations, but it's not possible (in a non-hacky way) - due to the limitations of rollup itself, not this plugin.

#1728 solidifies this limitation and makes the behavior around this more visible and clear, but it didn't introduce this issue.

@kahagerman
Copy link

kahagerman commented Oct 22, 2024

@marekdedic from your experiments, do you think an upstream fix to rollup itself could feasibly resolve this limitation?

@marekdedic
Copy link
Author

@kahagerman No. From the point of view of rollup, the limitation is that any output files should be in the output directory. I think that that is a perfectly reasonable limitation when looking at it from rollup's point of view.

@Danielku15
Copy link

Danielku15 commented Nov 24, 2024

I face the same problem like many here. My usecase is also to bundle the types of my library using rollup-plugin-dts but I need an entry-point d.ts. The only solution seems now to set declarationDir=outDir. But this pollutes your output folderdirectly with d.ts files for all .ts.

I'd like to have outDir=dist, declarationDir=dist/types and output.file=dist/mylib.mjs so I can have another rollup bundle with input.file=dist/types/mylib.d.ts + rollup-plugin-dts.

As far I understood the discussion and change in #1728 is to prevent having types fully outside the output directory (e.g. /dist and /types). But allowing declarationDir=dist/types with a output.file=dist/mylib.mjs is not against the goals of rollup.

// Checks if the given path lies within Rollup output dir
if (outputOptions.dir) {
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.`
);
}
} else {
const fromTsDirToRollup = relative(compilerOptions[dirProperty]!, outputDir);
if (fromTsDirToRollup.startsWith('..')) {
context.error(
`@rollup/plugin-typescript: Path of Typescript compiler option '${dirProperty}' must be located inside the same directory as the Rollup 'file' option.`
);
}
}

If my understanding is correct, the code above could be fixed like this:

// Checks if the given path lies within Rollup output dir 
const fromRollupDirToTs = relative(outputDir, compilerOptions[dirProperty]!); 
if (fromRollupDirToTs.startsWith('..')) { 
  context.error(
    outputOptions.dir 
     ? `@rollup/plugin-typescript: Path of Typescript compiler option '${dirProperty}' must be located inside Rollup 'dir' option.` 
     : `@rollup/plugin-typescript: Path of Typescript compiler option '${dirProperty}' must be located inside the same directory as the Rollup 'file' option.` 
  ); 
}

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

7 participants