meteor-helper
meteor-helper copied to clipboard
PEM--
Modifying process.env.NODE_ENV breaks React
React has some places where it conditionally defines and calls functions. In the compiled version of React, the condition used is process.env.NODE_ENV === 'production'. Since _modifyProcessEnv() changes this value at runtime, React can wind up not defining the function, but then later attempting to call it. This seems to be the root cause of facebook/nuclide#1172 and steelbrain/linter-ui-default#212.
Humm... According to this line https://github.com/PEM--/meteor-helper/blob/v0.29.0/lib/meteor-helper-view.coffee#L194, process.env.NODE_ENV is only modified when you specify it in your configuration.
And from what I recall from React's source, code is only added when process.env.NODE_ENV !== 'production'. Hence, if you're passing any specific NODE_ENV except production, you should be good to go.
Wow, thanks for the quick response @PEM--! 😊
Did you see the React links in the OP? The first one defines a function when process.env.NODE_ENV !== 'production' and the second calls the function on the same condition. However, it's reevaluated each time so if the value changes from 'production' to something else, it's going to try to call a function that hasn't been defined.
Indeed, I've seen it. Nuclide should not be shipped in development mode if they rely on React.
Same stuff for linter-ui-default 😉
What's surprise me is that I don't get this issue on my setup... Anyway, shipping code in dev mode seems a bit... well... humm... anyway 😄
Hm…Well, Nuclide isn't shipping code "in dev mode," really (it's a runtime switch in React). But I see where you're coming from. Ideally, switching wouldn't be possible I suppose. But practically, I think this is the best choice for Nuclide if we want to have dependencies that require React. (Bundling would be a big overhead.)
Can I ask why meteor-helper has to change the current process's NODE_ENV? Couldn't it just pass a modified version to the BufferedProcess?
Q: Can I ask why meteor-helper has to change the current process's NODE_ENV? R: My users are using it to trigger different Meteor build.
Q: Couldn't it just pass a modified version to the BufferedProcess?
R: Indeed doable though that does not solve the nuclide usage of React in development package. Installing its dependencies with npm --production would solve all their problem and would avoid shipping code not used by their users. As a side effect, they would not have issues anymore caused by other package which are toying with process.env.NODE_ENV.
Will do 😉
Pretty stoked on the fact that Atom's will be using React. Nice move 👍
EDITED: Well, Atom is getting back on React. Nice, nice, nice 😄