fix: set module.exports = ReactJWPlayer for Node ESM-for-CJS default-import#208
Open
esetnik wants to merge 1 commit into
Open
fix: set module.exports = ReactJWPlayer for Node ESM-for-CJS default-import#208esetnik wants to merge 1 commit into
esetnik wants to merge 1 commit into
Conversation
…import
Set `module.exports` (and `module.exports.default`) to the
`ReactJWPlayer` class directly from `src/react-jw-player.jsx`, so the
compiled `dist/react-jw-player.js` works under both of the
default-import conventions that bundlers use for CJS modules:
- Babel-interop path (Rollup, webpack < 5, Vite 7): reads
`module.exports.default` (or `exports.default`), which we still set
via the explicit `module.exports.default = ReactJWPlayer` assignment.
- Node ESM-for-CJS path (Vite 8 / rolldown, webpack 5, Node's own
ESM-importing-CJS): reads `module.exports`, which is the
`ReactJWPlayer` class itself — not a
`{default: ReactJWPlayer, __esModule: true}` namespace object.
Before this change the compiled entry ended with
`exports.default = ReactJWPlayer;` + the babel-emitted
`Object.defineProperty(exports, "__esModule", {value: true});`, with
no `module.exports` reassignment, so consumers on the Node ESM-for-CJS
path received the full namespace as their default import and
`<ReactJWPlayer />` crashed with `Element type is invalid: ... got: object`
(React error micnews#130) when bundled by Vite 8.
Closes micnews#207.
References:
- rolldown/rolldown#8061 (rolldown's
isNodeMode heuristic that triggers the path split)
- https://rolldown.rs/in-depth/bundling-cjs#ambiguous-default-import-from-cjs-modules
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #207.
Context
When a React app bundled by Vite 8 (or any Node ESM-for-CJS importer) does:
…it currently gets the CJS namespace object
{default: ReactJWPlayer, __esModule: true}rather than theReactJWPlayerclass, and<ReactJWPlayer />crashes withElement type is invalid: ... got: object(React error#130).The split comes from rolldown's (intentional) isNodeMode heuristic — see rolldown/rolldown#8061, closed as expected behavior. When the consumer has
"type": "module", rolldown treats the CJS default import asmodule.exportsitself (Node's semantics), notmodule.exports.default(Babel's semantics).react-jw-player's compileddist/react-jw-player.jscurrently ends with:…plus the babel-emitted
Object.defineProperty(exports, "__esModule", {value: true});at the top. It never reassignsmodule.exports. So under Node semantics the consumer receives the whole namespace and React throws.Change (this PR)
One-file change in
src/react-jw-player.jsx: assignmodule.exports = ReactJWPlayer(andmodule.exports.default = ReactJWPlayerto preserve the.defaultaccess path) in place of theexport default. After babel compiles,dist/react-jw-player.jsends with:module.exportsnow points at theReactJWPlayerclass directly, satisfying both bundler conventions. No new dependency, no build-config change.Alternative approach
If you'd rather keep
src/react-jw-player.jsxas pure ESM (export default ReactJWPlayer), the equivalent fix is to addbabel-plugin-add-module-exportsto.babelrcand let it emit themodule.exports = exports["default"];tail automatically:{ - "presets": ["es2015", "react"], - "plugins": ["transform-object-assign"] + "presets": ["es2015", "react"], + "plugins": ["transform-object-assign", "add-module-exports"] }That plugin is widely used (ships in many UI libs for exactly this reason) but hasn't had a release since 2019 — given how long this repo has been stable on the current
dist/shape, I went with the source-level approach to avoid introducing any new dep. Happy to switch to the plugin if you prefer.Verification
dist/withnpm run build— diff is localized to the entry file.dist/react-jw-player.jsinto a Vite 8 app that was crashing with React#130on<ReactJWPlayer />; the error goes away and the player mounts correctly. Same app continues to work under Vite 7.Happy to iterate on the comment block in
src/react-jw-player.jsxif you'd rather keep the entry terse.