Remove HMR feature #336
Comments
|
The problem is - "metro" is using React-Hot-Loader v2. Right now it's version 4, and version 5 is planned to fix 1024, and probably any other issue. It's better to upgrade it, than to remove. |
|
Related: The HMR feature causes code which uses static class members to be compiled incorrectly at times, see this thread: software-mansion/react-native-gesture-handler#406 (comment) |
osdnk
pushed a commit
to software-mansion/react-native-gesture-handler
that referenced
this issue
Jan 13, 2019
Fixes the issue we encountered #406 (comment) To review, the problem is as follows: The correct syntax is: ``` static propTypes = { ...GenericTouchable.internalPropTypes } ``` Metro/Babel compiles this to: ``` // Define GenericTouchable GenericTouchable.propTypes = (0, _objectSpread2.default)({}, GenericTouchable.internalPropTypes, GenericTouchable.publicPropTypes); ``` But when metro then runs the HMR transform, this becomes: ``` // Create _class _class.propTypes = (0, _objectSpread2.default)({}, GenericTouchable.internalPropTypes, GenericTouchable.publicPropTypes), ``` It rewrites the class name to `_class` during initialization, but misses the references in the spread, which then causes a undefined-access exception. See this Gist for full outputs: https://gist.github.com/miracle2k/3bc7f1c50080397dbf0d92cfb3101677#file-generictouchable-babel-with-metro-preset-js Part of the problem seems to be that metro uses a very old version of [react-hot-loader](facebook/metro#336). The code as it is currently in the library, using `...this.publicPropTypes` has two problems: First, it does not compile at all with babel, when this library becomes imported as part of using `react-native-web`. Second, while it does compile using metro, the generated code instead will be: ``` // Create _class _class.propTypes = (0, _objectSpread2.default)({}, this.internalPropTypes, this.publicPropTypes), ``` With `this` probably referring to the window or global scope, thus not causing a crash, but not actually setting the proptypes correctly.
|
True. Current HMR solution is a bit aggresive. Even offensive. |
janicduplessis
pushed a commit
to janicduplessis/react-native-gesture-handler
that referenced
this issue
Feb 16, 2020
…-mansion#412) Fixes the issue we encountered software-mansion#406 (comment) To review, the problem is as follows: The correct syntax is: ``` static propTypes = { ...GenericTouchable.internalPropTypes } ``` Metro/Babel compiles this to: ``` // Define GenericTouchable GenericTouchable.propTypes = (0, _objectSpread2.default)({}, GenericTouchable.internalPropTypes, GenericTouchable.publicPropTypes); ``` But when metro then runs the HMR transform, this becomes: ``` // Create _class _class.propTypes = (0, _objectSpread2.default)({}, GenericTouchable.internalPropTypes, GenericTouchable.publicPropTypes), ``` It rewrites the class name to `_class` during initialization, but misses the references in the spread, which then causes a undefined-access exception. See this Gist for full outputs: https://gist.github.com/miracle2k/3bc7f1c50080397dbf0d92cfb3101677#file-generictouchable-babel-with-metro-preset-js Part of the problem seems to be that metro uses a very old version of [react-hot-loader](facebook/metro#336). The code as it is currently in the library, using `...this.publicPropTypes` has two problems: First, it does not compile at all with babel, when this library becomes imported as part of using `react-native-web`. Second, while it does compile using metro, the generated code instead will be: ``` // Create _class _class.propTypes = (0, _objectSpread2.default)({}, this.internalPropTypes, this.publicPropTypes), ``` With `this` probably referring to the window or global scope, thus not causing a crash, but not actually setting the proptypes correctly.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
So I suggest to remove it from metro.
The text was updated successfully, but these errors were encountered: