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

Decorating memoized component #2037

Closed
Kerizer opened this issue Feb 17, 2021 · 6 comments
Closed

Decorating memoized component #2037

Kerizer opened this issue Feb 17, 2021 · 6 comments

Comments

@Kerizer
Copy link

Kerizer commented Feb 17, 2021

Thanks so much for filing an issue or feature request! Please fill out the following (wherever relevant):

Steps to Reproduce

  1. Wrap functional component into memo function (imported from react or other library)
  2. Decorate this component with codepush

Expected Behavior

Codepush is working

Actual Behavior

When checking if render method exists there is no check if RootComponent.prototype exists - and it does not exists if RootComponent is wrapped into memo - https://github.com/microsoft/react-native-code-push/blob/master/CodePush.js#L581
image

Simplest solution would be to change if (RootComponent.prototype.render) { with if (RootComponent.prototype && RootComponent.prototype.render) { (let's say somebody really needs to use memo on the codepushuble component for whatever reason)

Environment

  • react-native-code-push version: 7.0.0
  • react-native version: Any
  • iOS/Android/Windows version: Any
  • Does this reproduce on a debug build or release build? - Both
  • Does this reproduce on a simulator, or only on a physical device? -Both

(The more info the faster we will be able to address it!)

@Kerizer
Copy link
Author

Kerizer commented Feb 18, 2021

I can also send a PR in case if this is ok solution. If for some reason you don't want people to decorate memoized components it's also possible to throw some more descriptive error

@muxbert
Copy link

muxbert commented Aug 10, 2021

@Kerizer I have the same issue when using mobX observer wrapper on the root component, did you fix this issue?

@ghost ghost added the stale label Dec 13, 2021
@ghost
Copy link

ghost commented Dec 13, 2021

This issue has been automatically marked as stale because it has not had any activity for 60 days. It will be closed if no further activity occurs within 15 days of this comment.

@Kerizer
Copy link
Author

Kerizer commented Dec 14, 2021

This issue has been automatically marked as stale because it has not had any activity for 60 days. It will be closed if no further activity occurs within 15 days of this comment.

Not resolved yet

@pxpeterxu
Copy link

Two workarounds:

1. Don't use React.memo

React.memo also has minimal benefits for the top-level component since by definition, it'll only get rendered once in your app, and its props are never going to change. You probably can get by without React.memo

2. Use React.memo, but fake it.

A quick workaround (since the only thing that App.prototype is used for is to check if it has a render() function, which functional components do not): just fake a prototype!

let RootComponent = () => (
  return <View><Text>Hello world</Text></View>;
);

const RootComponentPrototype = RootComponent.prototype;
RootComponent = React.memo(RootComponent);
RootComponent.prototype = RootComponentPrototype;

In terms of an actual patch for this, we probably could add a RootComponent.prototype && RootComponent.prototype.render in the if statement on https://github.com/microsoft/react-native-code-push/blob/v7.0.4/CodePush.js#L581

@DmitriyKirakosyan
Copy link
Contributor

Closing as duplicate of #2701.
The PR is merged and the fix will be available in the next release.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants