New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Move React Core Integration and Injection to the Core Repo #6338

Merged
merged 20 commits into from Apr 20, 2016

Conversation

Projects
None yet
5 participants
@sebmarkbage
Member

sebmarkbage commented Mar 25, 2016

This creates a new react-native-renderer package which contains the React integration for React Native. (Although in reality the files live in the react package just like for react-dom because builds.)

I moved the most core files over but I still want to see how we can further minimize the contract. The individual commits show the process.

Currently everything in src/renderers/native/ReactNative/__mocks__/ have to required from the platform.

This also contains some patches to make this compatible with 15.0. As well as enabling spread properties.

I still need to run further test in the React Native repo to ensure everything works as expected.

We might want to consider this as part of 15.0 to avoid publishing a 15.1 immediately after since we'd have to do a minor bump for this stuff.

@sebmarkbage sebmarkbage added this to the 15.0 milestone Mar 25, 2016

@iamdustan

This comment has been minimized.

Show comment
Hide comment
@iamdustan

iamdustan Mar 25, 2016

Contributor

I’m curious if you’ve considered doing the inverse of this and moving the DOM renderer out to it’s own repo rather than consolidating the FB renderers in here?

Contributor

iamdustan commented Mar 25, 2016

I’m curious if you’ve considered doing the inverse of this and moving the DOM renderer out to it’s own repo rather than consolidating the FB renderers in here?

@sebmarkbage

This comment has been minimized.

Show comment
Hide comment
@sebmarkbage

sebmarkbage Mar 25, 2016

Member

Once we have a more stable contract for renderers we might do that but currently it is unstable. It is too hard to iterate on the internals and keep them in sync when they're in separate repos. It is also too easy to create massive cross dependencies.

The idea isn't that these files will remain RN specific but to make more of this stuff shared and narrow the external contract.

Member

sebmarkbage commented Mar 25, 2016

Once we have a more stable contract for renderers we might do that but currently it is unstable. It is too hard to iterate on the internals and keep them in sync when they're in separate repos. It is also too easy to create massive cross dependencies.

The idea isn't that these files will remain RN specific but to make more of this stuff shared and narrow the external contract.

@iamdustan

This comment has been minimized.

Show comment
Hide comment
@iamdustan

iamdustan Mar 25, 2016

Contributor

Copy that. The React external contract is already so solid. It will be an amazing feat of engineering and API design whenever React gets to the point where the custom renderer contract is as stable and small as the public API.

Contributor

iamdustan commented Mar 25, 2016

Copy that. The React external contract is already so solid. It will be an amazing feat of engineering and API design whenever React gets to the point where the custom renderer contract is as stable and small as the public API.

Show outdated Hide outdated packages/react-native-renderer/package.json
},
"homepage": "https://facebook.github.io/react-native/",
"dependencies": {
"fbjs": "0.1.0-alpha.4",

This comment has been minimized.

@iamdustan

iamdustan Mar 25, 2016

Contributor

I’m assuming this is copied out of RN? The current release listed on https://www.npmjs.com/package/fbjs is v0.8.0-alpha.3

@iamdustan

iamdustan Mar 25, 2016

Contributor

I’m assuming this is copied out of RN? The current release listed on https://www.npmjs.com/package/fbjs is v0.8.0-alpha.3

This comment has been minimized.

@sebmarkbage

sebmarkbage Mar 25, 2016

Member

good call. This was actually long before fbjs was in RN. I should update this. @zpao Your warning didn't catch this. :)

@sebmarkbage

sebmarkbage Mar 25, 2016

Member

good call. This was actually long before fbjs was in RN. I should update this. @zpao Your warning didn't catch this. :)

Show outdated Hide outdated src/renderers/native/NodeHandle/NodeHandle.js
* +------------------------------------------------------------+
* | Platform Implementations |
* | ----------------------------------------- |
* | React Canvas | React DOM Worker | React DOM main |

This comment has been minimized.

@iamdustan

iamdustan Mar 25, 2016

Contributor

I see the increased sharing idea. This is exciting. :)

@iamdustan

iamdustan Mar 25, 2016

Contributor

I see the increased sharing idea. This is exciting. :)

This comment has been minimized.

@sebmarkbage

sebmarkbage Mar 25, 2016

Member

This is an old note. It's just copied over. I actually don't think this external API point will hold up.

@sebmarkbage

sebmarkbage Mar 25, 2016

Member

This is an old note. It's just copied over. I actually don't think this external API point will hold up.

var emptyObject = {};
class ReactNativePropRegistry {
static register(object: Object): number {

This comment has been minimized.

@iamdustan

iamdustan Mar 25, 2016

Contributor

will babel-plugin-transform-class-properties need to be added to this repo for this or will the RN packager handle transforming this yet?

@iamdustan

iamdustan Mar 25, 2016

Contributor

will babel-plugin-transform-class-properties need to be added to this repo for this or will the RN packager handle transforming this yet?

This comment has been minimized.

@zpao

zpao Apr 19, 2016

Member

It's a static method so I think we're ok.

@zpao

zpao Apr 19, 2016

Member

It's a static method so I think we're ok.

@facebook-github-bot

This comment has been minimized.

Show comment
Hide comment
@facebook-github-bot

facebook-github-bot Mar 25, 2016

@sebmarkbage updated the pull request.

facebook-github-bot commented Mar 25, 2016

@sebmarkbage updated the pull request.

Show outdated Hide outdated src/renderers/native/ReactNative/ReactNativeBaseComponent.js
var UIManager = require('UIManager');
var deepFreezeAndThrowOnMutationInDev = require('deepFreezeAndThrowOnMutationInDev');
var invariant = require('fbjs/lib/invariant');

This comment has been minimized.

@zpao

zpao Mar 25, 2016

Member

Pretty sure these are going to get rewritten to './fbjs/lib/invariant', might just want to make them 'invariant' for now.

@zpao

zpao Mar 25, 2016

Member

Pretty sure these are going to get rewritten to './fbjs/lib/invariant', might just want to make them 'invariant' for now.

This comment has been minimized.

@sebmarkbage

sebmarkbage Mar 25, 2016

Member

hehe. I should've pushed last night. Fixed this locally.

@sebmarkbage

sebmarkbage Mar 25, 2016

Member

hehe. I should've pushed last night. Fixed this locally.

var ReactReconciler = require('ReactReconciler');
var ReactUpdateQueue = require('ReactUpdateQueue');
var ReactUpdates = require('ReactUpdates');
var UIManager = require('UIManager');

This comment has been minimized.

@zpao

zpao Mar 25, 2016

Member

UIManager module doesn't exist. This will almost certainly be broken. I'm guessing there are a bunch of others in the same boat (presumably modules that aren't brought over from RN). We need to whitelist all of these in the map - https://github.com/facebook/react/blob/master/gulpfile.js#L33 (map is a key:value mapping, map: Object.assign({}, require('fbjs/module-map'), {UIManager: 'UIManager', etc})

@zpao

zpao Mar 25, 2016

Member

UIManager module doesn't exist. This will almost certainly be broken. I'm guessing there are a bunch of others in the same boat (presumably modules that aren't brought over from RN). We need to whitelist all of these in the map - https://github.com/facebook/react/blob/master/gulpfile.js#L33 (map is a key:value mapping, map: Object.assign({}, require('fbjs/module-map'), {UIManager: 'UIManager', etc})

This comment has been minimized.

@sebmarkbage

sebmarkbage Mar 25, 2016

Member

Also fixed this locally last night.

@sebmarkbage

sebmarkbage Mar 25, 2016

Member

Also fixed this locally last night.

nextElement: ReactElement,
containerTag: number,
callback?: ?(() => void)
): ?ReactComponent {

This comment has been minimized.

@zpao

zpao Mar 25, 2016

Member

Do we need to be generating the .flow.js files for RN to typecheck? We don't currently do that (because nothing else in React is typed).

@zpao

zpao Mar 25, 2016

Member

Do we need to be generating the .flow.js files for RN to typecheck? We don't currently do that (because nothing else in React is typed).

This comment has been minimized.

@sebmarkbage

sebmarkbage Mar 25, 2016

Member

hm. Good point. Either that or leave them untransformed for now, since RN will transform them anyway.

@sebmarkbage

sebmarkbage Mar 25, 2016

Member

hm. Good point. Either that or leave them untransformed for now, since RN will transform them anyway.

@zpao

This comment has been minimized.

Show comment
Hide comment
@zpao

zpao Mar 25, 2016

Member

We might want to consider this as part of 15.0

2 RCs deep is not really the time to drop this…

Is there a less aggressive thing we can do to make sure RN works with 15 and do something like this for 15.1? Also, looks like this is purely new stuff and doesn't touch any of our existing code that would be in the React package. Could we perhaps just ship React 15 without needing this and then clean this up and ship the new package, like next week.

Also also, how is this going to affect our internal syncing?

Member

zpao commented Mar 25, 2016

We might want to consider this as part of 15.0

2 RCs deep is not really the time to drop this…

Is there a less aggressive thing we can do to make sure RN works with 15 and do something like this for 15.1? Also, looks like this is purely new stuff and doesn't touch any of our existing code that would be in the React package. Could we perhaps just ship React 15 without needing this and then clean this up and ship the new package, like next week.

Also also, how is this going to affect our internal syncing?

@sebmarkbage

This comment has been minimized.

Show comment
Hide comment
@sebmarkbage

sebmarkbage Mar 25, 2016

Member

I'm still working on compatibility with RN. There are a lot of things broken with 15 in RN. That's exactly why we need to fix this since we've gone way too long without making it RN compatible.

Not sure yet if that would need a new release of react to fix it regardless.

Ideally we should only have to release a new version of react-native-renderer to do these fixes, but we can't because everything still actually lives in react and we don't have flat bundles that copies the relevant shared code to each renderer package. They still share the same modules.

Member

sebmarkbage commented Mar 25, 2016

I'm still working on compatibility with RN. There are a lot of things broken with 15 in RN. That's exactly why we need to fix this since we've gone way too long without making it RN compatible.

Not sure yet if that would need a new release of react to fix it regardless.

Ideally we should only have to release a new version of react-native-renderer to do these fixes, but we can't because everything still actually lives in react and we don't have flat bundles that copies the relevant shared code to each renderer package. They still share the same modules.

@zpao

This comment has been minimized.

Show comment
Hide comment
@zpao

zpao Mar 25, 2016

Member

we can't because everything still actually lives in react

Oh right… that.

Member

zpao commented Mar 25, 2016

we can't because everything still actually lives in react

Oh right… that.

@zpao

This comment has been minimized.

Show comment
Hide comment
@zpao

zpao Mar 25, 2016

Member

I'm still working on compatibility with RN. There are a lot of things broken with 15 in RN

Honestly, that makes me want to push to 15.1 even more. Do you expect any core API changes to accommodate RN? As long as our core API doesn't change and we wouldn't need to ship a v16, I'm leaning strongly towards just saying we get 15.0 out and do what ever else we need in 15.1, even if that happens relatively soon after. We've been sitting on this release for too long already and I don't really want to push a couple more weeks & do more RCs. That's the nice thing about having these major version numbers :)

Member

zpao commented Mar 25, 2016

I'm still working on compatibility with RN. There are a lot of things broken with 15 in RN

Honestly, that makes me want to push to 15.1 even more. Do you expect any core API changes to accommodate RN? As long as our core API doesn't change and we wouldn't need to ship a v16, I'm leaning strongly towards just saying we get 15.0 out and do what ever else we need in 15.1, even if that happens relatively soon after. We've been sitting on this release for too long already and I don't really want to push a couple more weeks & do more RCs. That's the nice thing about having these major version numbers :)

@sebmarkbage

This comment has been minimized.

Show comment
Hide comment
@sebmarkbage

sebmarkbage Mar 25, 2016

Member

Also also, how is this going to affect our internal syncing?

We might need to stub out the missing files to satisfy the module systems but other than that it shouldn't affect us. We can just pull in the files that won't be required.

As long as our core API doesn't change and we wouldn't need to ship a v16

I don't think we'll need to make it 16. We can't make it a patch though.

The minor could potentially break other renderers like React ART since, again, they still share the same internal modules. If that's the case it is arguable that it would need to be a v16 release.

Member

sebmarkbage commented Mar 25, 2016

Also also, how is this going to affect our internal syncing?

We might need to stub out the missing files to satisfy the module systems but other than that it shouldn't affect us. We can just pull in the files that won't be required.

As long as our core API doesn't change and we wouldn't need to ship a v16

I don't think we'll need to make it 16. We can't make it a patch though.

The minor could potentially break other renderers like React ART since, again, they still share the same internal modules. If that's the case it is arguable that it would need to be a v16 release.

@facebook-github-bot

This comment has been minimized.

Show comment
Hide comment
@facebook-github-bot

facebook-github-bot Mar 27, 2016

@sebmarkbage updated the pull request.

facebook-github-bot commented Mar 27, 2016

@sebmarkbage updated the pull request.

@zpao zpao modified the milestones: 15.x, 15.0 Mar 30, 2016

@facebook-github-bot

This comment has been minimized.

Show comment
Hide comment
@facebook-github-bot

facebook-github-bot Mar 30, 2016

@sebmarkbage updated the pull request.

facebook-github-bot commented Mar 30, 2016

@sebmarkbage updated the pull request.

@facebook-github-bot

This comment has been minimized.

Show comment
Hide comment
@facebook-github-bot

facebook-github-bot Apr 5, 2016

@sebmarkbage updated the pull request.

facebook-github-bot commented Apr 5, 2016

@sebmarkbage updated the pull request.

@facebook-github-bot

This comment has been minimized.

Show comment
Hide comment
@facebook-github-bot

facebook-github-bot Apr 5, 2016

@sebmarkbage updated the pull request.

facebook-github-bot commented Apr 5, 2016

@sebmarkbage updated the pull request.

@sebmarkbage

This comment has been minimized.

Show comment
Hide comment
@sebmarkbage

sebmarkbage Apr 5, 2016

Member

This now works with RN. Haven't tested the Flow stuff yet. That's next. I believe we can manually release this as a separate package by copying files over to the react-native-renderer package.

Member

sebmarkbage commented Apr 5, 2016

This now works with RN. Haven't tested the Flow stuff yet. That's next. I believe we can manually release this as a separate package by copying files over to the react-native-renderer package.

Show outdated Hide outdated src/renderers/native/ReactNative/ReactNative.js
@@ -15,61 +15,16 @@
// the JS environment

This comment has been minimized.

@sophiebits

sophiebits Apr 9, 2016

Member

Commit message says you added a shim for isomorphic but I don't see it here.

@sophiebits

sophiebits Apr 9, 2016

Member

Commit message says you added a shim for isomorphic but I don't see it here.

This comment has been minimized.

@sebmarkbage

sebmarkbage Apr 9, 2016

Member

Ah, sorry. This got removed when I rebased on top of latest 15.0 changes in which React.js is already isomorphic. It wasn't before.

@sebmarkbage

sebmarkbage Apr 9, 2016

Member

Ah, sorry. This got removed when I rebased on top of latest 15.0 changes in which React.js is already isomorphic. It wasn't before.

Show outdated Hide outdated src/renderers/native/ReactNative/__mocks__/merge.js
// TODO: Replace Object.assign callers with requires
Object.assign = require('Object.assign');

This comment has been minimized.

@sophiebits

sophiebits Apr 9, 2016

Member

Why here?

@sophiebits

sophiebits Apr 9, 2016

Member

Why here?

This comment has been minimized.

@sebmarkbage

sebmarkbage Apr 9, 2016

Member

This is gone now since we have the transform doing this. I guess I didn't push that change.

@sebmarkbage

sebmarkbage Apr 9, 2016

Member

This is gone now since we have the transform doing this. I guess I didn't push that change.

Show outdated Hide outdated src/renderers/native/ReactNative/ReactNativeBaseComponent.js
this.viewConfig.uiViewClassName + ' with rootID ' + rootID);
}
var nativeTopRootTag = nativeContainerInfo._tag;
console.log('mountInCmp', nativeContainerInfo, nativeTopRootTag);

This comment has been minimized.

@sophiebits
@sophiebits

This comment has been minimized.

@sebmarkbage

sebmarkbage Apr 9, 2016

Member

This is on an outdated version. Not sure how you got to it.

@sebmarkbage

sebmarkbage Apr 9, 2016

Member

This is on an outdated version. Not sure how you got to it.

This comment has been minimized.

@sophiebits

sophiebits Apr 9, 2016

Member

I was going commit by commit.

@sophiebits

sophiebits Apr 9, 2016

Member

I was going commit by commit.

This comment has been minimized.

@sophiebits

sophiebits Apr 9, 2016

Member

Otherwise there's no way to distinguish the copying from what you did AFAIK.

@sophiebits

sophiebits Apr 9, 2016

Member

Otherwise there's no way to distinguish the copying from what you did AFAIK.

Show outdated Hide outdated src/renderers/native/ReactNative/ReactNativeTextComponent.js
@@ -57,10 +57,9 @@ assign(ReactNativeTextComponent.prototype, {
var nextStringText = '' + nextText;
if (nextStringText !== this._stringText) {
this._stringText = nextStringText;
console.log('receiveComponent', this, this._rootNodeID);

This comment has been minimized.

@sophiebits
@sophiebits
@sophiebits

This comment has been minimized.

Show comment
Hide comment
@sophiebits

sophiebits Apr 9, 2016

Member

All looks good I think.

Member

sophiebits commented Apr 9, 2016

All looks good I think.

@facebook-github-bot

This comment has been minimized.

Show comment
Hide comment
@facebook-github-bot

facebook-github-bot Apr 9, 2016

@sebmarkbage updated the pull request.

facebook-github-bot commented Apr 9, 2016

@sebmarkbage updated the pull request.

componentInstance,
transaction,
null,
ReactNativeContainerInfo(containerTag),

This comment has been minimized.

@sebmarkbage
@facebook-github-bot

This comment has been minimized.

Show comment
Hide comment
@facebook-github-bot

facebook-github-bot Apr 13, 2016

@sebmarkbage updated the pull request.

facebook-github-bot commented Apr 13, 2016

@sebmarkbage updated the pull request.

@facebook-github-bot

This comment has been minimized.

Show comment
Hide comment
@facebook-github-bot

facebook-github-bot Apr 13, 2016

@sebmarkbage updated the pull request.

facebook-github-bot commented Apr 13, 2016

@sebmarkbage updated the pull request.

@facebook-github-bot

This comment has been minimized.

Show comment
Hide comment
@facebook-github-bot

facebook-github-bot Apr 18, 2016

@sebmarkbage updated the pull request.

facebook-github-bot commented Apr 18, 2016

@sebmarkbage updated the pull request.

@sebmarkbage sebmarkbage changed the title from RFC: Move React Core Integration and Injection to the Core Repo to Move React Core Integration and Injection to the Core Repo Apr 18, 2016

@facebook-github-bot

This comment has been minimized.

Show comment
Hide comment
@facebook-github-bot

facebook-github-bot Apr 18, 2016

@sebmarkbage updated the pull request.

facebook-github-bot commented Apr 18, 2016

@sebmarkbage updated the pull request.

@facebook-github-bot

This comment has been minimized.

Show comment
Hide comment
@facebook-github-bot

facebook-github-bot Apr 19, 2016

@sebmarkbage updated the pull request.

facebook-github-bot commented Apr 19, 2016

@sebmarkbage updated the pull request.

"version": "16.0.0-alpha",
"description": "React package for use inside react-native.",
"main": "index.js",
"repository": {

This comment has been minimized.

@zpao

zpao Apr 19, 2016

Member

Want to make it short and drop the object, just make it "facebook/react"?

@zpao

zpao Apr 19, 2016

Member

Want to make it short and drop the object, just make it "facebook/react"?

}
}
/**

This comment has been minimized.

@zpao

zpao Apr 19, 2016

Member

We're almost definitely going to lose docs on the RN site as a result. I guess we'll figure that out later.

@zpao

zpao Apr 19, 2016

Member

We're almost definitely going to lose docs on the RN site as a result. I guess we'll figure that out later.

sebmarkbage added some commits Aug 12, 2015

Strip isomorphic stuff from the ReactNative module
Also, add a shim for the isomorphic module for ios and android so that
react-native doesn't pull in react-dom when React is required.
Remove duplicates and move files out of native/vendor
Moving the event plugins into their dedicated folder

Removing the ExecutionEnvironment override. I will just have to fix where needed. Probably related to the Chrome debugger?
Move React Native platform files back
These files are really only related to the platform itself and not
the React integration so I'll move them back.
Add Object Spread Support to Our Babel Config
Won't really use this in prod code yet but I have a benchmark that
uses it.
Add test and mocks
Mock UIManager

Comment out dontMock that actually needs mocking
Add React Native Modules to module map + fix fbjs
Our module rewrite whitelist needs to ignore providesModule files
that are going to be required from the global React Native
environment.

We also need to add ReactDOM to providesModule since we removed it
from React Native.
Fix ReactNativeComponentEnvironment providesModule
This has a different file name from its providesModule. Screws up
our build scripts.
Build up a native component cache for event dispatching
Changes to event overloading structure
Listeners are not attached by ID in the API
...even though they technically still are attached by.
Don't try to get the target node for responder if there is no listener
This can happen in edge cases where he listeners are already
unmounted or not mounted yet or something.
Update devtools injection
This isn't actually used right now so I can't test it. Because the
Chrome devtools are broken for React Native. The Nuclide integration
is in the react-native repo.
Remove ReactDOM.native shim
This is causing build errors.

This should be in the downstream repo if anything.

Relay has its own shim that should be preferred.

@sebmarkbage sebmarkbage merged commit c84ad52 into facebook:master Apr 20, 2016

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
@facebook-github-bot

This comment has been minimized.

Show comment
Hide comment
@facebook-github-bot

facebook-github-bot Apr 20, 2016

@sebmarkbage updated the pull request.

facebook-github-bot commented Apr 20, 2016

@sebmarkbage updated the pull request.

zpao added a commit to zpao/react that referenced this pull request Apr 20, 2016

@zpao zpao modified the milestones: 15.0.x, 15.x Apr 20, 2016

zpao added a commit that referenced this pull request Apr 20, 2016

zpao added a commit that referenced this pull request Apr 20, 2016

Merge pull request #6338 from sebmarkbage/reactnative2
Move React Core Integration and Injection to the Core Repo
(cherry picked from commit c84ad52)

zpao added a commit that referenced this pull request Apr 20, 2016

Merge pull request #6553 from zpao/cleanuppkg
Clean up package.json after #6338
(cherry picked from commit bddecc9)

ghost pushed a commit to facebook/react-native that referenced this pull request Apr 21, 2016

Move React Core Integration to a Dependency
Summary:Adding the react native renderer dependency and various fixes to support React 15.

Don't use dispatchID for touchableHandleResponderGrant

This callback argument was removed because "IDs" no longer exist. Instead, we'll
use the tag from the event target.

The corresponding PR on React Core is: facebook/react#6338

Reviewed By: spicyj

Differential Revision: D3159788

fb-gh-sync-id: 60e5cd2aa0af69d83fcdac3dfde0a85a748cb7b9
fbshipit-source-id: 60e5cd2aa0af69d83fcdac3dfde0a85a748cb7b9

@zpao zpao added the semver-patch label Apr 22, 2016

@zpao zpao modified the milestones: 15.0.2, 15.0.x Apr 28, 2016

ptmt added a commit to ptmt/react-native that referenced this pull request May 9, 2016

Move React Core Integration to a Dependency
Summary:Adding the react native renderer dependency and various fixes to support React 15.

Don't use dispatchID for touchableHandleResponderGrant

This callback argument was removed because "IDs" no longer exist. Instead, we'll
use the tag from the event target.

The corresponding PR on React Core is: facebook/react#6338

Reviewed By: spicyj

Differential Revision: D3159788

fb-gh-sync-id: 60e5cd2aa0af69d83fcdac3dfde0a85a748cb7b9
fbshipit-source-id: 60e5cd2aa0af69d83fcdac3dfde0a85a748cb7b9

zebulgar added a commit to nightingale/react-native that referenced this pull request Jun 18, 2016

Move React Core Integration to a Dependency
Summary:Adding the react native renderer dependency and various fixes to support React 15.

Don't use dispatchID for touchableHandleResponderGrant

This callback argument was removed because "IDs" no longer exist. Instead, we'll
use the tag from the event target.

The corresponding PR on React Core is: facebook/react#6338

Reviewed By: spicyj

Differential Revision: D3159788

fb-gh-sync-id: 60e5cd2aa0af69d83fcdac3dfde0a85a748cb7b9
fbshipit-source-id: 60e5cd2aa0af69d83fcdac3dfde0a85a748cb7b9

samerce added a commit to iodine/react-native that referenced this pull request Aug 23, 2016

Move React Core Integration to a Dependency
Summary:Adding the react native renderer dependency and various fixes to support React 15.

Don't use dispatchID for touchableHandleResponderGrant

This callback argument was removed because "IDs" no longer exist. Instead, we'll
use the tag from the event target.

The corresponding PR on React Core is: facebook/react#6338

Reviewed By: spicyj

Differential Revision: D3159788

fb-gh-sync-id: 60e5cd2aa0af69d83fcdac3dfde0a85a748cb7b9
fbshipit-source-id: 60e5cd2aa0af69d83fcdac3dfde0a85a748cb7b9

cpojer pushed a commit to facebook/metro that referenced this pull request Jan 26, 2017

Move React Core Integration to a Dependency
Summary:Adding the react native renderer dependency and various fixes to support React 15.

Don't use dispatchID for touchableHandleResponderGrant

This callback argument was removed because "IDs" no longer exist. Instead, we'll
use the tag from the event target.

The corresponding PR on React Core is: facebook/react#6338

Reviewed By: spicyj

Differential Revision: D3159788

fb-gh-sync-id: 60e5cd2aa0af69d83fcdac3dfde0a85a748cb7b9
fbshipit-source-id: 60e5cd2aa0af69d83fcdac3dfde0a85a748cb7b9
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment