-
Notifications
You must be signed in to change notification settings - Fork 576
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
Add common js module to realm/react #6050
Conversation
* fixes jest testing issues
format: "esm", | ||
sourcemap: true, | ||
}, | ||
], | ||
plugins: [nodeResolve(), typescript({ noEmitOnError: true })], | ||
plugins: [commonjs(), nodeResolve(), typescript({ noEmitOnError: true })], |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm curious if / why this was needed? What commonjs dependencies are bundled into @realm/react
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Jest is commonjs by default. It takes siginificant amount of manipulation to the jest framework to get it to support es modules.
I have also found that many react native libraries also produce a cjs in their dist for this reason.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Before this release, if you take a vanilla react native project...add @realm/react
and add something using @realm/react
in the default tests, it will fail on import
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sure, but adding this line is not nessesary to emit a CommonJS bundle. It's only needed if you want to include some CommonJS dependency into the bundle.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah ok...then it can most likely be removed. I just assumed I needed it to produce a commonjs output. Thanks for clarifying.
What, How & Why?
This closes #6049
☑️ ToDos
Compatibility
label is updated or copied from previous entryCOMPATIBILITY.md
package.json
s (if updating internal packages)Breaking
label has been applied or is not necessaryIf this PR adds or changes public API's: