react-native-code-push icon indicating copy to clipboard operation
react-native-code-push copied to clipboard

Decorating memoized component

Open Kerizer opened this issue 4 years ago • 5 comments

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 avatar Feb 17 '21 20:02 Kerizer

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

Kerizer avatar Feb 18 '21 06:02 Kerizer

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

muxbert avatar Aug 10 '21 14:08 muxbert

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.

ghost avatar Dec 13 '21 13:12 ghost

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

Kerizer avatar Dec 14 '21 18:12 Kerizer

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

pxpeterxu avatar Jan 06 '22 04:01 pxpeterxu