search

jaredpalmer / tsdx

Zero-config CLI for TypeScript package development
https://tsdx.io
MIT License
11.28k stars 507 forks source link

Enable useTsconfigDeclarationDir when declarationDir #465

Closed etienne-dldc closed 4 years ago

etienne-dldc commented 4 years ago

Current Behavior

The useTsconfigDeclarationDir in typescript2 rollup plugin is set to false.

Desired Behavior

When declarationDir is set in tsconfig.json set useTsconfigDeclarationDir to true.

Suggested Solution

useTsconfigDeclarationDir: tsconfigJSON && tsconfigJSON.declarationDir

Who does this impact? Who is this for?

This would fix #135 and in general improve experience in Monorepos (making "Go To Definition" work).

Describe alternatives you've considered

Using tsdx.config.js but the config is not easy to change and it's needed in every package of the monorepo.

agilgur5 commented 4 years ago

Not totally sure how this is related to #184, but in general, that solution makes sense to me as necessary to support declarationDir 👍 Can probably use optional chaining (?.) syntax now as well.

Want to add a PR w/ test for this?

etienne-dldc commented 4 years ago

Whoops, wrong issue number: #135 Yeah, I can create a PR 😃

etienne-dldc commented 4 years ago

It just appear to me that my "Suggested Solution" is stupid because rollup-plugin-typescript2 will ignore useTsconfigDeclarationDir if there are not declarationDir defined so my proposal is the same as setting this to true and there would be no way to have a declarationDir but still compile enerything in dist.

  1. This is fine, we just set useTsconfigDeclarationDir: true
  2. We expose a way to opt-in this option a. With an additional CLI argument --useDeclarationDir b. We read the typings field in package.json and set useTsconfigDeclarationDir if it point to the same folder as declarationDir we turn useTsconfigDeclarationDir on c. ???

Personally, I thing 2.a is the best options because it's non breaking and avoid unexpected behavior for people who might have a declarationDir by accident (copy/paste, default, other build tool...).

agilgur5 commented 4 years ago

rollup-plugin-typescript2 will ignore useTsconfigDeclarationDir if there are not declarationDir defined so my proposal is the same as setting this to true

I'm not certain that this is true. rpts2 defaults declarationDir to cwd, but if useTsconfigDeclarationDir is set, it's undefined: https://github.com/ezolenko/rollup-plugin-typescript2/blob/947da2523cf83c5ecd217439878bc28e2dd0c73f/src/get-options-overrides.ts#L29-L33

Is there some other piece of code you were looking at that made you think that? I still think your suggested solution is correct, whereas useTsconfigDeclarationDir: true seems like it would break some things due to the cwd -> undefined change.

Personally, I thing 2.a is the best options because it's non breaking and avoid unexpected behavior for people who might have a declarationDir by accident (copy/paste, default, other build tool...).

While thinking about breaking changes is a very good mentality to have when making changes to libraries (👍 👍 ), I don't think the accidental case is the important one. If someone has declarationDir set, but it's not being used, that's a bug (TSDX is not intentionally ignoring that config option). Also tsdx create does not add a declarationDir and as a "zero-config" CLI, other build tools shouldn't be necessary with TSDX. That is to say, I'm fairly positive usage of declarationDir inside of TSDX projects is very low. In any case, could always release as minor (but Jared controls releases, so I'm not really sure how it would end up)

agilgur5 commented 4 years ago

Closed by #468