Verify that the component passed to createAnimatedComponent is not functional #15019

janicduplessis · 2017-07-14T21:19:31Z

Stateless functional components don't support refs and we need that for the component to work, it used to crash with this error message: undefined is not an object (evaluating 'this._component.getScrollableNode'). This makes it clear what the issue is.

Fixes some of the errors in #10635, not sure if it fixes all the cases described in the issue though.

Test plan
Tested that passing a component with createClass or extends Component works but passing a function causes an error.

Release Notes

[GENERAL] [ENHANCEMENT] [Animated] - Verify that the component passed to createAnimatedComponent is not functional

janicduplessis · 2017-07-14T21:20:33Z

Libraries/Animated/src/AnimatedImplementation.js

@@ -1732,6 +1732,12 @@ class AnimatedProps extends Animated {
 }

 function createAnimatedComponent(Component: any): any {
+  invariant(
+    Component.prototype.render,


Is there a better way to check this? It does work but meh...

janicduplessis · 2017-09-09T05:13:04Z

@sahrens Could you have a look at this?

pull-bot · 2017-09-09T05:19:55Z

	Warnings
⚠️	📋 Release Notes - This PR appears to be missing Release Notes.

@facebook-github-bot label Needs more information

@facebook-github-bot label Core Team

Generated by 🚫 dangerJS

sahrens

Hmm, is this the best way to check? I thought React had some helper functions that would be more robust.

ide · 2017-09-09T17:43:38Z

Libraries/Animated/src/createAnimatedComponent.js

@@ -17,7 +17,15 @@ const AnimatedProps = require('./nodes/AnimatedProps');
 const React = require('React');
 const ViewStylePropTypes = require('ViewStylePropTypes');

+const invariant = require('fbjs/lib/invariant');
+
 function createAnimatedComponent(Component: any): any {


For Flow checking within the RN codebase, you could add something like this:

createAnimatedComponent<P>( Component: Class<React.Component<P>> ): Class<React.Component<*>> { ... }

I believe this will filter out functions too.

I remember trying to type this properly but I hit some issue. React components typing changed in flow 0.53 I can try again to see if I can figure out something that works.

janicduplessis · 2017-09-09T18:04:11Z

@sahrens Yea I’m not sure if there is a better way, I commented on the diff to ask that but it doesn’t show up since I rebased. Maybe ask someone from the react team.

janicduplessis · 2017-10-03T05:25:34Z

@bvaughn Is there a public API to check if a component is functional? (needed to know if refs are available).

bvaughn · 2017-10-03T16:22:17Z

@janicduplessis

FWIW we use this type of duck typing in React too (eg ReactFiberBeginWork, ReactTestUtilsEntry).

ReactBaseClass (aka React.Component, React.PureComponent) also defines an isReactComponent prop that we use some places (like in ReactFiber).

facebook-github-bot · 2017-10-09T17:03:31Z

@janicduplessis I tried to find reviewers for this pull request and wanted to ping them to take another look. However, based on the blame information for the files in this pull request I couldn't find any reviewers. This sometimes happens when the files in the pull request are new or don't exist on master anymore. Is this pull request still relevant? If yes could you please rebase? In case you know who has context on this code feel free to mention them in a comment (one person is fine). Thanks for reading and hope you will continue contributing to the project.

functional

janicduplessis · 2017-10-12T20:41:36Z

Thanks @bvaughn ! Update to use isReactComponent.

@ide I played with trying to flow type it properly again but there was an issue with default props, it didn't seem to pick it up properly anymore so it caused a bunch of errors where some props that should not be required were.

janicduplessis · 2017-11-05T16:13:30Z

ping @sahrens @bvaughn :)

bvaughn

Whoops, sorry for not seeing your earlier update. This looks good to me.

bvaughn · 2017-11-05T20:29:48Z

Libraries/Animated/src/createAnimatedComponent.js

 function createAnimatedComponent(Component: any): any {
+  invariant(
+    Component.prototype && Component.prototype.isReactComponent,
+    '`createAnimatedComponent` does not support stateless functional components, ' +


tiny nit: "," should be a ";" I think

I'll trust you on that one. I never know when ; should be used, I guess I wasn't listening at school that day :)

stale · 2018-01-04T21:03:48Z

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Maybe the issue has been fixed in a recent release, or perhaps it is not affecting a lot of people. If you think this issue should definitely remain open, please let us know why. Thank you for your contributions.

ide · 2018-01-11T23:39:52Z

@janicduplessis would you want to rebase this?

janicduplessis · 2018-01-12T19:28:23Z

@ide Looks like tests need to be updated to pass this new check, will try to find some time to update this soon.

…omp-error

janicduplessis · 2018-02-13T05:54:25Z

Updated and re-tested, we actually need to allow string components too since in prod views are just RCTView strings.

facebook-github-bot

@sahrens is landing this pull request. If you are a Facebook employee, you can view this diff on Phabricator.

…nctional Summary: Stateless functional components don't support refs and we need that for the component to work, it used to crash with this error message: `undefined is not an object (evaluating 'this._component.getScrollableNode')`. This makes it clear what the issue is. Fixes some of the errors in facebook#10635, not sure if it fixes all the cases described in the issue though. **Test plan** Tested that passing a component with createClass or extends Component works but passing a function causes an error. [GENERAL] [ENHANCEMENT] [Animated] - Verify that the component passed to createAnimatedComponent is not functional Closes facebook#15019 Differential Revision: D6988096 Pulled By: sahrens fbshipit-source-id: ec0ffa763245e786f44b4a1d56c0738876c25782

facebook-github-bot added CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. GH Review: review-needed labels Jul 14, 2017

janicduplessis commented Jul 14, 2017

View reviewed changes

janicduplessis added JavaScript Animated labels Jul 14, 2017

janicduplessis requested a review from sahrens August 3, 2017 19:45

miguelc1221 mentioned this pull request Aug 23, 2017

V2 posture monitor backbonelabs/backbone-mobile#359

Merged

janicduplessis force-pushed the create-animated-comp-error branch from 5f55c4a to 704fbd5 Compare September 9, 2017 05:12

sahrens reviewed Sep 9, 2017

View reviewed changes

ide reviewed Sep 9, 2017

View reviewed changes

janicduplessis added 2 commits October 12, 2017 15:36

Verify that the component passed to createAnimatedComponent is not

e25b54d

functional

Use isReactComponent in invariant

affbd87

janicduplessis force-pushed the create-animated-comp-error branch from 704fbd5 to affbd87 Compare October 12, 2017 19:52

Merge branch 'master' into create-animated-comp-error

a0189fc

bvaughn approved these changes Nov 5, 2017

View reviewed changes

bvaughn reviewed Nov 5, 2017

View reviewed changes

Update createAnimatedComponent.js

0957389

stale bot added the Stale There has been a lack of activity on this issue and it may be closed soon. label Jan 4, 2018

stale bot closed this Jan 11, 2018

ide reopened this Jan 11, 2018

stale bot removed the Stale There has been a lack of activity on this issue and it may be closed soon. label Jan 11, 2018

Merge branch 'master' into create-animated-comp-error

95b4d32

janicduplessis added 4 commits February 13, 2018 00:17

Merge remote-tracking branch 'upstream/master' into create-animated-c…

a597faa

…omp-error

Try fix tests

8bdf2a5

Try fix test p2

d89319f

Allow string components

fcd1cdc

facebook-github-bot added the Import Started This pull request has been imported. This does not imply the PR has been approved. label Feb 14, 2018

facebook-github-bot approved these changes Feb 14, 2018

View reviewed changes

facebook-github-bot closed this in 10b642a Feb 14, 2018

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Verify that the component passed to createAnimatedComponent is not functional #15019

Verify that the component passed to createAnimatedComponent is not functional #15019

janicduplessis commented Jul 14, 2017 •

edited

Loading

janicduplessis Jul 14, 2017

janicduplessis commented Sep 9, 2017

pull-bot commented Sep 9, 2017 •

edited

Loading

sahrens left a comment

ide Sep 9, 2017

janicduplessis Sep 9, 2017

janicduplessis commented Sep 9, 2017

janicduplessis commented Oct 3, 2017

bvaughn commented Oct 3, 2017 •

edited

Loading

facebook-github-bot commented Oct 9, 2017

janicduplessis commented Oct 12, 2017

janicduplessis commented Nov 5, 2017

bvaughn left a comment

bvaughn Nov 5, 2017

janicduplessis Nov 5, 2017

stale bot commented Jan 4, 2018

ide commented Jan 11, 2018

janicduplessis commented Jan 12, 2018

janicduplessis commented Feb 13, 2018

facebook-github-bot left a comment

Verify that the component passed to createAnimatedComponent is not functional #15019

Verify that the component passed to createAnimatedComponent is not functional #15019

Conversation

janicduplessis commented Jul 14, 2017 • edited Loading

Release Notes

janicduplessis Jul 14, 2017

Choose a reason for hiding this comment

janicduplessis commented Sep 9, 2017

pull-bot commented Sep 9, 2017 • edited Loading

sahrens left a comment

Choose a reason for hiding this comment

ide Sep 9, 2017

Choose a reason for hiding this comment

janicduplessis Sep 9, 2017

Choose a reason for hiding this comment

janicduplessis commented Sep 9, 2017

janicduplessis commented Oct 3, 2017

bvaughn commented Oct 3, 2017 • edited Loading

facebook-github-bot commented Oct 9, 2017

janicduplessis commented Oct 12, 2017

janicduplessis commented Nov 5, 2017

bvaughn left a comment

Choose a reason for hiding this comment

bvaughn Nov 5, 2017

Choose a reason for hiding this comment

janicduplessis Nov 5, 2017

Choose a reason for hiding this comment

stale bot commented Jan 4, 2018

ide commented Jan 11, 2018

janicduplessis commented Jan 12, 2018

janicduplessis commented Feb 13, 2018

facebook-github-bot left a comment

Choose a reason for hiding this comment

janicduplessis commented Jul 14, 2017 •

edited

Loading

pull-bot commented Sep 9, 2017 •

edited

Loading

bvaughn commented Oct 3, 2017 •

edited

Loading