Prevent process polyfill to be included in builds because of process … #9518
Conversation
| typeof process !== 'undefined' && | ||
| process.env && | ||
| process.env.NODE_ENV === 'test' | ||
| ) { |
There was a problem hiding this comment.
Please just change this require to be unconditional. It is no longer necessary to do this trickery now. (The original reason for this was related to some Jest behavior that's irrelevant with flat bundles.)
There was a problem hiding this comment.
should i just var ReactComponentTreeHook = require('ReactComponentTreeHook') at the top of the file?
This ReactComponentTreeHook is later conditionally required in __DEV__ block. What about it then? Should something be adjusted?
There was a problem hiding this comment.
should i just var ReactComponentTreeHook = require('ReactComponentTreeHook') at the top of the file?
Yes.
This ReactComponentTreeHook is later conditionally required in DEV block.
You can remove that. It’s unnecessary now.
| typeof process !== 'undefined' && | ||
| process.env && | ||
| process.env.NODE_ENV === 'test' | ||
| ) { |
There was a problem hiding this comment.
Let's not touch this file. It's gone in next version of React.
There was a problem hiding this comment.
Should i revert this change and the latter? Would be cool to fix this whole thing in 15.x
There was a problem hiding this comment.
Yes, please revert. I don’t see us fixing this in 15 given that the hack was added to work around an accidental breaking change in 15.2.1: #7240. Let’s wait for 16 here please.
| if ( | ||
| typeof process !== 'undefined' && | ||
| process.env && | ||
| process.env.NODE_ENV === 'test' |
There was a problem hiding this comment.
Same here, let's leave this one as is because it's a goner.
…guards preventing code replace
Andarist
force-pushed
the
prevent-process-polyfill
branch
from
7c06ee3
to
1df84e5
Compare
|
@gaearon ok, so i've amended the commit according to your suggestions |
|
Can you run |
|
This looks good to me, thanks! |
|
@gaearon sorry for not confirming sooner, ive tried to figure this out sooner, but the mentioned file Had to realise its used only for |
Related - reduxjs/redux#2244
By guarding
processlike this its polyfill is included by default when bundled with webpack. It should be highly desirable to avoid such side effect.I understand that this PR is not complete, but as your build setup is way more complex than what is used in most projects I would need some guidelines how this should be proceeded further. Maybe introducing
__TEST__in similar manner how u introduce__DEV__?Also this should land in
15.xbranch too and aint sure how this should be tackled here.cc @gaearon
Also maybe that could have been tackled on webpack's side as most of the people dont realise this is a problem and guarding against
processseems like a right way to do things in the code. Dunno if we can safely predict what was the user intention though. cc @TheLarkInn @sokra