privatenumber / pkgroll

📦 Zero-config package bundler for Node.js + TypeScript
MIT License
1.01k stars 23 forks source link

Disable `composite` for type declarations #54

Closed halostatue closed 3 months ago

halostatue commented 4 months ago

Disable composite for type declarations

We are using pkgroll with Typescript monorepos (managed by pnpm workspaces) and project references. The latter requires compilerOptions.composite = true in referenced projects for most tsc invocations. If you set references without compilerOptions.composite = true, tsc will fail, indicating that composite needs to be set.

When I run pkgroll and have composite set, I get TS6307 for every file included from src/index.ts. rollup-plugin-dts does not handle composite (see Swatinem/rollup-plugin-dts#127, which was closed without fix or explanation). I looked at several other related tools (rollup-plugin-typescript2, rollup-plugin-ts, others mentioned in Swatinem/rollup-plugin-dts) and could not get any of them working to build just the DTS files (rollup-plugin-typescript2 was the most promising, but I could not get it working in the time that I allotted for this investigation).

Going back to rollup-plugin-dts, we find the compilerOptions option, and I tried explicitly setting composite: false, which allows my type files to build again without errors.

I don't think that this is the correct fix; ideally, rollup-plugin-dts would be fixed to better handle composite projects or would override composite at all times. An alternative change, although much more invasive to pkgroll, would be to have pkgroll accept an alternate tsconfig.json as a parameter:

$ pkgroll --tsconfig tsconfig.pkgroll.json

In the short term, this has eliminated the build issue that I have been seeing, and it may be sufficient to resolve this until a better fix can be determined in some way, whether by a --tsconfig parameter, upstream fixes, or changing to a different bundler plugin for type declaration plugins.

This issue can be seen at:

https://github.com/halostatue/pkgroll-monorepo-types-issue

privatenumber commented 4 months ago

Thanks for the PR!

Would you mind adding a test to demonstrate the issue you're addressing?

halostatue commented 4 months ago

Thanks for the PR!

Would you mind adding a test to demonstrate the issue you're addressing?

I wouldn't mind, but it’s going to be at least couple of weeks before I can get to it, because I discovered it in a multi-thousands-of-lines project during refactoring — and the project is sadly very late (it was supposed to be done mid-February).

I’m also not entirely sure how the test would work with your setup, because it requires a typescript monorepo project, so as I understand it, it would be an entirely separate fixture layout for this test.

halostatue commented 3 months ago

@privatenumber I managed to figure out a simple reproduction and have updated the main description with a link to the repo.

I have also added a unit test and fixture that covers this. It's a non-trivial case:

  1. You must be exporting types and bundling the resulting .d.{,m,c}ts files.
  2. You must have more than one file exporting types in src/, but everything is exported through the main file. That is, src/index.ts can simply export type { Name } from './other.js', which is exported from src/other.ts.
  3. The tsconfig.json must have compilerOptions.composite: true.

I will reiterate that I believe that this change is a hack and is fundamentally incorrect (and probably should not work, but so far it seems to). I believe that either Swatinem/rollup-plugin-dts needs to be fixed or pkgroll should look at using rollup-plugin-typescript2 for type exporting (it looks like it can be configured just for type exports, and it looks like while there were similar bugs related to composite projects, they seem to have been squashed).

I don't understand enough of what Swatinem/rollup-plugin-dts is doing to make a suitable bug report there, but hopefully the repro repo will help you understand it so that the underlying bug can be fixed, not just closed.

BTW, without the code change, this is the error reported from the unit test:

src/index.ts(1,22): error TS6307: File '/private/var/folders/c6/k_j3j3w11590l8x6t8rwxw200000gn/T/fs-fixture-1709648068713-1/packages/one/src/name.ts' is not listed within the file list of project ''. Projects must list all files or use an 'include' pattern.\n
privatenumber commented 3 months ago

I'm unable to push to your PR (your repo) all of a sudden, so I pushed here: https://github.com/privatenumber/pkgroll/pull/55 Commit history is still preserved.

git push git@github.com:KineticCafe/pkgroll.git fake-support-composite
ERROR: Permission to KineticCafe/pkgroll.git denied to privatenumber.
fatal: Could not read from remote repository.

Please make sure you have the correct access rights
and the repository exists.

I consolidated the fixture and made it more minimal.

halostatue commented 3 months ago

Thank you very much!