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

Change source of truth for types to umd/index.d.ts to resolve issue with typescript import in 5.x (Resolves #109) #130

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

SimplyLinn
Copy link

@SimplyLinn SimplyLinn commented Nov 22, 2023

The previous PR to resolve this issue (#124) ended up duplicating the types to the umd module.

Moving the source-of-truth to the ./umd/index.d.ts file enables the ./index.d.ts file to import the class from the umd subdirectory with no issues. The other way around, however, seems to be the cause of the issues.

My suggestion is to move the source of truth to the umd declaration file, and import it from the root index.d.ts, as to avoid duplicated code, while still keeping the types functional.

This would resolve #109

@SimplyLinn SimplyLinn changed the title Change source of truth for types to umd/index.d.ts to resolve issue with typescript import in 5.x Change source of truth for types to umd/index.d.ts to resolve issue with typescript import in 5.x (Resolves #109) Nov 22, 2023
@arnidan
Copy link

arnidan commented Jan 18, 2024

@dcodeIO hello! Could you review the PR?

@dcodeIO
Copy link
Owner

dcodeIO commented Jan 18, 2024

Have ya'll verified that the proposed change indeed works for both ESM and CJS/UMD?

@alecgibson
Copy link

@dcodeIO we're trying to move a project from CJS to ESM, and it looks like if we swap the imports like in this PR, it works in both CJS and ESM.

@TangrisJones
Copy link

Would love to get this merged! 🙏

@yinzara
Copy link

yinzara commented Feb 6, 2024

Also would love to get this merged. Requires we enable skipLibCheck which we don't love

@aakasheoran
Copy link

@dcodeIO Were you able to review this? We need this fix. I have tested it as well and it works fine.

@gillg
Copy link

gillg commented Jun 5, 2024

Any news ?

tec27 added a commit to ShieldBattery/ShieldBattery that referenced this pull request Sep 17, 2024
tec27 added a commit to ShieldBattery/ShieldBattery that referenced this pull request Oct 3, 2024
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.

Unable import in TypeScript since v5+
9 participants