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 #3435 - CommonJS and ESM loading confusion #3436

Merged
merged 1 commit into from
Nov 24, 2022

Conversation

tomi-bigpi
Copy link
Contributor

  • Fix TipTap getting loaded as CommonJS when the intent is to use the ES Module version.
  • Use .cjs and .mjs file extensions to help resolution in Node
  • package.json change also makes explicit exports required
  • Update core utilities exports to include all utilities
  • Update tests to use exported utilities

* Fix TipTap getting loaded as CommonJS when the intent is to use the ES Module version.
* `package.json` change also makes explicit exports required
* Update `core` utilities exports to include all utilities
* Update tests to use exported utilities
@netlify
Copy link

netlify bot commented Nov 21, 2022

Deploy Preview for tiptap-embed ready!

Name Link
🔨 Latest commit 1287ad3
🔍 Latest deploy log https://app.netlify.com/sites/tiptap-embed/deploys/637bf76f33ca40000899078f
😎 Deploy Preview https://deploy-preview-3436--tiptap-embed.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site settings.

@bdbch
Copy link
Member

bdbch commented Nov 24, 2022

Thanks @tomi-bigpi

since the PR is quite large I'll need some time to review it locally. I'll get back to you when I'm done!

@bdbch
Copy link
Member

bdbch commented Nov 24, 2022

I didn't find any issues while testing this locally. The code also looks fine to me and the change makes sense. I'll merge it for now to see if we'll have issues with the new script names with the next release.

Thanks for your support @tomi-bigpi

@tomi-bigpi
Copy link
Contributor Author

I didn't find any issues while testing this locally. The code also looks fine to me and the change makes sense. I'll merge it for now to see if we'll have issues with the new script names with the next release.

My take is that the likeliest thing to break is if anyone is directly importing any files as the exports option also requires explicit exports of classes/functions/etc. (In addition to the issue mentioned above)

Thanks for the quick turnaround!

andrewlu0 pushed a commit to trybaseplate/tiptap that referenced this pull request Oct 20, 2023
…3436)

* Fix TipTap getting loaded as CommonJS when the intent is to use the ES Module version.
* `package.json` change also makes explicit exports required
* Update `core` utilities exports to include all utilities
* Update tests to use exported utilities
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