Skip to content
This repository has been archived by the owner on Jun 26, 2020. It is now read-only.

Fix commonjs exports #1267

Closed
wants to merge 1 commit into from
Closed

Conversation

vovkasm
Copy link

@vovkasm vovkasm commented Jan 4, 2019

Fixed: facebook#1266 and react-native#22863
@vovkasm
Copy link
Author

vovkasm commented Jan 4, 2019

@bvaughn Take attention please. It would be also very helpful to have releases tagged in git.

@vovkasm
Copy link
Author

vovkasm commented Jan 4, 2019

Also same issue: #1265

@vovkasm
Copy link
Author

vovkasm commented Jan 4, 2019

Also the tests passed I'm not sure is it right fix in common. Yes it fix RN issue, but should this module be available to direct inclusion from browser environment? May be better solution to split builds and make distinct builds for various targets (node, browser, es modules) and set appropriate entry points in package.json?
What people think? If this is the case, I can rework patch.

@bvaughn
Copy link
Contributor

bvaughn commented Jan 4, 2019

Hm, it seems like this was something I missed/broke when upgrading Webpack 1 -> 4 (PR #1235) and we're just noticing it now because we haven't released react-devtools-core since then.

The libraryTarget used to be "umd" before that change.

Copy link
Contributor

@bvaughn bvaughn left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for helping track down the root cause of this!

I'm going to restore the value to "umd" to be safest.

@bvaughn
Copy link
Contributor

bvaughn commented Jan 4, 2019

Just published v3.5.1 with the "umd" change– which seems to fix this issue. So thanks again!

@bvaughn bvaughn closed this Jan 4, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants