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

Add concurrentRoot property to ReactNativeTypes #21648

Merged
merged 3 commits into from
Jun 10, 2021

Conversation

sammy-SC
Copy link
Contributor

@sammy-SC sammy-SC commented Jun 8, 2021

Summary

I added concurrent root flag to render function for Fabric in #21552.

I forgot to add it to ReactNativeTypes for its Flow type. This PR fixes that.

Test Plan

Make sure the function signature from ReactNativeTypes.js aligns with function in ReactFabric.js.

@sammy-SC sammy-SC changed the title Add concurrentRoot property to ReactNativeTypes WIP Add concurrentRoot property to ReactNativeTypes Jun 8, 2021
@sammy-SC sammy-SC marked this pull request as draft June 8, 2021 19:17
@sammy-SC sammy-SC changed the title WIP Add concurrentRoot property to ReactNativeTypes Add concurrentRoot property to ReactNativeTypes Jun 8, 2021
@sizebot
Copy link

sizebot commented Jun 8, 2021

Comparing: 1a3f1af...18737a0

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 = 127.29 kB 127.29 kB = 40.81 kB 40.81 kB
oss-experimental/react-dom/cjs/react-dom.production.min.js = 130.11 kB 130.11 kB = 41.75 kB 41.75 kB
facebook-www/ReactDOM-prod.classic.js = 404.72 kB 404.72 kB = 74.90 kB 74.90 kB
facebook-www/ReactDOM-prod.modern.js = 393.27 kB 393.27 kB = 73.09 kB 73.09 kB
facebook-www/ReactDOMForked-prod.classic.js = 404.72 kB 404.72 kB = 74.90 kB 74.90 kB
react-native/shims/ReactFabric.js = 0.79 kB 0.76 kB = 0.43 kB 0.41 kB

Significant size changes

Includes any change greater than 0.2%:

Expand to show
Name +/- Base Current +/- gzip Base gzip Current gzip
react-native/shims/ReactNativeTypes.js +0.42% 7.11 kB 7.14 kB +0.65% 2.00 kB 2.01 kB
react-native/shims/ReactFabric.js = 0.79 kB 0.76 kB = 0.43 kB 0.41 kB

Generated by 🚫 dangerJS against 18737a0

@sammy-SC sammy-SC marked this pull request as ready for review June 9, 2021 11:31
@@ -182,6 +182,7 @@ export type ReactNativeType = {
element: Element<ElementType>,
containerTag: number,
callback: ?() => void,
concurrentRoot: ?boolean,
Copy link
Contributor

Choose a reason for hiding this comment

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

This param isn't on the Paper render, only Fabric.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Got it, I added it because Flow was still complaining but I found why.
Fabric is using Paper's type (https://github.com/facebook/react/blob/master/scripts/rollup/shims/react-native/ReactFabric.js#L16)

I changed it in 3744c12

I hope it was fine to remove the todo (cc @sebmarkbage).

Copy link
Contributor

Choose a reason for hiding this comment

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

Nice find.

@sammy-SC sammy-SC force-pushed the add-concurrent-root-parameter branch from 3744c12 to 18737a0 Compare June 10, 2021 08:20
@rickhanlonii rickhanlonii merged commit c96b78e into facebook:master Jun 10, 2021
@sammy-SC sammy-SC deleted the add-concurrent-root-parameter branch June 10, 2021 16:14
facebook-github-bot pushed a commit to facebook/react-native that referenced this pull request Jun 11, 2021
Summary:
This sync includes the following changes:
- **[c96b78e0e](facebook/react@c96b78e0e )**: Add concurrentRoot property to ReactNativeTypes ([#21648](facebook/react#21648)) //<Samuel Susla>//
- **[1a3f1afbd](facebook/react@1a3f1afbd )**: [React Native] Fabric get current event priority ([#21553](facebook/react#21553)) //<Samuel Susla>//
- **[48a11a3ef](facebook/react@48a11a3ef )**: Update next React version ([#21647](facebook/react#21647)) //<Andrew Clark>//
- **[5aa0c5671](facebook/react@5aa0c5671 )**: Fix Issue with Undefined Lazy Imports By Refactoring Lazy Initialization Order ([#21642](facebook/react#21642)) //<Sebastian Markbåge>//

Changelog:
[General][Changed] - React Native sync for revisions 0eea577...c96b78e

jest_e2e[run_all_tests]

Reviewed By: bvaughn

Differential Revision: D29029542

fbshipit-source-id: 9f2e19b4714b5697b5b846f2db8eb28c25420932
koto pushed a commit to koto/react that referenced this pull request Jun 15, 2021
* Add concurrentRoot property to ReactNativeTypes

* Add concurrentRoot to ReactNativeType

* Use ReactFabricType instead of ReactNativeType
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.

None yet

5 participants