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

Issue with react-proxy, forwardRefs & decorators #24553

Closed
dslounge opened this issue Apr 21, 2019 · 20 comments
Closed

Issue with react-proxy, forwardRefs & decorators #24553

dslounge opened this issue Apr 21, 2019 · 20 comments
Labels
Bug JavaScript 📮Known Issues This indicates an issue that refers to a bug or limitation of RN that is not currently being handled React Resolution: Locked This issue was locked by the bot. Tech: Bundler 📦 This issue is related to the bundler (Metro, Haul, etc) used.

Comments

@dslounge
Copy link

dslounge commented Apr 21, 2019

🐛 Bug Report

I'm having problems upgrading a project from RN 55 to RN 58.6
(UPDATE: this also occurs on 59, repro: https://github.com/dslounge/rnDecorators59).

My project has been using styled-components for a long time and it worked well. As I'm upgrading this project to RN 58.6, the app breaks due to issues with the withTheme decorator and react-proxy. Here are a few screenshots:

The error I'm getting:

The error location. createClassProxy.js from react-proxy doesn't handle forwardRefs:

image

This is a sample of the code that breaks. (repro: https://github.com/dslounge/rnDecorators59)

import React from "react";
import { withTheme } from "styled-components/native";
import { Text, View } from "react-native";

// THIS DOESN'T WORK
@withTheme
export class Button extends React.Component {
  render() {
    const { theme } = this.props;

    return (
      <View>
        <Text style={theme.buttonColor}>some text</Text>
      </View>
    );
  }
}

//THIS WORKS:
// class ButtonUI extends React.Component {
//   render() {
//     const { theme } = this.props;

//     return (
//       <View>
//         <Text style={{ color: theme.buttonColor }}>this should be red</Text>
//       </View>
//     );
//   }
// }

// export const Button = withTheme(ButtonUI);

My babel configuration seems to be fine:

{
  "presets": ["module:metro-react-native-babel-preset"],
  "plugins": [
    ["@babel/plugin-proposal-decorators", { "legacy": true }],
    ["@babel/plugin-proposal-export-namespace-from"]
  ]
}

I don't quite understand why this would break with an upgrade. As far as I know, styled-components has been using forwardRefs for a long time, and react-proxy hasn't changed in a long time either. I can only think that the breaking change is the babel upgrade.

This was my babelrc before upgrading:

{
  "presets": ["react-native-stage-0/decorator-support"],
  "env": {
    "development": {
      "plugins": ["transform-react-jsx-source"]
    }
  }
}

I'm not sure if react-native-stage-0/decorator-support behaves differently from @babel/plugin-proposal-decorators.

The only other thing I can think of is that there's some underlying change with Metro? I would love to understand what I'm missing here.

Related Issues:

I've looked for answers, I haven't found much, but some people are having this issue:

To Reproduce

  • Create a new RN 59 project
  • Add decorator support to an RN project and install styled-components
  • Make a component and try to use @withTheme decorator from styled-components
  • Get Expected a constructor error.

I've created a repo reproducing this issue: https://github.com/dslounge/rnDecorators59

Expected Behavior

I'm expecting my existing use of styled-components withTheme decorator to work just like it worked in RN 55.

Code Example

Repo reproducing the issue: https://github.com/dslounge/rnDecorators59

Environment

  React Native Environment Info:
    System:
      OS: macOS 10.14
      CPU: (4) x64 Intel(R) Core(TM) i5-4278U CPU @ 2.60GHz
      Memory: 292.31 MB / 8.00 GB
      Shell: 5.3 - /bin/zsh
    Binaries:
      Node: 10.13.0 - ~/.nvm/versions/node/v10.13.0/bin/node
      Yarn: 1.10.1 - /usr/local/bin/yarn
      npm: 6.4.1 - ~/.nvm/versions/node/v10.13.0/bin/npm
      Watchman: 4.7.0 - /usr/local/bin/watchman
    SDKs:
      iOS SDK:
        Platforms: iOS 12.1, macOS 10.14, tvOS 12.1, watchOS 5.1
      Android SDK:
        API Levels: 19, 21, 23, 24, 25, 26, 27, 28
        Build Tools: 19.1.0, 23.0.1, 23.0.3, 24.0.3, 25.0.1, 25.0.2, 25.0.3, 26.0.1, 26.0.2, 26.0.3, 27.0.3, 28.0.2, 28.0.3
        System Images: android-28 | Google Play Intel x86 Atom
    IDEs:
      Android Studio: 3.2 AI-181.5540.7.32.5014246
      Xcode: 10.1/10B61 - /usr/bin/xcodebuild
    npmPackages:
      react: 16.8.3 => 16.8.3
      react-native: 0.59.5 => 0.59.5
@react-native-bot

This comment has been minimized.

@dulmandakh

This comment has been minimized.

@dslounge

This comment has been minimized.

@hramos

This comment has been minimized.

@hramos hramos reopened this Apr 22, 2019
@brentvatne
Copy link
Collaborator

brentvatne commented Apr 22, 2019

it seems like withTheme isn't exported from styled-components/native but rather from styled-components: https://snack.expo.io/@notbrent/vigorous-popsicle

edit: here's a full example with the App.js from the original issue included as well: https://snack.expo.io/@notbrent/vigorous-popsicle-lol

edit 2: perhaps this only confirms that the issue didn't exist as far back as 0.57 ;P

@brentvatne
Copy link
Collaborator

brentvatne commented Apr 22, 2019

confirmed it's a React.forwardRef issue:

import React from "react";
import { Text, View } from "react-native";

// this works
function withLol(Component) {
  class MyThemedThing extends React.Component {
    render() {
      return <Component theme={{buttonColor: 'green'}} {...this.props} />
    }
  }

  return MyThemedThing;
}

// this doesn't work
function withForwardRef(Component) {
  const WithForwardRef = React.forwardRef((props, ref) => {
    return <Component theme={{buttonColor: 'green'}} {...props} ref={ref} />;
  });

  return WithForwardRef;
}

@withForwardRef
export class Button extends React.Component {
  render() {
    const { theme } = this.props;

    return (
      <View>
        <Text style={{color: theme.buttonColor}}>some text</Text>
      </View>
    );
  }
}

@elicwhite
Copy link
Member

@brentvatne, does forwardRef work as expected when used without decorators? Does it work with decorators in a new project?

@brentvatne
Copy link
Collaborator

@TheSavior - it works without decorators, yeah (const Button = withForwardRef(BaseButton) works). by new project do you mean a blank react-native init project? this is basically what @dslounge provided but with decorator babel plugin enabled

@dslounge
Copy link
Author

I think it ultimately comes down to this line in react-proxy. https://github.com/gaearon/react-proxy/blob/1.x/src/createClassProxy.js#L102. I'm just not sure why this started acting up all of a sudden with a react-native upgrade.

@ken0x0a
Copy link

ken0x0a commented Apr 30, 2019

Same problem here after start using yarn workspace.

When using single project (not monorepo), React.forwardRef was working fine. hmm...

@api182
Copy link

api182 commented Jun 14, 2019

I'm having this problem with 0.59, not using forwardRef but I am using MobX decorators...

@kelset
Copy link
Contributor

kelset commented Jun 17, 2019

Hey everyone, I was looking a bit closer at this issue today and I am concerned that this will affect more and more people, given that new versions of popular libraries like redux-react (v6 > v7 update) and mobx-react (v5 > v6 update) started using decorators.

I found that apparently the reason why this started happening after the react-native upgrade lies in Metro's HMR facebook/metro#272

And (back in December!) @gaearon wrote that

Again, if there’s a bug or missing thing in react-proxy I’d be happy to take a fix and close this. Did anyone send a PR?

So my best guess would be that:

Sorry, I don't have a perfect solution for this atm 😓But I hope that the new information can help coming up with a plan 🤞

@kelset kelset changed the title Issue with react-proxy, forwardRefs, styled-components Issue with react-proxy, forwardRefs & decorators Jun 17, 2019
@kelset kelset added Tech: Bundler 📦 This issue is related to the bundler (Metro, Haul, etc) used. 📮Known Issues This indicates an issue that refers to a bug or limitation of RN that is not currently being handled labels Jun 17, 2019
@gaearon
Copy link
Collaborator

gaearon commented Jun 17, 2019

We're going to get rid of react-proxy and that approach in the close future.

@wenkangzhou
Copy link

import { withTheme } from 'styled-components'

class SomeComp extends React.Component {
  render() {
    console.log(this.props.theme);
  }
}

export default withTheme(SomeComp);

this works for me

@gaearon
Copy link
Collaborator

gaearon commented Jul 2, 2019

react-proxy has been deleted in master.

We've replaced the transform with a different one that doesn't suffer from this issues.
The fix is expected to land in React Native 0.61.

See #18899 (comment) for details.

@gaearon gaearon closed this as completed Jul 2, 2019
@dslounge
Copy link
Author

dslounge commented Jul 31, 2019

If you are having this issue when upgrading to RN 59, some of the workarounds written above might not work. You can change this line in node_modules/metro/src/lib/parseOptionsFromUrl.js and use patch-package

diff --git a/node_modules/metro/src/lib/parseOptionsFromUrl.js b/node_modules/metro/src/lib/parseOptionsFromUrl.js
index 3a55687..331bcc1 100644
--- a/node_modules/metro/src/lib/parseOptionsFromUrl.js
+++ b/node_modules/metro/src/lib/parseOptionsFromUrl.js
@@ -113,7 +113,7 @@ function parseOptionsFromUrl(reqUrl, platforms) {
     options: {
       customTransformOptions,
       dev,
-      hot: true,
+      hot: false,
       minify,
       platform,
       onProgress: null,

@gaearon
Copy link
Collaborator

gaearon commented Aug 1, 2019

In general I would recommend avoiding the use of decorators, as they're not a stable language feature yet. (Although, as I said, this specific problem will be fixed in 0.61)

@JoshWalshaw
Copy link

In general I would recommend avoiding the use of decorators, as they're not a stable language feature yet. (Although, as I said, this specific problem will be fixed in 0.61)

I don't suppose you know how long we'll be waiting for the 0.61 release by any chance do you?

@kelset
Copy link
Contributor

kelset commented Aug 13, 2019

the cut of 0.61 will likely happen over the next couple weeks - sorry it's taking so long again :(

@gaearon
Copy link
Collaborator

gaearon commented Sep 25, 2019

0.61 is out with a fix.

@facebook facebook locked as resolved and limited conversation to collaborators Sep 25, 2019
@react-native-bot react-native-bot added the Resolution: Locked This issue was locked by the bot. label Jul 2, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Bug JavaScript 📮Known Issues This indicates an issue that refers to a bug or limitation of RN that is not currently being handled React Resolution: Locked This issue was locked by the bot. Tech: Bundler 📦 This issue is related to the bundler (Metro, Haul, etc) used.
Projects
None yet
Development

No branches or pull requests