Skip to content

Conversation

@trueadm
Copy link
Contributor

@trueadm trueadm commented Jul 14, 2017

This is related to #10176. Dangling require() calls were created from the build due to the usage not being in a __DEV__ block within the source code. This PR aims to move the require calls so they no longer dangle in the production builds.

Also, the need to have single quotes within modulesToStub blocks has been removed – it was error prone.

Copy link
Contributor

@nhunzaker nhunzaker left a comment

Choose a reason for hiding this comment

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

Nice, 👍

@gaearon
Copy link
Collaborator

gaearon commented Jul 19, 2017

Do you mind rebasing this?

@trueadm
Copy link
Contributor Author

trueadm commented Jul 19, 2017

@gaearon Done just now, please give it another check when you have time :) thanks!

if (__DEV__) {
var warning = require('fbjs/lib/warning');
var {getCurrentStackAddendum} = require('ReactComponentTreeHook');
var {getStackAddendum} = require('ReactDebugCurrentFrame');
Copy link
Collaborator

Choose a reason for hiding this comment

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

This looks like code change?

Copy link
Collaborator

Choose a reason for hiding this comment

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

Oops nevermind.

Copy link
Collaborator

@gaearon gaearon left a comment

Choose a reason for hiding this comment

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

OK

@gaearon
Copy link
Collaborator

gaearon commented Jul 19, 2017

Let's get this in. It's not a long term solution but at least it's consistent with what we already do.
Longer term we'll want #10176 (comment). We can look at it with ES modules transition.

@trueadm
Copy link
Contributor Author

trueadm commented Jul 19, 2017

Cool :) Thanks

@trueadm trueadm merged commit 357925a into facebook:master Jul 19, 2017
@gaearon gaearon deleted the dev-only-requires-properly-handled branch July 19, 2017 15:30
@abigsmall
Copy link
Contributor

abigsmall commented Jul 23, 2017

@trueadm I noticed that in some files there remain invocations of warning that aren’t explicitly wrapped in if (__DEV__) blocks, such as for getTouchIdentifier in ResponderTouchHistoryStore.js: is that something to be concerned about?

Sorry if commenting on already merged PRs is not recommended, figured I’d try this rather than create a new GitHub issue

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants