Skip to content
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

Enable setJSResponder/setIsJSResponder for React Native Fabric #21439

Merged

Conversation

JoshuaGross
Copy link
Contributor

Summary

Reenable setJSResponder / setIsJSResponder for RN Fabric.

What could possibly go wrong?

Test Plan

We have tested this extensively internally and will test before landing any ReactJS sync to the RN repo.

@JoshuaGross JoshuaGross changed the title Enable setJSResponder/setIsJSResponder for React Native Enable setJSResponder/setIsJSResponder for React Native Fabric May 5, 2021
Copy link

@mdvacca mdvacca left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Accepting to unblock development and testing of JSResponder in React Native Fabric

@sizebot
Copy link

sizebot commented May 5, 2021

Comparing: 212d290...294c542

Critical size changes

Includes critical production bundles, as well as any change greater than 2%:

Name +/- Base Current +/- gzip Base gzip Current gzip
oss-stable/react-dom/cjs/react-dom.production.min.js = 122.53 kB 122.53 kB = 39.34 kB 39.34 kB
oss-experimental/react-dom/cjs/react-dom.production.min.js = 129.06 kB 129.06 kB = 41.43 kB 41.43 kB
facebook-www/ReactDOM-prod.classic.js = 406.59 kB 406.59 kB = 75.22 kB 75.22 kB
facebook-www/ReactDOM-prod.modern.js = 394.42 kB 394.42 kB = 73.28 kB 73.28 kB
facebook-www/ReactDOMForked-prod.classic.js = 406.59 kB 406.59 kB = 75.22 kB 75.22 kB

Significant size changes

Includes any change greater than 0.2%:

(No significant changes)

Generated by 🚫 dangerJS against 294c542

@sebmarkbage
Copy link
Collaborator

You need to add the method to react-native-host-hooks.js that defines the flow interface for nativeFabricUIManager.

Also, yarn prettier.

@sebmarkbage
Copy link
Collaborator

It also needs to be mocked in packages/react-native-renderer/src/__mocks__/react-native/Libraries/ReactPrivate/InitializeNativeFabricUIManager.js

@JoshuaGross JoshuaGross merged commit bd070eb into facebook:master May 6, 2021
facebook-github-bot pushed a commit to facebook/react-native that referenced this pull request May 11, 2021
Summary:
This sync includes the following changes:
- **[b8fda6cab](facebook/react@b8fda6cab )**: [React Native] Set allowConcurrentByDefault = true ([#21491](facebook/react#21491)) //<Ricky>//
- **[1bb8987cc](facebook/react@1bb8987cc )**: Renamed function in error log issue #21446 ([#21449](facebook/react#21449)) //<faebzz>//
- **[bd070eb2c](facebook/react@bd070eb2c )**: Enable setJSResponder/setIsJSResponder for React Native Fabric ([#21439](facebook/react#21439)) //<Joshua Gross>//
- **[e9a4a44aa](facebook/react@e9a4a44aa )**: Add back root override for strict mode ([#21428](facebook/react#21428)) //<Ricky>//
- **[d1542de3a](facebook/react@d1542de3a )**: Unify React.memo and React.forwardRef display name logic ([#21392](facebook/react#21392)) //<Brian Vaughn>//
- **[9a130e1de](facebook/react@9a130e1de )**: StrictMode includes strict effects by default ([#21418](facebook/react#21418)) //<Brian Vaughn>//
- **[15fb8c304](facebook/react@15fb8c304 )**: createRoot API is no longer strict by default ([#21417](facebook/react#21417)) //<Brian Vaughn>//
- **[aea7c2aab](facebook/react@aea7c2aab )**: Re-land "Support nesting of startTransition and flushSync (alt) ([#21149](facebook/react#21149))" //<Andrew Clark>//
- **[bacc87068](facebook/react@bacc87068 )**: Re-land "Flush discrete passive effects before paint ([#21150](facebook/react#21150))" //<Andrew Clark>//
- **[098600c42](facebook/react@098600c42 )**: Re-land "Fix: flushSync changes priority inside effect ([#21122](facebook/react#21122))" //<Andrew Clark>//
- **[df420bc0a](facebook/react@df420bc0a )**: Re-land "Delete LanePriority type ([#21090](facebook/react#21090))" //<Andrew Clark>//
- **[ab5b37927](facebook/react@ab5b37927 )**: Re-land "Clean up host pointers in level 2 of clean-up flag ([#21112](facebook/react#21112))" //<Andrew Clark>//
- **[fd907c1f1](facebook/react@fd907c1f1 )**: Re-land "Use highest priority lane to detect interruptions ([#21088](facebook/react#21088))"" //<Andrew Clark>//
- **[269dd6ec5](facebook/react@269dd6ec5 )**: subtreeFlag warning: Fix legacy suspense false positive ([#21388](facebook/react#21388)) //<Andrew Clark>//
- **[9e9dac650](facebook/react@9e9dac650 )**: Add unstable_concurrentUpdatesByDefault ([#21227](facebook/react#21227)) //<Ricky>//
- **[86f3385d9](facebook/react@86f3385d9 )**: Revert "Use highest priority lane to detect interruptions ([#21088](facebook/react#21088))" //<Andrew Clark>//
- **[c6702656f](facebook/react@c6702656f )**: Revert "Clean up host pointers in level 2 of clean-up flag ([#21112](facebook/react#21112))" //<Andrew Clark>//
- **[1bd41c664](facebook/react@1bd41c664 )**: Revert "Delete LanePriority type ([#21090](facebook/react#21090))" //<Andrew Clark>//
- **[e7e0a90bd](facebook/react@e7e0a90bd )**: Revert "Fix: flushSync changes priority inside effect ([#21122](facebook/react#21122))" //<Andrew Clark>//
- **[7bac7607a](facebook/react@7bac7607a )**: Revert "Flush discrete passive effects before paint ([#21150](facebook/react#21150))" //<Andrew Clark>//
- **[207d4c3a5](facebook/react@207d4c3a5 )**: Revert "Support nesting of startTransition and flushSync (alt) ([#21149](facebook/react#21149))" //<Andrew Clark>//

Changelog:
[General][Changed] - React Native sync for revisions 2a7bb41...b8fda6c

jest_e2e[run_all_tests]

Reviewed By: yungsters

Differential Revision: D28351439

fbshipit-source-id: 29620f96c9fb9f02b0d856111d3882d3c69fd1c5
koto pushed a commit to koto/react that referenced this pull request Jun 15, 2021
…ook#21439)

* Enable setJSResponder/setIsJSResponder for React Native

* yarn prettier

* add types to react-native-host-hooks

* yarn prettier

* mock setIsJSResponder
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants