Skip to content

[Transition Tracing] Code Cleanup #24880

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 2 commits into from
Jul 12, 2022
Merged

[Transition Tracing] Code Cleanup #24880

merged 2 commits into from
Jul 12, 2022

Conversation

lunaruan
Copy link
Contributor

@lunaruan lunaruan commented Jul 8, 2022

This PR cleans up some of the transition tracing code by:

  • Looping through marker transitions only when we process the markerComplete callback (rather than in the commit phase) so we block for less time during commit.
  • Renaming PendingSuspenseBoundaries to pendingBoundaries
  • Cleaning up the callback functions

@lunaruan lunaruan requested a review from acdlite July 8, 2022 19:46
@facebook-github-bot facebook-github-bot added CLA Signed React Core Team Opened by a member of the React Core Team labels Jul 8, 2022
@sizebot
Copy link

sizebot commented Jul 8, 2022 •
edited
Loading

Comparing: 5e8c196...8ed3491

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 = 133.08 kB 133.08 kB = 42.71 kB 42.71 kB
oss-experimental/react-dom/cjs/react-dom.production.min.js = 138.35 kB 138.35 kB = 44.27 kB 44.27 kB
facebook-www/ReactDOM-prod.classic.js = 468.15 kB 468.06 kB +0.03% 84.40 kB 84.43 kB
facebook-www/ReactDOM-prod.modern.js = 453.39 kB 453.30 kB +0.03% 82.16 kB 82.19 kB
facebook-www/ReactDOMForked-prod.classic.js = 468.15 kB 468.06 kB +0.03% 84.41 kB 84.43 kB

Significant size changes

Includes any change greater than 0.2%:

(No significant changes)

Generated by 🚫 dangerJS against 8ed3491

@lunaruan lunaruan force-pushed the refactor_callbacks branch from 95ba07b to 8ed3491 Compare July 12, 2022 19:10
@lunaruan lunaruan merged commit fa20b31 into facebook:main Jul 12, 2022
facebook-github-bot pushed a commit to facebook/react-native that referenced this pull request Sep 8, 2022
@sammy-SC @facebook-github-bot
React Native sync for revisions c1f5884...4ea064e
7b79dfc 
Summary:
This sync includes the following changes:
- **[4ea064eb0](facebook/react@4ea064eb0 )**: Don't fire passive effects during initial mount of a hidden Offscreen tree ([#24967](facebook/react#24967)) //<Andrew Clark>//
- **[2c7dea736](facebook/react@2c7dea736 )**: Implement Offscreen in Fizz ([#24988](facebook/react#24988)) //<Andrew Clark>//
- **[49f8254d6](facebook/react@49f8254d6 )**: Bug fix for <App /> vs. <Counter /> ([#24972](facebook/react#24972)) //<davidrenne>//
- **[6b28bc9c5](facebook/react@6b28bc9c5 )**: test: Throw custom error instead of relying on runtime error ([#24946](facebook/react#24946)) //<Sebastian Silbermann>//
- **[9bd0dd4c1](facebook/react@9bd0dd4c1 )**: test(react-debug-tools): Improve coverage of currentDispatcher.current setter ([#24945](facebook/react#24945)) //<Sebastian Silbermann>//
- **[59bc52a16](facebook/react@59bc52a16 )**: Add 4.5.0 release to eslint rules CHANGELOG ([#24853](facebook/react#24853)) //<Sebastian Silbermann>//
- **[cfb6cfa25](facebook/react@cfb6cfa25 )**: Reused components commit with timing as new ones //<Andrew Clark>//
- **[679eea328](facebook/react@679eea328 )**: Extract layout effects to separate functions //<Andrew Clark>//
- **[41287d447](facebook/react@41287d447 )**: Use recursion to traverse during "reappear layout" phase //<Andrew Clark>//
- **[697702bf3](facebook/react@697702bf3 )**: Use recursion to traverse during "disappear layout" phase //<Andrew Clark>//
- **[02206099a](facebook/react@02206099a )**: Use recursion to traverse during passive unmount phase ([#24918](facebook/react#24918)) //<Andrew Clark>//
- **[f62949519](facebook/react@f62949519 )**: [Transition Tracing] Rename transitionCallbacks to unstable_transitionCallbacks  ([#24920](facebook/react#24920)) //<Luna Ruan>//
- **[7a4336c40](facebook/react@7a4336c40 )**: Use recursion to traverse during passive mount phase //<Andrew Clark>//
- **[bb1357b38](facebook/react@bb1357b38 )**: Wrap try-catch directly around each user function //<Andrew Clark>//
- **[de3c06984](facebook/react@de3c06984 )**: Move flag check into each switch case //<Andrew Clark>//
- **[f5916d15b](facebook/react@f5916d15b )**: [Transition Tracing][Code Cleanup] Delete Marker Name Change Tests ([#24908](facebook/react#24908)) //<Luna Ruan>//
- **[fa20b319f](facebook/react@fa20b319f )**: [Transition Tracing] Code Cleanup ([#24880](facebook/react#24880)) //<Luna Ruan>//
- **[5e8c1961c](facebook/react@5e8c1961c )**: [Transition Tracing] onMarkerProgress ([#24861](facebook/react#24861)) //<Luna Ruan>//
- **[b641d0209](facebook/react@b641d0209 )**: Use recursion to traverse during layout phase //<Andrew Clark>//
- **[a1b1e391e](facebook/react@a1b1e391e )**: Wrap try-catch directly around each user function //<Andrew Clark>//
- **[3df7e8f5d](facebook/react@3df7e8f5d )**: Move flag check into each switch case //<Andrew Clark>//
- **[b8c96b136](facebook/react@b8c96b136 )**: Move ref commit effects inside switch statement //<Andrew Clark>//
- **[e225fa43a](facebook/react@e225fa43a )**: [Transition Tracing] Don't call transition callbacks if no transition name specified ([#24887](facebook/react#24887)) //<Luna Ruan>//
- **[dd2d65227](facebook/react@dd2d65227 )**: [Transition Tracing] Tracing Marker Name Change in Update Warning ([#24873](facebook/react#24873)) //<Luna Ruan>//
- **[80208e769](facebook/react@80208e769 )**: [Transition Tracing] Add onTransitionProgress Callback ([#24833](facebook/react#24833)) //<Luna Ruan>//
- **[30eb267ab](facebook/react@30eb267ab )**: Land forked reconciler changes ([#24878](facebook/react#24878)) //<Andrew Clark>//
- **[5e4e2dae0](facebook/react@5e4e2dae0 )**: Defer setState callbacks until component is visible ([#24872](facebook/react#24872)) //<Andrew Clark>//
- **[8e35b5060](facebook/react@8e35b5060 )**: [Transition Tracing] Refactor Code to Remove OffscreeInstance TODOs ([#24855](facebook/react#24855)) //<Luna Ruan>//
- **[deab1263a](facebook/react@deab1263a )**: [Transition Tracing] Change Transition Type Passed Pending Transitions ([#24856](facebook/react#24856)) //<Luna Ruan>//
- **[82e9e9909](facebook/react@82e9e9909 )**: Suspending inside a hidden tree should not cause fallbacks to appear ([#24699](facebook/react#24699)) //<Andrew Clark>//

Changelog:
[General][Changed] - React Native sync for revisions c1f5884...4ea064e

jest_e2e[run_all_tests]

Reviewed By: philIip, NickGerleman

Differential Revision: D39305648

fbshipit-source-id: 627ead5035c77fbc902b306e17897e425ad7fb99
OlimpiaZurek pushed a commit to OlimpiaZurek/react-native that referenced this pull request May 22, 2023
@sammy-SC @OlimpiaZurek
React Native sync for revisions c1f5884...4ea064e
1883835 
Summary:
This sync includes the following changes:
- **[4ea064eb0](facebook/react@4ea064eb0 )**: Don't fire passive effects during initial mount of a hidden Offscreen tree ([facebook#24967](facebook/react#24967)) //<Andrew Clark>//
- **[2c7dea736](facebook/react@2c7dea736 )**: Implement Offscreen in Fizz ([facebook#24988](facebook/react#24988)) //<Andrew Clark>//
- **[49f8254d6](facebook/react@49f8254d6 )**: Bug fix for <App /> vs. <Counter /> ([facebook#24972](facebook/react#24972)) //<davidrenne>//
- **[6b28bc9c5](facebook/react@6b28bc9c5 )**: test: Throw custom error instead of relying on runtime error ([facebook#24946](facebook/react#24946)) //<Sebastian Silbermann>//
- **[9bd0dd4c1](facebook/react@9bd0dd4c1 )**: test(react-debug-tools): Improve coverage of currentDispatcher.current setter ([facebook#24945](facebook/react#24945)) //<Sebastian Silbermann>//
- **[59bc52a16](facebook/react@59bc52a16 )**: Add 4.5.0 release to eslint rules CHANGELOG ([facebook#24853](facebook/react#24853)) //<Sebastian Silbermann>//
- **[cfb6cfa25](facebook/react@cfb6cfa25 )**: Reused components commit with timing as new ones //<Andrew Clark>//
- **[679eea328](facebook/react@679eea328 )**: Extract layout effects to separate functions //<Andrew Clark>//
- **[41287d447](facebook/react@41287d447 )**: Use recursion to traverse during "reappear layout" phase //<Andrew Clark>//
- **[697702bf3](facebook/react@697702bf3 )**: Use recursion to traverse during "disappear layout" phase //<Andrew Clark>//
- **[02206099a](facebook/react@02206099a )**: Use recursion to traverse during passive unmount phase ([facebook#24918](facebook/react#24918)) //<Andrew Clark>//
- **[f62949519](facebook/react@f62949519 )**: [Transition Tracing] Rename transitionCallbacks to unstable_transitionCallbacks  ([facebook#24920](facebook/react#24920)) //<Luna Ruan>//
- **[7a4336c40](facebook/react@7a4336c40 )**: Use recursion to traverse during passive mount phase //<Andrew Clark>//
- **[bb1357b38](facebook/react@bb1357b38 )**: Wrap try-catch directly around each user function //<Andrew Clark>//
- **[de3c06984](facebook/react@de3c06984 )**: Move flag check into each switch case //<Andrew Clark>//
- **[f5916d15b](facebook/react@f5916d15b )**: [Transition Tracing][Code Cleanup] Delete Marker Name Change Tests ([facebook#24908](facebook/react#24908)) //<Luna Ruan>//
- **[fa20b319f](facebook/react@fa20b319f )**: [Transition Tracing] Code Cleanup ([facebook#24880](facebook/react#24880)) //<Luna Ruan>//
- **[5e8c1961c](facebook/react@5e8c1961c )**: [Transition Tracing] onMarkerProgress ([facebook#24861](facebook/react#24861)) //<Luna Ruan>//
- **[b641d0209](facebook/react@b641d0209 )**: Use recursion to traverse during layout phase //<Andrew Clark>//
- **[a1b1e391e](facebook/react@a1b1e391e )**: Wrap try-catch directly around each user function //<Andrew Clark>//
- **[3df7e8f5d](facebook/react@3df7e8f5d )**: Move flag check into each switch case //<Andrew Clark>//
- **[b8c96b136](facebook/react@b8c96b136 )**: Move ref commit effects inside switch statement //<Andrew Clark>//
- **[e225fa43a](facebook/react@e225fa43a )**: [Transition Tracing] Don't call transition callbacks if no transition name specified ([facebook#24887](facebook/react#24887)) //<Luna Ruan>//
- **[dd2d65227](facebook/react@dd2d65227 )**: [Transition Tracing] Tracing Marker Name Change in Update Warning ([facebook#24873](facebook/react#24873)) //<Luna Ruan>//
- **[80208e769](facebook/react@80208e769 )**: [Transition Tracing] Add onTransitionProgress Callback ([facebook#24833](facebook/react#24833)) //<Luna Ruan>//
- **[30eb267ab](facebook/react@30eb267ab )**: Land forked reconciler changes ([facebook#24878](facebook/react#24878)) //<Andrew Clark>//
- **[5e4e2dae0](facebook/react@5e4e2dae0 )**: Defer setState callbacks until component is visible ([facebook#24872](facebook/react#24872)) //<Andrew Clark>//
- **[8e35b5060](facebook/react@8e35b5060 )**: [Transition Tracing] Refactor Code to Remove OffscreeInstance TODOs ([facebook#24855](facebook/react#24855)) //<Luna Ruan>//
- **[deab1263a](facebook/react@deab1263a )**: [Transition Tracing] Change Transition Type Passed Pending Transitions ([facebook#24856](facebook/react#24856)) //<Luna Ruan>//
- **[82e9e9909](facebook/react@82e9e9909 )**: Suspending inside a hidden tree should not cause fallbacks to appear ([facebook#24699](facebook/react#24699)) //<Andrew Clark>//

Changelog:
[General][Changed] - React Native sync for revisions c1f5884...4ea064e

jest_e2e[run_all_tests]

Reviewed By: philIip, NickGerleman

Differential Revision: D39305648

fbshipit-source-id: 627ead5035c77fbc902b306e17897e425ad7fb99
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@acdlite acdlite acdlite approved these changes

Assignees
No one assigned
Labels
CLA Signed React Core Team Opened by a member of the React Core Team
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@lunaruan @sizebot @acdlite @facebook-github-bot