fix: experimentalDts for Node16/NodeNext module resolution

[aryaemami59]

This PR:

• Fixes experimentalDts to work correctly under TypeScript's Node16 and NodeNext module resolution.
• Resolves experimentalDts does not work with Node16 and NodeNext module resolution #1224.
• Resolves TypeScript files are seen as JavaScript when tsup resolves types of an external package in certain conditions #1023.

[codesandbox]

Review or Edit in CodeSandbox

Open the branch in Web Editor • VS Code • Insiders

Open Preview

[vercel]

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name	Status	Preview	Comments	Updated (UTC)
tsup	✅ Ready (Inspect)	Visit Preview	💬 Add feedback	Oct 19, 2024 5:19am

[pkg-pr-new]

Open in Stackblitz

pnpm add https://pkg.pr.new/egoist/tsup@1225

commit: ba6eb7d

[ocavue]

To matches the behavior of tsc's output, .js should only be added for Node16/NodeNext, not for Bundler etc.

[aryaemami59]

@ocavue tsc's Bundler module resolution can still handle (and output) relative .js\.mjs\.cjs file extensions.

[ocavue]

tsc's Bundler module resolution can still handle (and output) relative .js\.mjs\.cjs file extensions.

Thanks. TIL.

Does other moduleResolution options like Node can handle relative file extensions too?

[aryaemami59]

Does other moduleResolution options like Node can handle relative file extensions too?

@ocavue I think so, but just to be safe let me double check.

[aryaemami59]

Does other moduleResolution options like Node can handle relative file extensions too?

@ocavue Answer is yes!

[ocavue]

I still believe it’s safer to match tsc’s behavior (i.e., whether to include the .js extension in the generated .d.ts files). The output of tsc is the de facto standard, and there may be other tools besides TypeScript that consume .d.ts files.

In my opinion, a better approach would be to use moduleResolution to determine whether to add the .js extension.

[aryaemami59]

@ocavue Can't say I agree with that but if we wanted to offer some more flexibility we could put this functionality behind an option or at least provide an option to disable it? Something like addRelativePathExtension? The issue is as it stands right now (prior to this PR) the resulting type definitions for both ESM and CJS end up importing from the same _tsup-dts-rollup file. Maybe we could somehow merge the final type definition file with _tsup-dts-rollup and remove _tsup-dts-rollup from the final outDir?

[ocavue]

addRelativePathExtension seems too complex to me.

[aryaemami59]

@ocavue What do you think of removing _tsup-dts-rollup files from the final outDir?

addRelativePathExtension seems too complex to me.

I was going for something that was explicit and self-explanatory. While it may sound complicated, it sounds fairly obvious what it does. But fair enough, what would you have in mind?

[ocavue]

What do you think of removing _tsup-dts-rollup files from the final outDir?

Most packages only have one entrypoint, and removing _tsup-dts-rollup is a good idea for them. _tsup-dts-rollup.d.ts is a silly workaround for the fact that @microsoft/api-extractor doesn't support multi-entrypoints packages. Maybe a better solution is only output _tsup-dts-rollup.d.ts if there are more than one entrypoints.

---

I just don't think it's necessary to config addRelativePathExtension manually. Other .d.ts generator (i.e. tsc) don't have such an option.

[aryaemami59]

@ocavue I tried recreating the issue that you said this would cause, but I can't. I'm going to try some more, but is there anyway you could provide a more concrete example so I can make sure this PR doesn't cause the issues you're describing?

Edit: I added some tests for it, please let me know if it needs anything else.

[ocavue]

is there anyway you could provide a more concrete example

Here you are:

1. Open https://stackblitz.com/github/issueset/tsup-multi-dts or clone https://github.com/issueset/tsup-multi-dts
2. Run pnpm install && pnpm run build
3. You will get the following error

src/bad.ts:5:7 - error TS2322: Type 'import("/private/tmp/tsup-multi-dts/packages/my-lib-bad/dist/foo").SharedClass' is not assignable to type 'import("/private/tmp/tsup-multi-dts/packages/my-lib-bad/dist/bar").SharedClass'.
  Types of property 'symbol' are incompatible.
    Type 'typeof import("/private/tmp/tsup-multi-dts/packages/my-lib-bad/dist/foo").sharedSymbol' is not assignable to type 'typeof import("/private/tmp/tsup-multi-dts/packages/my-lib-bad/dist/bar").sharedSymbol'.