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

fix(typescript): correctly resolve filenames of declaration files for output.file #1728

Open
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

lameuler
Copy link

@lameuler lameuler commented Jun 3, 2024

Rollup Plugin Name: @rollup/plugin-typescript

This PR contains:

  • bugfix
  • feature
  • refactor
  • documentation
  • other

Are tests included?

  • yes (bugfixes and features will not be merged without tests)
  • no

Breaking Changes?

  • yes (breaking changes will not be merged unless absolutely necessary)
  • no

If yes, then include "BREAKING CHANGES:" in the first commit message body, followed by a description of what is breaking.

List any relevant issue numbers:

Description

Currently, when output.file is specified instead of output.dir, the filename for emitting declaration files with this.emitFile is resolved incorrectly. This means that given a declarationDir that is outside of the bundle output directory (which is output.dir || dirname(output.file!)), the declarations will still be generated inside the bundle, albeit in a location not actually matching declarationDir that does not respect the absolute file path generated by typescript.

Whereas when using output.dir, declarationDir is actually validated and filenames for declarations are resolved correctly.

For example, with output.file = 'some-path/index.js' and declarationDir = 'types', the declaration files will be generated at some-path/types/index.d.ts, instead of types/index.d.ts which is the correct location (but invalid as it would be output the bundle directory). When output.dir = 'some-path' is used instead, validation fails as expected.

This can be fixed by just using dirname(output.file) in place of output.dir when output.file is specified, and including a validation check that declarationDir is valid when using output.file.

@lameuler lameuler requested a review from shellscape as a code owner June 3, 2024 03:15
Copy link
Author

@lameuler lameuler left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

baseDir, which is used to resolve the pathname for this.emitFile using path.relative(baseDir, id), should just be the bundle output directory since id is the absolute file path we want to emit.

@shellscape
Copy link
Collaborator

Thanks for the PR. Since this will change some expectations around outputs, we'll have to mark this as a breaking change.

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

Successfully merging this pull request may close these issues.

2 participants