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: cbor-web imports do not work in esm #772

Merged
merged 3 commits into from
Jun 14, 2023

Conversation

rflechtner
Copy link
Contributor

fixes KILTProtocol/ticket#2749

Fixes a broken import in esm code. The fix is ugly, but it's actually what a recent post on dual-esm-cjs libraries recommends (https://evertpot.com/universal-commonjs-esm-typescript-packages/)

How to test:

I tried importing the relevant functions in both an mjs and a cjs script, that worked.

Checklist:

  • I have verified that the code works
  • I have verified that the code is easy to understand
    • If not, I have left a well-balanced amount of inline comments
  • I have left the code in a better state
  • I have documented the changes (where applicable)
    • Either PR or Ticket to update the Docs
    • Link the PR/Ticket here

@rflechtner
Copy link
Contributor Author

@arty-name what I could do to make this a little nicer is move that import magic to a file in the utils package, and then import from there?

Copy link
Collaborator

@arty-name arty-name left a comment

Choose a reason for hiding this comment

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

Yeah, I guess a separate file in utils will be a nice abstraction

@rflechtner
Copy link
Contributor Author

Yeah, I guess a separate file in utils will be a nice abstraction

Hope that's better?

@rflechtner rflechtner merged commit 6727213 into hotfix-0.33.1 Jun 14, 2023
@rflechtner rflechtner deleted the rf-hotfix-cbor-web-esm-import branch June 14, 2023 11:52
rflechtner added a commit that referenced this pull request Jun 14, 2023
* fix: cbor-web imports do not work in esm (#772)
* chore: set release version 0.33.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