facebook / react-native

A framework for building native applications using React
https://reactnative.dev
MIT License
118.37k stars 24.25k forks source link

[React Fiber] EventPluginHub causes many crashes #12905

Closed K-Leon closed 7 years ago

K-Leon commented 7 years ago

Description

After Upgrade to 0.43 and corresponding Fiber Update there got a new Bug introduced regarding Event Handling. If you press twice on a button and due to a lag or something like that the event get's fired when the calling element got already unmounted the app crashes.

undefined is not an object (evaluating 'props[registrationName]') undefined is not an object (evaluating 'props[registrationName]')

https://github.com/facebook/react-native/blob/fd4ad6ca82676e4f63bad26eb0869981439ba106/Libraries/Renderer/src/renderers/shared/shared/event/EventPluginHub.js#L150

( I symbolicated this by hand so here is the original issue and the corresponding minified line )

undefined is not an object (evaluating 'u[n]') undefined is not an object (evaluating 'u[n]')

[_d(function(e,n,t,r){"use strict";function u(e){return"button"===e||"input"===e||"select"===e||"textarea"===e}function o(e,n,t){switch(e){case"onClick":case"onClickCapture":case"onDoubleClick":case"onDoubleClickCapture":case"onMouseDown":case"onMouseDownCapture":case"onMouseMove":case"onMouseMoveCapture":case"onMouseUp":case"onMouseUpCapture":return!(!t.disabled||!u(n));default:return!1}}var s=n(95),i=n(96),c=n(97),a=n(98),l=n(99),p=n(22),f=null,v=function(e,n){e&&(i.executeDispatchesInOrder(e,n),e.isPersistent()||e.constructor.release(e))},d=function(e){return v(e,!0)},E=function(e){return v(e,!1)},g={injection:{injectEventPluginOrder:s.injectEventPluginOrder,injectEventPluginsByName:s.injectEventPluginsByName},getListener:function(e,n){var t;if("number"==typeof e.tag){var r=i.getFiberCurrentPropsFromNode(e.stateNode);if(!r)return null;if(t=r[n],o(n,e.type,r))return null}else{if("string"==typeof e._currentElement)return null;if(!e._rootNodeID)return null;var u=e._currentElement.props;if(t=u[n],o(n,e._currentElement.type,u))return null}return p(!t||"function"==typeof t,"Expected %s listener to be a function, instead got type %s",n,typeof t),t},extractEvents:function(e,n,t,r){for(var u,o=s.plugins,i=0;i<o.length;i++){var c=o[i];if(c){var l=c.extractEvents(e,n,t,r);l&&(u=a(u,l))}}return u},enqueueEvents:function(e){e&&(f=a(f,e))},processEventQueue:function(e){var n=f;f=null,e?l(n,d):l(n,E),p(!f,"processEventQueue(): Additional events were enqueued while processing an event queue. Support for this has not yet been implemented."),c.rethrowCaughtError()}};t.exports=g},94);](url) 

Reproduction

What you need to Trigger this Bug:

a) Release Build b) A View with Button that is expensive to render - best on a different scene c) Now Switch fast between Scenes and Press Button ( Press Buttons like maniac ;) ). At some point if there's a lag or sth. and an Touch Event i too late an issue is thrown at that point and the whole app crashes:

Additional Information

cc: @bvaughn @acdlite @sebmarkbage

K-Leon commented 7 years ago

A simple Check if props === undefined and Return null fixed it for me. I'm not sure if this is an acceptable solution - if so I'll send a pr

sebmarkbage commented 7 years ago

I think I know why this is but I'd like to confirm. Do you have a small repo?

Did you possibly have a number that you were tapping instead of a string?

E.g. <Text>{123}</Text>

I think this only happens for numeric text.

Sometimes this pattern can accidentally create numbers: <Text>{number && 'Text'}</Text>

K-Leon commented 7 years ago

We have a very large RN App and we've seen this in Production - so i don't have an easy repro :/. Even though we absolutely use the pattern you described. We have all kinds of numbers and mixed numbers in text fields at the masks i tested.

K-Leon commented 7 years ago

Even though we have no crash if we use everything "slowly" - tap by tap. And I can confirm adding null Check solved it.

sebmarkbage commented 7 years ago

Opened a fix upstream in React. https://github.com/facebook/react/pull/9194

PARAGJYOTI commented 7 years ago

Still getting error in Strings . When I scroll fast in VirtualizedList and tap on some item , it crashes showing cannot get property of undefined _props[registrationName] .

sebmarkbage commented 7 years ago

@PARAGJYOTI on which version of React Native?

PARAGJYOTI commented 7 years ago

Its 0.43.3 . I fixed it , although Its a hack .

I replaced the line 150 to

listener = props ? props[registrationName] :null;

It's seems to be working now .

On Tue, Apr 11, 2017 at 1:41 AM, Sebastian Markbåge < notifications@github.com> wrote:

@PARAGJYOTI https://github.com/PARAGJYOTI on which version of React Native?

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/facebook/react-native/issues/12905#issuecomment-293064412, or mute the thread https://github.com/notifications/unsubscribe-auth/AKYl_wQeuwFnzoJtxqXccIcrKtT67WkXks5ruozpgaJpZM4MbwV6 .

henninghall commented 7 years ago

I had the same issue and can confirm that it occurs when a number is tapped. @PARAGJYOTI 's solution works fine. I also managed to avoid the crashes by changing
<Text>{99}</Text> to <Text>{''+99}</Text> which indicates that the problem only occurs for numbers.

AndrewJack commented 7 years ago

It's fixed in 0.44.0-rc by these two commits https://github.com/facebook/react-native/commit/5f8e46b8b43c752642ded162b0d1a18ee46c13fa & https://github.com/facebook/react-native/commit/17cb70efdd1ed6378677e720a022c9d83ad87dd6

CodeXtinction commented 7 years ago

@sebmarkbage that is exactly what is happening to me

darleikroth commented 7 years ago

@sebmarkbage Thank you!!

Did you possibly have a number that you were tapping instead of a string? ... I think this only happens for numeric text.

AlanFoster commented 7 years ago

Running into this too within 0.43.3; If this is fixed on the latest master - I wonder if this is something that could be back-ported for a 0.43.4 release?

henrikra commented 7 years ago

We got this crash in production over 20 times in 24 hours :( So in next version of react-native this will be fixed?

emrehayirci commented 7 years ago

I am getting same error when using react-native-calendar-picker component. I wonder is there a solution?

Error occours to me when i try to select a date for three times. After getting error three times i can finally pick a date at 4th press

jjzazuet commented 7 years ago

@emrehayirci Hi, for what I can understand in the bug, if we're still stuck on RN 0.43.xx, I think we can try to get the calendar picker component to render the actual day numbers as strings, which may correct the issue. My apologies, as I don't have a concrete code snippet at the moment. Cheers!

hramos commented 7 years ago

Hi there! This issue is being closed because it has been inactive for a while. Maybe the issue has been fixed in a recent release, or perhaps it is not affecting a lot of people. Either way, we're automatically closing issues after a period of inactivity. Please do not take it personally!

If you think this issue should definitely remain open, please let us know. The following information is helpful when it comes to determining if the issue should be re-opened:

If you would like to work on a patch to fix the issue, contributions are very welcome! Read through the contribution guide, and feel free to hop into #react-native if you need help planning your contribution.

codebymikey commented 6 years ago

Should be fixed now.