Avoid leaking local types by the generated declarations

[Andarist]

↪️ Pull Request

When working on a PR here, I noticed that some type stopped being exported. This seemed quite odd because they were never exported in the first place.

We can verify this both by checking src/index.ts here and by checking the generated .d.ts here.

🚨 Test instructions

git clone git@github.com:bvaughn/react-resizable-panels.git && corepack pnpm i && pnpm run prerelease && pnpm run typescript

With this script, I'd expect the typechecking to trip over this import (and some others as well). If we apply my patch then we will witness the expected behavior.

✔️ PR Todo

• Added/updated unit tests for this change
• Included links to related issues/PRs

Note: I could use some pointers as to where I should put some tests for this and how they should look like (unit, integration, etc).

[Andarist]

You are using outFile here and that alters TS behavior when it comes to the generated code as it tries to concatenate files. On top of that, the emitted declarations assume a script mode (or something akin to it), and thus export {}; is not appended to them, and thus all local declarations leak.

I understand that this might be seen as a breaking change so it's up to you to decide if this is a good change. I believe though that the change itself is good. As a library author, I want to control what I export and what I don't, I don't want a tool to leak things that were meant to be private.

If you decide to pick up this change then it's likely that shake.js should be changed as well. After all, non-exported types could be pruned along with their otherwise unused dependencies.

[Andarist]

Actually, I tested this more and no changes to shake.js would be required. Those leaked types were leaked because they were still used by other retained things. So shaking works as expected.

[devongovett]

Could you explain more what you mean by leak? If they weren't exported in the type definitions, why would TypeScript allow importing them? And why would an empty export statement fix this? I have seen such a thing when there are no other exports in the file used to "tell" tools that something is ESM, but in this case, there are other exports already so I would assume that it isn't necessary?

[Andarist]

Could you explain more what you mean by leak?

You can see in this TS playground that the PanelGroupOnLayout is usable/importable and cross-reference it with the generated .d.ts to see that it has no export modifier nor it's exported through any other statement~: https://unpkg.com/browse/react-resizable-panels@0.0.45/dist/react-resizable-panels.d.ts#L7

If they weren't exported in the type definitions, why would TypeScript allow importing them?

That's a good question. It's related to compatibility with declarations written in times when ESM was being born and stuff (that's my guess). It seems that export alone in declaration file doesn't truly make those declaration files treated as modules. I think that in the past the ESM-like syntax might have been reused for declaration files without its full semantics.

I've made some tests and it seems that you either need export {} (or export { name }) or export { name } from './file.js' or export * from './file.js' to make it work "as expected". Since you "flat bundle" declaration files through the outFile-based logic... you don't really have references to any other files in the output. So it creates this weird "leak".

I'm pretty sure that Andrew Branch from the TypeScript team could shed more light on this. But at the same time... it's probably not worth pinging him since this is how TypeScript behaves, we don't really have to understand it deeply - we just need to take this for granted. I totally agree though that this is weird and surprising.

[Andarist]

@devongovett friendly 🏓