Skip to content

Conversation

@cif
Copy link
Contributor

@cif cif commented May 28, 2019

Fixes a problem with transpiled code in CJS modules:

export * from 'react-router'

to

Object.keys(reactRouter).forEach(function (key) { exports[key] = reactRouter[key]; });

This was surfaced from Jest tests failing when updating to v5(uses CJS). When react-router-dom cjs attempts to create its exports, Object.defineProperty(exports, '__esModule', { value: true }); in react-router CSJ module has created a readonly __esModule prop via Object.defineProperty(exports, '__esModule', { value: true }); which gets mutated in the above code.

The best solution I could think of was to just explicitly state what you want from react-router in react-router-dom. I noticed react-router-native was doing the same and updated that as well as I have used jest for react-native applications.

@cif
Copy link
Contributor Author

cif commented May 28, 2019

Linked issue #6649

@timdorr
Copy link
Member

timdorr commented May 28, 2019

The simpler thing would be to use output.esModule: false

But I'm not sure why this is an issue. We do the exact same thing on Redux and I've never seen this issue come up, whereas that package is even more popular than this one. What would be attempting to write to a module's properties? That sounds like more of a Jest issue than our own.

@timdorr
Copy link
Member

timdorr commented May 28, 2019

Actually, thinking through my original comment on that issue, it's definitely related to having different versions of react-router and react-router-dom in your installed modules. That would be the only way that error would occur. In which case, while really opaque, this is definitely an error that should occur to prevent more subtle breakage.

@timdorr timdorr closed this May 28, 2019
@cif
Copy link
Contributor Author

cif commented May 28, 2019

@timdorr Let me try to explain more clearly what's happening:

  1. cjs/react-router-dom.js in its CJS module defines a read only __esModule prop like so:
    Object.defineProperty(exports, '__esModule', { value: true });

  2. cjs/react-router-dom.js requires react-router like so:
    reactRouter = require('react-router');

  3. cjs/react-router-dom.js tries to export all of the exported properties of reactRouter like so:
    Object.keys(reactRouter).forEach(function (key) { exports[key] = reactRouter[key]; });

  4. because exports.__esModule is a readonly property, an error is thrown.

The versions are definitely 5.0.0 for both react-router and react-router-dom in this case. I can setup an example repo if that helps?

I don't see where in your link to Redux the same behavior seen here is exhibited

@lock lock bot locked as resolved and limited conversation to collaborators Jul 27, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants