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

Add missing type export conditions and index.d.cts #283

Merged
merged 1 commit into from
Jul 18, 2023
Merged

Add missing type export conditions and index.d.cts #283

merged 1 commit into from
Jul 18, 2023

Conversation

AtoraSuunva
Copy link
Contributor

While the library has types, it's missing the proper conditional types exports in package.json that causes any project using node16 or bundler module resolution in Typescript to fail to build:

Could not find a declaration file for module '@dice-roller/rpg-dice-roller'. '<project>/node_modules/@dice-roller/rpg-dice-roller/lib/esm/bundle.js' implicitly has an 'any' type.
  There are types at '<project>/node_modules/@dice-roller/rpg-dice-roller/types/index.d.ts', but this result could not be resolved when respecting package.json "exports". The '@dice-roller/rpg-dice-roller' library may need to update its package.json or typings.

You can also see the types failing to resolve here:
image

This PR adds in the missing types condition to exports (and a required index.d.cts that's just a copy of index.d.ts) so that Typescript is happy and can resolve types and compile:
image

Note that you'll still need skipLibCheck: true in tsconfig.json regardless of your module settings until #247 is fixed though.

@AtoraSuunva AtoraSuunva requested a review from GreenImp as a code owner July 17, 2023 21:42
Copy link
Collaborator

@GreenImp GreenImp left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks great, thanks!

@GreenImp GreenImp merged commit b4abd67 into dice-roller:develop Jul 18, 2023
@GreenImp
Copy link
Collaborator

This has just been released in v5.4.1

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.

2 participants