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: ignore export assign cannot be used in module TS error, fixes #1018 #1022

Merged
merged 3 commits into from
Nov 14, 2024
Merged

fix: ignore export assign cannot be used in module TS error, fixes #1018 #1022

merged 3 commits into from
Nov 14, 2024

Conversation

ghiscoding
Copy link
Contributor

@ghiscoding ghiscoding commented Nov 14, 2024

fixes #1018

Summary

Fixes issue #1018 by simply ignoring the TypeScript error since the TS error has no negative effect whatsoever on the functionality of these types. This makes TypeScript and "Are the Types Wrong" both happy and everything is back to normal and we can get back to work (finally).

The quick explanation is that sometime even when using ESM imports, CJS types exports get picked up (depending on the users tsconfig module resolutions) and export = ... is a TypeScript error, but we can easily patch it by ignoring it.

image

Background & Context

I found this article Default exports in CommonJS libraries which explains why export = _default was patched in the first place. However this gives a TypeScript error in some cases (depending on your tsconfig module resolutions and other configs), this error can simply be ignored altogether because the types still work correctly, the error when active though will block a TypeScript build, however simply ignoring the error will let us continue with our build successfully.

image

Tasks

I simply modified the existing fix-cjs-types.js script and ran a build.

This also keeps "Are the Types Wrong" happy since nothing really changed for the types themselves, so all is good and we're all happy 🎉

image

Dependencies

  • Resolved dependency
  • Open dependency

@ghiscoding
Copy link
Contributor Author

@reduckted could you review the PR?

@reduckted
Copy link
Contributor

Looks good. Thanks for investigating and finding a fix! ❤️

@cure53 cure53 merged commit 0d54293 into cure53:main Nov 14, 2024
8 checks passed
@cure53
Copy link
Owner

cure53 commented Nov 14, 2024

Thanks everyone!

luxaritas pushed a commit to luxaritas/DOMPurify that referenced this pull request Nov 19, 2024
feature: Added configurability for text & HTML integration points
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants