Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Workaround metro not compiling static properties correctly. #412

Merged
merged 1 commit into from
Jan 13, 2019

Conversation

miracle2k
Copy link
Contributor

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.

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.

@osdnk
Copy link
Contributor

osdnk commented Jan 13, 2019

Thanks, I had an intuition that's incorrect, but I have encountered the problem you've mentioned, so I found solution I committed workable and then leave it as temporary fix, which I forgot about. Sorry guys.

Copy link
Contributor

@osdnk osdnk left a comment

Choose a reason for hiding this comment

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

💯

@osdnk osdnk merged commit ee464b5 into software-mansion:master Jan 13, 2019
janicduplessis pushed a commit to janicduplessis/react-native-gesture-handler that referenced this pull request 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
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants