Skip to content
This repository has been archived by the owner on Sep 10, 2022. It is now read-only.

'withProps' on 'View' component crashes in release mode #525

Closed
hellogerard opened this issue Sep 21, 2017 · 9 comments
Closed

'withProps' on 'View' component crashes in release mode #525

hellogerard opened this issue Sep 21, 2017 · 9 comments
Labels

Comments

@hellogerard
Copy link

hellogerard commented Sep 21, 2017

This one is a doozy. I had a simple block component built by styling a standard View component. This was the basic idea:

const Block = withProps({
  style: { height: 100, width: 100, backgroundColor: 'blue' },
})(View);

export default Block;

This worked fine in debug mode. In release mode, withProps caused a crash. Here is the trace from Xcode:

2017-09-20 21:55:25.918 [error][tid:com.facebook.react.JavaScript] undefined is not an object (evaluating 'this._currentElement = element')
2017-09-20 21:55:25.921 [fatal][tid:com.facebook.react.ExceptionsManagerQueue] Unhandled JS Exception: undefined is not an object (evaluating 'this._currentElement = element')
2017-09-20 21:55:25.929 RecomposeBug[67656:3088671] *** Terminating app due to uncaught exception 'RCTFatalException: Unhandled JS Exception: undefined is not an object (evaluating 'this._currentElement = element')', reason: 'Unhandled JS Exception: undefined is not an object (evaluating 'this._curre..., stack:
RCTView@32853:13
mountComponent@31640:44
mountComponent@31358:53
mountChildren@32740:66
initializeChildren@32819:45

Here is the same trace when running in debug mode, but with NODE_ENV = production.

screen shot 2017-09-20 at 10 03 29 pm

It also works if I use a different component (i.e. Text, ScrollView). So this is probably a super-edge case, but still I found it in the wild. The fact that it only occurs in release mode made this one very hard to track down.

I have a repo you can use to reproduce. If you open it in Xcode and hit Play, it will crash. But you can also just wrap a View in withProps anywhere and it seems to crash. I also noticed that wrapping a View in onlyUpdateForPropTypes also has this same issue.

https://github.com/hellogerard/RecomposeBug

EDIT: Meant to add my packages:

"react": "16.0.0-alpha.12",
"react-native": "0.48.3",
"recompose": "^0.25.0"

@conrad-vanl
Copy link

conrad-vanl commented Sep 21, 2017

Potential insight:

React-Native uses a different component for View whether it's in release mode or not: https://github.com/facebook/react-native/blob/master/Libraries/Components/View/View.js#L137-L140

RCTView references the native component, which I'm pretty sure does not have a render method defined on the prototype, which might point to some earlier issues, such as: #489

@istarkov
Copy link
Contributor

Any ideas about how we could check that its a class component ?https://github.com/acdlite/recompose/blob/master/src/packages/recompose/isClassComponent.js
Something like prototype has keys other than etc
We would like to accept a PR

@conrad-vanl
Copy link

@istarkov I'd love to put some thoughts against. Can you give some background about how you're using isClassComponent and why?

@istarkov
Copy link
Contributor

Mostly for this optimization https://github.com/acdlite/recompose/blob/4b37008169c193e30014f49073173ce23b61af90/src/packages/recompose/isReferentiallyTransparentFunctionComponent.js

Idea is that instead of creating new react elements on every HOC for some hocs we can replace element creation with direct function call on production env

As an example mapProps - it's functional HOC, let's have

compose(
  mapProps(fnA),
  mapProps(fnB)
)(Comp)

in dev it wil generate something like

const CompB = (props) => <Comp {...fnB(props)} />
const CompA = (props) => <CompB {...fnA(props)} />

But in release it will generate

const CompB = (props) => <Comp {...fnB(props)} />
const CompA = (props) => CompB(fnA(props))

So for components with isReferentiallyTransparentFunctionComponent true we do this optimisation as this reduces Vtree size and removes a call to createElement

@istarkov
Copy link
Contributor

render check was added by author of preact to make sure that recompose can work with preact too. Before we used check on some internal react class property which was not documented and I'm not sure it exists in React 16

@istarkov
Copy link
Contributor

@developit may be you have any ideas about how to check that something is React/Preact class component without breaking compatibility with preact? As seems like render can be not in the prototype (we have issues that some people binds render moving it out of prototype, or like in issue above no render on native view)

@istarkov
Copy link
Contributor

istarkov commented Sep 21, 2017

PS. I dont think that this optimisation somehow improves performance of real apps, may be in very rare usecases. Also there are babel plugins exists as I know with this optimization inside, in that case we can recommend to use babel plugin and remove it from code.

@istarkov
Copy link
Contributor

oops we cant, this is runtime optimisation :-(

@istarkov istarkov added the bug label Sep 26, 2017
@istarkov
Copy link
Contributor

istarkov commented Oct 6, 2017

#538 we removed Eager optimisations in 0.26 must work now

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

No branches or pull requests

3 participants