React 16 migration

[necolas]

@gaearon posted an overview of the changes coming to React: facebook/react#8854

Things to figure out

1. What's the replacement for React Native's mixins, since React.createClass is going away?
2. How are we supposed to make custom renderers with Fiber?
3. What's the solution for ResponderEventPlugin / Touchable, since react-dom/lib/* files are going away and so might the synthetic event system?

[necolas]

Some notes Dan sent me:

• Renderer API
• ReactDOMFiber implementation
• ReactARTFiber
• Test renderer
• RNFiber
• NoopFiber

Think "Instance" = DOM node (or any object). So createInstance(), appendChild(), finalizeChildren(), insertBefore(), etc., specifies how to operate those instances. prepareUpdate() generates an object with changed properties based on prevProps and nextProps, and commitUpdate() applies it to the instance. Context things are used for SVG nesting.

[necolas]

I'm not going to pursue a custom renderer at this time.

The only module still using react-dom internals is the one injecting the ResponderEventPlugin[1].

import EventPluginHub from 'react-dom/lib/EventPluginHub';
import ResponderEventPlugin from 'react-dom/lib/ResponderEventPlugin';
import ResponderTouchHistoryStore from 'react-dom/lib/ResponderTouchHistoryStore';

@gaearon mentioned that the EventPluginHub is hanging off __SECRET_INTERNALS_DO_NOT_USE_OR_YOU_WILL_BE_FIRED in react-dom[2] and that it might be possible to expose more if needed. But the Responder modules are not used in the default react-dom export. From what I can tell, hey have non-trivial dependencies on other React internals that aren't exposed in the public release of react-dom, only the internal Facebook variant (see this patch).

List of dependencies needed by those 2 Responder files:

var EventPluginUtils = require('EventPluginUtils');
var EventPropagators = require('EventPropagators');
var ReactTreeTraversal = require('ReactTreeTraversal');
var ResponderSyntheticEvent = require('ResponderSyntheticEvent');
var accumulate = require('accumulate');

…none of them are available from the public export (they aren't used by react-dom from what I can remember). Therefore, I don't think forking the Responder modules is a good solution as it involves copy-pasting many modules out of React/DOM.

Next steps: find out how the ResponderEventPlugin can be injected when using react-dom (rather than creating a custom renderer).

[w4096]

I don't think we need ResponderEventPlugin.

Responder event is base on Touch Event, so we can use touch event handle responder event.

In View/Text render method, convert those event handler to touchEvent.

let touchEventHandles = convert({
    onStartShouldSetResponder,
    onMoveShouldSetResponder,
    onStartShouldSetResponderCapture,
    onMoveShouldSetResponderCapture,
    onResponderGrant,
    onResponderReject,
    onResponderStart,
    onResponderMove,
    onResponderEnd,
    onResponderRelease,
    onResponderTerminate,
    onResponderTerminationRequest,
});

touchEventHandles include:

• TouchStart
• TouchStartCapture
• TouchEnd
• TouchEndCapture
• TouchMove
• TouchMoveCapture

pass touchEventHandles to underlay div.

return (
  <div
    className={className} 
    {...touchEventHandler}
    style={style}
  >
    {children}
  </div>
);

We can copy ResponderTouchHistoryStore file directly. In this file they use three function isStartish, isMoveish, isEndish, We can copy those function from EventPluginUtils.

[fmoo]

So I've been playing with rnw on react@16 for the last few days, and I've run into two main issues that I was able to overcome, so I thought I'd share (if anyone else is stuck):

1. injectingResponderEventPlugin - This has been discussed a lot so far. I forked react and added ResponderEventPlugin and ResponderTouchHistoryStore to __SECRET_INTERNALS_DO_NOT_USE_OR_YOU_WILL_BE_FIRED, and pulled from there instead of lib and it basically Just Worked.

2. setValueForStyles - in development mode, warnValidStyle() looks at ReactStack internals (component._currentElement), but with Fiber, this is undefined and causes problems. I commented the implementation out in my local fork because I'm not too familiar with React's internals and wasn't sure how to keep the checks.

[necolas]

I don't think ResponderEventPlugin can hang off __SECRET_INTERNALS_DO_NOT_USE_OR_YOU_WILL_BE_FIRED as it will add to the react-dom bundle size but isn't used by react-dom. Perhaps ResponderEventPlugin can be an exception that is published to react-dom/lib (or somewhere else) and the few internal dependencies it shares with react-dom can hang off __SECRET_INTERNALS_DO_NOT_USE_OR_YOU_WILL_BE_FIRED. That's what I was planning to try first anyway.

Thanks, I didn't know about setValueForStyles. Probably we can either update the implementation to match what's in Fiber or remove those dev checks.

[joncursi]

Really looking forward to seeing this evolve. React Native, as of 0.46.1, is now shipping with 16.0.0-alpha.12. I just tried upgrading to this version on my web project but hit some errors around...

1. Styling

Uncaught TypeError: Cannot read property 'style' of null
    at Image.setNativeProps (http://localhost:3000/packages/modules.js?hash=0255d5e0b09eeaa924e020f2b357ba134b695bd5:67186:1560)
    at Object.ApplyAnimatedValues [as current] (http://localhost:3000/packages/modules.js?hash=0255d5e0b09eeaa924e020f2b357ba134b695bd5:91984:10)
    at AnimatedProps.callback [as _callback] (http://localhost:3000/packages/modules.js?hash=0255d5e0b09eeaa924e020f2b357ba134b695bd5:93969:35)
    at AnimatedProps.update (http://localhost:3000/packages/modules.js?hash=0255d5e0b09eeaa924e020f2b357ba134b695bd5:94118:6)
    at http://localhost:3000/packages/modules.js?hash=0255d5e0b09eeaa924e020f2b357ba134b695bd5:91624:69
    at Set.forEach (native)
    at _flush (http://localhost:3000/packages/modules.js?hash=0255d5e0b09eeaa924e020f2b357ba134b695bd5:91624:16)
    at AnimatedValue._updateValue (http://localhost:3000/packages/modules.js?hash=0255d5e0b09eeaa924e020f2b357ba134b695bd5:91771:1)
    at AnimatedValue.setValue (http://localhost:3000/packages/modules.js?hash=0255d5e0b09eeaa924e020f2b357ba134b695bd5:91665:6)
    at http://localhost:3000/app/app.js?hash=211f254af1297630757623342ff3c53ab74c829c:1504:25

2. the Animated API
3. Touchables are not responding to onPress

[fmoo]

@necolas - So, instead of pulling those objects into ReactDOM's internals, I created a new react-dom submodule, "react-dom/unstable-native-dependencies".

And here's an example rev that updates my local fork of react-native-web to use it

Does this approach seem sound? I wanted to hear back from you before putting up a PR to react.

[gaearon]

That’s what we planned to do, pretty much down to the naming!

[necolas]

Nice. I was thinking of doing something like that. Let's see if the React team are ok with it (edit: ^ looks like it :))

[gaearon]

Oh lol this is you Peter 😄 I didn't realize and thought it's a coincidence

[necolas]

Oh you work at Facebook too @fmoo! Excellent, thanks so much for moving this forward

[gaearon]

React team is spread a bit too thin right now so we're thankful to everyone who helps check items off the 16 release plan. 🎊

[mroswald]

@necolas Here's a proposal to get setValueForStyles react 16 compatible: NewStore@b8f84e2

You'd like me to open a PR for that? It's hard to add tests for that without mixing dependencies

[necolas]

I think we can either copy over the equivalent changes React has made to CSSPropertyOperations (e.g. https://github.com/facebook/react/blob/4b2eac3de7e1dbf5c2dd742fd9989974a83972cb/src/shared/utils/getComponentName.js) or consider exposing it from React as a few CSS libraries have copy-pasted those internals

[gaearon]

I’d like to avoid exposing for now. Don’t mind the copy paste.

[necolas]

The changes in the master branch of React should make this possible soon. Once a new alpha is published I'll try it out.

[gaearon]

We'll publish first beta in a few days.

[gaearon]

We did publish beta 1. facebook/react#10294

[L1fescape]

Sweet! Tried out @fmoo's changes to react-native-web along with react@next, react-dom@next, and react-native@0.46.4 and it works! 🎉

Are you guys planning on creating a PR to merge @fmoo's changes into react-native-web or are there different plans?

[necolas]

Once a new alpha is published I'll try it out.

I'm doing that now. The beta only just came out. Patience please :)

[necolas]

Although I've managed to get RNW to run on React 16, I'm not sure how to support both React 15 and React 16 at the same time.

[ericwooley]

@necolas Maybe bump react-native-web to version match RN.

So a plan would be to bump RNW immediately to 46, and below that version would be react 15?

All the alpha versions have been a huge nightmare for trying to use react-native-web, so maybe as far as the react-native-web is concerned, they just wouldn't be supported versions? The range that depends on alpha versions of react is 0.43-0.46.

I'm so glad it's over. This is what it had come down too in order to support both. https://github.com/ericwooley/react-nativeish/blob/upgrade-rn/package.json#L16

[necolas]

Yeah there's nothing I can do about RN releases depending on alpha/beta versions of React. And trying to stay entirely compatible with specific RN versions is going to be as impossible for RNW as it is for Preact with React.

[ericwooley]

So maybe publish a version of react-native-web@next for the 16 alphas?

[jlongster]

Is it important to continue supporting React 15 going forwards? You could publish a specific branch for React 16 in the short term, and when React 16 is officially released and gaining adoption merge that in and drop React 15 support. React 15 users can continue using the last version that supports it, and just like users won't get new features in React 15, they just won't get any new fixes here.

[TikiTDO]

@jlongster There are still a lot of third party libs that use React 15 or below, and it will take some time for those to migrate over once the release process for 16 is completed. Few of those would try to get the jump on migrating to an alpha/beta version of React.

When using those libs in conjunction with something that uses React 16, the packager just silently includes both the React platforms, which then leads into all sorts of creative and entertaining issues. It's not the biggest problem if you've experienced it a few times, and know to watch out for it, but a lot of people wouldn't which would lead to a support nightmare.

I think it's important to take these issues into consideration.

[rmevans9]

Hello @necolas!

I am excited to hear you got react-native-web working on React 16. I really would like to take that for a spin as I am evaluating react-native-web at work currently to see if it is a feasible option for something we are working on. Looking at the branches on this repo it doesn't look like you pushed up the react 16 version of react-native-web to a branch. How much effort would it take to push that up? If you can do that I will gladly put it through some testing here while evaluating the solution I am looking at.

Thanks!

[jaulz]

Any chance for a new branch? :)

[appden]

I have it running pretty well on top of React 16 except for it complaining about the hit-slop view not having a key. I'm using a Webpack alias to have it run without modifying react-native-web source.

That Webpack alias looks like this in the config:

'react-dom/lib': path.resolve(__dirname, './shims/react-dom/lib'),

shims/react-dom/lib/EventPluginHub.js

import ReactDOM from 'react-dom';

const { EventPluginHub } = ReactDOM.__SECRET_INTERNALS_DO_NOT_USE_OR_YOU_WILL_BE_FIRED;
export default EventPluginHub;

shims/react-dom/lib/ResponderEventPlugin.js

export { ResponderEventPlugin as default } from 'react-dom/unstable-native-dependencies';

shims/react-dom/lib/ResponderTouchHistoryStore.js

export { ResponderTouchHistoryStore as default } from 'react-dom/unstable-native-dependencies';

[cloudle]

Awesome update, was waiting for this for a while.. thanks @necolas.

[jaulz]

Great work - thanks a lot!