Skip to content

[react-native] Pull up enableFastAddPropertiesInDiffing check #33043

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

Merged
merged 1 commit into from
Apr 29, 2025

Conversation

javache
Copy link
Member

@javache javache commented Apr 28, 2025

Summary

We don't need the isArray check for this experiment, as fastAddProperties already does the same. Also renaming slowAddProperties to make it clearer we can fully remove this codepath once fastAddProperties is fully rolled out.

How did you test this change?

yarn test packages/react-native-renderer -r=xplat --variant=true

@react-sizebot
Copy link

Comparing: c498bfc...45e125e

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.js = 6.68 kB 6.68 kB = 1.83 kB 1.83 kB
oss-stable/react-dom/cjs/react-dom-client.production.js = 527.72 kB 527.72 kB = 93.07 kB 93.07 kB
oss-experimental/react-dom/cjs/react-dom.production.js = 6.69 kB 6.69 kB = 1.83 kB 1.83 kB
oss-experimental/react-dom/cjs/react-dom-client.production.js = 633.34 kB 633.34 kB = 111.25 kB 111.25 kB
facebook-www/ReactDOM-prod.classic.js = 671.13 kB 671.13 kB = 117.70 kB 117.70 kB
facebook-www/ReactDOM-prod.modern.js = 661.41 kB 661.41 kB = 116.14 kB 116.14 kB

Significant size changes

Includes any change greater than 0.2%:

(No significant changes)

Generated by 🚫 dangerJS against 45e125e

@javache javache requested a review from rubennorte April 28, 2025 12:51
Copy link
Contributor

@rubennorte rubennorte left a comment

Choose a reason for hiding this comment

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

Looks good. Is this covered by tests?

Copy link

@Hardanish-Singh Hardanish-Singh left a comment

Choose a reason for hiding this comment

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

LGTM!

@javache
Copy link
Member Author

javache commented Apr 29, 2025

Looks good. Is this covered by tests?

Yup. Validated that when I broke this method in the experiment path, tests would fail.

@javache javache merged commit 0038c50 into facebook:main Apr 29, 2025
245 checks passed
@javache javache deleted the rn-fast-diff-properties branch April 29, 2025 10:10
github-actions bot pushed a commit that referenced this pull request Apr 29, 2025
## Summary

We don't need the isArray check for this experiment, as
`fastAddProperties` already does the same. Also renaming
slowAddProperties to make it clearer we can fully remove this codepath
once fastAddProperties is fully rolled out.

## How did you test this change?

```
yarn test packages/react-native-renderer -r=xplat --variant=true
```

DiffTrain build for [0038c50](0038c50)
github-actions bot pushed a commit to code/lib-react that referenced this pull request Apr 29, 2025
…ok#33043)

## Summary

We don't need the isArray check for this experiment, as
`fastAddProperties` already does the same. Also renaming
slowAddProperties to make it clearer we can fully remove this codepath
once fastAddProperties is fully rolled out.

## How did you test this change?

```
yarn test packages/react-native-renderer -r=xplat --variant=true
```

DiffTrain build for [0038c50](facebook@0038c50)
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