Skip to content

fix: Set correct module type in basic package #729

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

Merged
merged 1 commit into from
Apr 24, 2025
Merged

Conversation

danez
Copy link
Contributor

@danez danez commented Apr 20, 2025

This is a fix/workaround for the CommonJS/esm issue.

The root cause was adding the exports fields in package.json. Before it was introduced, Node.js would pick up what is defined in the main field (cjs) and bundlers used the module field (esm). When the exports field was introduced, Node.js started picking up the esm version too, but is nagging about no types field in package.json. After the types field was added, commonjs in bundlers is not working anymore because package.json says that all .js files in the package are ESM.

Possibilities:

  • Remove exports and type fields in package.json
  • Add 2 new package.json files and define the type correctly for the generated files. (What this PR does)

Fixes #724
Fixes #719

Ref #710
Ref #680

@jaywcjlove
Copy link
Member

@danez Oh, I didn’t know there was such a solution. I’m not sure if there’s any documentation available for me to learn from, but can I merge it first? It looks like this change won’t affect the dependent packages. I noticed that you didn’t remove the exports and types fields from the original package.json.

thx!

@danez
Copy link
Contributor Author

danez commented Apr 24, 2025

Yes this PR works as is.

I did not remove the fields from package.json, because I did not want to break vite and stuff. With this PR it should be fixed and no need to remove.

@jaywcjlove jaywcjlove merged commit d6ee7f8 into uiwjs:master Apr 24, 2025
1 check passed
@jaywcjlove
Copy link
Member

@danez Alright, I'll go ahead and merge this. Based on my testing, it doesn't affect our existing project.

@JimCarnicelli
Copy link

Hmm. I'm still seeing the error showing up:

image

All I have to do to trigger this is npm i @uiw/react-codemirror to upgrade from 4.21.21 to 4.32.10 and I get the error. The line it's complaining about is unambiguous too:

image

Thanks for the quick response on this. And hopefully there's a similarly easy fix for the remaining bug. Let me know if you need any other info about my project. Cheers.

jaywcjlove added a commit that referenced this pull request Apr 27, 2025
github-actions bot pushed a commit that referenced this pull request Apr 27, 2025
@jaywcjlove
Copy link
Member

@JimCarnicelli I’m not sure if this will fully resolve the issue, but I didn’t encounter any problems during my testing. You can refer to this example: React Codemirror Next.js Example.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment