Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: Add types to package export map #569

Draft
wants to merge 4 commits into
base: main
Choose a base branch
from

Conversation

tylerbutler
Copy link

@tylerbutler tylerbutler commented May 25, 2024

This is a possible fix for StoneCypher/fsl#1295.

As noted in that issue, TypeScript is unable to find the included jssm types in some configurations. Part of the problem is that there are no types in the export map, so this PR updates the export map to include types entries.

I also created a rolled-up declaration for ESM and CJS using rollup-plugin-dts. This seems to work well in my testing and is the smallest fix I know of. I prototyped those changes and tested them on the repro project at https://github.com/tylerbutler/jssm-node16 and compilation worked as expected for both CJS and ESM.

Because jssm is using Rollup v2, I used an older release of rollup-plugin-dts since the most recent version requires Rollup 3.

Possible risks:

As it stands today without this change, TypeScript projects using more recent moduleResolution settings will fail to compile outright. With this change, those projects can compile and will see jssm types. However, there is a risk that rollup-plugin-dts produces invalid or flawed declaration files, or that there are further types-related issues that are uncovered by this change.

That said, this change should be exceedingly safe in terms of runtime behavior, because all the changes are limited to the types-level only, and thus only affect compilation in TypeScript projects. The runtime jssm JS has not changed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant