fix(typescript): avoid resolving definition files with arbitrary extensions

[ychavoya]

Rollup Plugin Name: 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: resolves #1858

Description

This PR updates the regular expression used to detect declaration files (*.d.ts) to include declaration files for arbitrary extensions, such as the ones allowed with the allowArbitraryExtensions compiler option (*.d.*.ts). By including these in the regex, they are not resolved and therefore prevent the error displayed in #1858.

Also, in order to add a test for this behavior, the typescript version in the dev dependencies was upgraded to v5 and therefore some other pieces of code had to be changed.

[emersion]

I was wondering whether an arbitrary string is allowed between .d. and .ts, or if only a restricted set of characters are allowed. It turns out any arbitrary string is allowed here:

https://github.com/microsoft/TypeScript/blob/01c23d68b1113a4b71acb8484f1bf78a98307933/src/compiler/parser.ts#L10568

tl;dr this regexp sounds correct to me.

[emersion]

What is this warning? Is it about the module value not being aligned with the moduleResolution value?

Shouldn't we resolve it instead?

[ychavoya]

Resolved! I was not sure if I should update this to include the warning, or this other statement https://github.com/rollup/plugins/pull/1949/changes#diff-9b47757e1347eae5ab5cefedce0669826e925f9b073a02815b9507b782f2211bL1540 since changing the module changes how the code is output in the test.

[emersion]

What the test cares about is the unresolved import, so I'd say it's better if we avoid any other unnecessary noise :P

[emersion]

Nit: could we avoid the cast here by using new Map<string, ExtendedConfigCacheEntry>() instead?

[ychavoya]

Thanks, I missed that when changing it

[emersion]

Looks good apart from this minor warning-related comment!

[emersion]

These new changes look good! Can you squash them into the commits that introduced what they're fixing?

[ychavoya]

Done, I squashed the changes into the typescript upgrade, so now there are only the 3 commits I initially added. Let me know if you want them combined as well

[emersion]

LGTM!

[emersion]

@shellscape, would you mind having a look?

[shellscape]

@emersion I would advise not adding an approving review to PRs in repos which you do not maintain.

@CharlieHelps please do a thorough review of this PR to determine if it's ready to merge. Look at the diff, reviews, comments, and referenced issues/docs/links to build a complete understanding. Then respond with a comment that includes a list of MUST fix items that must be fixed before the the PR is merged, and a list of IMPROVEMENTS that shouldn't block the merge but may be quick improvements or things that could be addressed in a follow up PR. The MUST fix items should explain exactly what's wrong and what needs to change to resolve the problem. Both should be numbered lists and the IMPROVEMENTS list should continue counting up from the MUST list. It's OK if there are no IMPROVEMENTS or MUST fix items, you can just omit the lists.

[charliecreates]

Thanks for the PR — the .d.<ext>.ts handling matches TypeScript’s allowArbitraryExtensions behavior.

MUST fix

1. Fix failing Linux CI (Validate Monorepo). The PR is currently red (e.g. Node v20 job): https://github.com/rollup/plugins/actions/runs/21718266321/job/63704024881.

   Locally, pnpm --filter "...[origin/master]" ci:coverage fails because the AVA runner times out / never exits (eventually OOM). On this branch it looks like the plugin is leaving a large number of live FSWatcher handles behind after a normal rollup() build (even without Rollup watch mode), which keeps the process alive.

   Change needed: ensure the TS watch program / file watchers are not left running for non-watch builds (e.g. don’t create a watch program when this.meta.watchMode is false, and/or dispose/close the program in a hook that runs during bundle.generate() such as generateBundle/buildEnd, since the tests don’t call bundle.close()).

2. Track down the TS1110: Type expected diagnostic shown in the failing run (RollupError: @rollup/plugin-typescript TS1110: Type expected.). Please identify which file triggers it and adjust module resolution/arbitrary-extension handling so TS consumes the *.d.<ext>.ts declaration (and doesn’t try to parse the arbitrary extension source itself).

IMPROVEMENTS
3. Consider addressing the open TS5110 warning discussion by tweaking the fixture/tsconfig to avoid emitting the warning (rather than asserting on it), so the test output stays clean: #1949 (comment).
4. Small typing cleanup from the review thread: prefer new Map<string, ExtendedConfigCacheEntry>() over casting, if you touch that area again: #1949 (comment).
5. Tiny TS nit in the new fixture: use number instead of Number in module.d.custom.ts.
6. The new assertion that checks bundle.cache.modules is a bit indirect; consider also asserting the build succeeds without warnings/errors for that import to keep the test resilient.