Remove RTR from ReactProfiler-test#28407

Merged

jackpope merged 6 commits intofacebook:mainfrom

jackpope:remove-rtr-usage-7

Mar 11, 2024

Contributor

jackpope commented Feb 21, 2024

Summary

Internal cleanup of ReactTestRenderer

How did you test this change?

yarn test packages/react/src/__tests__/ReactProfiler-test.internal.js

facebook-github-bot added CLA Signed React Core Team labels

react-sizebot commented Feb 21, 2024 •

edited

Loading

Comparing: 56e2005...c00d104

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	=	177.10 kB	177.10 kB	=	55.20 kB	55.20 kB
oss-experimental/react-dom/cjs/react-dom.production.min.js	=	177.64 kB	177.64 kB	=	55.53 kB	55.53 kB
facebook-www/ReactDOM-prod.classic.js	=	593.92 kB	593.92 kB	=	104.85 kB	104.85 kB
facebook-www/ReactDOM-prod.modern.js	=	577.18 kB	577.18 kB	=	101.91 kB	101.91 kB
test_utils/ReactAllWarnings.js	Deleted	66.60 kB	0.00 kB	Deleted	16.28 kB	0.00 kB

Significant size changes

Includes any change greater than 0.2%:

Expand to show

Name	+/-	Base	Current	+/- gzip	Base gzip	Current gzip
test_utils/ReactAllWarnings.js	Deleted	66.60 kB	0.00 kB	Deleted	16.28 kB	0.00 kB

Generated by 🚫 dangerJS against c00d104

jackpope commented

View reviewed changes

packages/react/src/__tests__/ReactProfiler-test.internal.js Outdated Show resolved Hide resolved

packages/react/src/__tests__/ReactProfiler-test.internal.js Outdated Show resolved Hide resolved

packages/react/src/__tests__/ReactProfiler-test.internal.js Outdated Show resolved Hide resolved

packages/react/src/__tests__/ReactProfiler-test.internal.js Outdated Show resolved Hide resolved

jackpope force-pushed the remove-rtr-usage-7 branch from 52b6489 to 6d766ed Compare

March 7, 2024 17:02

jackpope commented

View reviewed changes

packages/react/src/__tests__/ReactProfilerComponent-test.internal.js

		@@ -0,0 +1,137 @@
		/**

Contributor Author

jackpope Mar 7, 2024

Split this set of tests out into separate module because mixing ReactDOM and ReactNoop within the same test module can cause failures that are tough to debug.

packages/react/src/__tests__/ReactProfiler-test.internal.js

                 };
               }
-              describe('Profiler', () => {

Contributor Author

jackpope Mar 7, 2024

Moved to ReactProfilerComponent tests

packages/react/src/__tests__/ReactProfiler-test.internal.js

Comment on lines +172 to +185

+                    assertLog([
+                      'read current time',
+                      'read current time',
+                      'read current time',
+                      'read current time',
+                    ]);
                   } else {
                     assertLog([
                       'read current time',
                       'read current time',
                       'read current time',
                       'read current time',
                       'read current time',
+                      'read current time',
+                      'read current time',
+                      'read current time',

Contributor Author

jackpope Mar 7, 2024

These assertions have changed after moving from RTR to ReactNoop

Collaborator

eps1lon Mar 11, 2024

This is why we have the TODO above: https://github.com/facebook/react/pull/28407/files#diff-76a6c246baa2e1b7bb2de34aede8bf83b69884efcef1fecab903a7e0053b479fR167-R168

packages/react/src/__tests__/ReactProfiler-test.internal.js

Comment on lines -1217 to +1177

-                        // commit time
-                        expect(mountCall[5]).toBe(
+                        expect(mountCall[4]).toBe(
                           __DEV__ && replayFailedUnitOfWorkWithInvokeGuardedCallback
                             ? 54
                             : 44,
                         );
+                        // commit time
+                        expect(mountCall[5]).toBe(
+                          __DEV__ && replayFailedUnitOfWorkWithInvokeGuardedCallback
+                            ? 103
+                            : 83,
+                        );

Contributor Author

jackpope Mar 7, 2024

Additional timing changes

packages/react/src/__tests__/ReactProfiler-test.internal.js

                         expect(mountCall[3]).toBe(2);
                         // start time
-                        expect(mountCall[4]).toBe(5);
+                        expect(mountCall[4]).toBe(

Contributor Author

jackpope Mar 7, 2024

Timing changes after moving to ReactNoop

jackpope marked this pull request as ready for review

March 7, 2024 18:51

eps1lon reviewed

View reviewed changes

packages/react/src/__tests__/ReactProfiler-test.internal.js Outdated

Comment on lines 1088 to 1089

		// commit time: 5 initially + 14 of work
		// Add an additional 3 (ThrowsError) if we replayed the failed work

Collaborator

eps1lon Mar 8, 2024

This should be updated to reflect the changed behavior.

eps1lon reviewed

View reviewed changes

packages/react/src/__tests__/ReactProfiler-test.internal.js Outdated

Comment on lines 1108 to 1109

		// commit time: 19 (startTime) + 2 (ErrorBoundary) + 20 (AdvanceTime)
		// Add an additional 3 (ThrowsError) if we replayed the failed work

Collaborator

eps1lon Mar 8, 2024

Also outdated

Jack Pope added 6 commits

March 11, 2024 11:44


          Remove RTR from ReactProfiler-test

ac865b2


          Fix www test run from ci

e83292e


          Split DOM based tests

3a03645

c502f87

fa3f924


          Update comments

c00d104

jackpope force-pushed the remove-rtr-usage-7 branch from 30ed8a5 to c00d104 Compare

March 11, 2024 15:44

eps1lon approved these changes

View reviewed changes

jackpope merged commit eebdbf4 into facebook:main

jackpope deleted the remove-rtr-usage-7 branch

March 11, 2024 19:08

ztanner mentioned this pull request

Update React from 6c3b8dbfe to a4939017f vercel/next.js#63413

Closed

gnoff added a commit to gnoff/next.js that referenced this pull request


          Update React from 14898b6a9 to e373190fa

d674a96

- facebook/react#28596
- facebook/react#28625
- facebook/react#28616
- facebook/react#28491
- facebook/react#28583
- facebook/react#28427
- facebook/react#28613
- facebook/react#28599
- facebook/react#28611
- facebook/react#28610
- facebook/react#28606
- facebook/react#28598
- facebook/react#28549
- facebook/react#28557
- facebook/react#28467
- facebook/react#28591
- facebook/react#28459
- facebook/react#28590
- facebook/react#28564
- facebook/react#28582
- facebook/react#28579
- facebook/react#28578
- facebook/react#28521
- facebook/react#28550
- facebook/react#28576
- facebook/react#28577
- facebook/react#28571
- facebook/react#28572
- facebook/react#28560
- facebook/react#28569
- facebook/react#28573
- facebook/react#28546
- facebook/react#28568
- facebook/react#28562
- facebook/react#28566
- facebook/react#28565
- facebook/react#28559
- facebook/react#28508
- facebook/react#20432
- facebook/react#28555
- facebook/react#24730
- facebook/react#28472
- facebook/react#27991
- facebook/react#28514
- facebook/react#28548
- facebook/react#28526
- facebook/react#28515
- facebook/react#28533
- facebook/react#28532
- facebook/react#28531
- facebook/react#28407
- facebook/react#28522
- facebook/react#28538
- facebook/react#28509
- facebook/react#28534
- facebook/react#28527
- facebook/react#28528
- facebook/react#28519
- facebook/react#28411
- facebook/react#28520
- facebook/react#28518
- facebook/react#28493
- facebook/react#28504
- facebook/react#28499
- facebook/react#28501
- facebook/react#28496
- facebook/react#28471
- facebook/react#28351
- facebook/react#28486
- facebook/react#28490
- facebook/react#28488
- facebook/react#28468
- facebook/react#28321
- facebook/react#28477
- facebook/react#28479
- facebook/react#28480
- facebook/react#28478
- facebook/react#28464
- facebook/react#28475
- facebook/react#28456
- facebook/react#28319
- facebook/react#28345
- facebook/react#28337
- facebook/react#28335
- facebook/react#28466
- facebook/react#28462
- facebook/react#28322
- facebook/react#28444
- facebook/react#28448
- facebook/react#28449
- facebook/react#28446
- facebook/react#28447
- facebook/react#24580
- facebook/react#28514
- facebook/react#28548
- facebook/react#28526
- facebook/react#28515
- facebook/react#28533
- facebook/react#28532
- facebook/react#28531
- facebook/react#28407
- facebook/react#28522
- facebook/react#28538
- facebook/react#28509
- facebook/react#28534
- facebook/react#28527
- facebook/react#28528
- facebook/react#28519
- facebook/react#28411
- facebook/react#28520
- facebook/react#28518
- facebook/react#28493
- facebook/react#28504
- facebook/react#28499
- facebook/react#28501
- facebook/react#28496
- facebook/react#28471
- facebook/react#28351
- facebook/react#28486
- facebook/react#28490
- facebook/react#28488
- facebook/react#28468
- facebook/react#28321
- facebook/react#28477
- facebook/react#28479
- facebook/react#28480
- facebook/react#28478
- facebook/react#28464
- facebook/react#28475
- facebook/react#28456
- facebook/react#28319
- facebook/react#28345
- facebook/react#28337
- facebook/react#28335
- facebook/react#28466
- facebook/react#28462
- facebook/react#28322
- facebook/react#28444
- facebook/react#28448
- facebook/react#28449
- facebook/react#28446
- facebook/react#28447
- facebook/react#24580

gnoff mentioned this pull request

Update React from 14898b6a9 to e373190fa vercel/next.js#63685

Closed

gnoff added a commit to gnoff/next.js that referenced this pull request


          Update React from 14898b6a9 to e373190fa

1a2ce1f

- facebook/react#28596
- facebook/react#28625
- facebook/react#28616
- facebook/react#28491
- facebook/react#28583
- facebook/react#28427
- facebook/react#28613
- facebook/react#28599
- facebook/react#28611
- facebook/react#28610
- facebook/react#28606
- facebook/react#28598
- facebook/react#28549
- facebook/react#28557
- facebook/react#28467
- facebook/react#28591
- facebook/react#28459
- facebook/react#28590
- facebook/react#28564
- facebook/react#28582
- facebook/react#28579
- facebook/react#28578
- facebook/react#28521
- facebook/react#28550
- facebook/react#28576
- facebook/react#28577
- facebook/react#28571
- facebook/react#28572
- facebook/react#28560
- facebook/react#28569
- facebook/react#28573
- facebook/react#28546
- facebook/react#28568
- facebook/react#28562
- facebook/react#28566
- facebook/react#28565
- facebook/react#28559
- facebook/react#28508
- facebook/react#20432
- facebook/react#28555
- facebook/react#24730
- facebook/react#28472
- facebook/react#27991
- facebook/react#28514
- facebook/react#28548
- facebook/react#28526
- facebook/react#28515
- facebook/react#28533
- facebook/react#28532
- facebook/react#28531
- facebook/react#28407
- facebook/react#28522
- facebook/react#28538
- facebook/react#28509
- facebook/react#28534
- facebook/react#28527
- facebook/react#28528
- facebook/react#28519
- facebook/react#28411
- facebook/react#28520
- facebook/react#28518
- facebook/react#28493
- facebook/react#28504
- facebook/react#28499
- facebook/react#28501
- facebook/react#28496
- facebook/react#28471
- facebook/react#28351
- facebook/react#28486
- facebook/react#28490
- facebook/react#28488
- facebook/react#28468
- facebook/react#28321
- facebook/react#28477
- facebook/react#28479
- facebook/react#28480
- facebook/react#28478
- facebook/react#28464
- facebook/react#28475
- facebook/react#28456
- facebook/react#28319
- facebook/react#28345
- facebook/react#28337
- facebook/react#28335
- facebook/react#28466
- facebook/react#28462
- facebook/react#28322
- facebook/react#28444
- facebook/react#28448
- facebook/react#28449
- facebook/react#28446
- facebook/react#28447
- facebook/react#24580

ztanner mentioned this pull request

Update React from 14898b6a9 to a870b2d54 vercel/next.js#63976

Closed

EdisonVan pushed a commit to EdisonVan/react that referenced this pull request


          Remove RTR from ReactProfiler-test (facebook#28407)

ef7c685

## Summary

Internal cleanup of ReactTestRenderer

## How did you test this change?

`yarn test packages/react/src/__tests__/ReactProfiler-test.internal.js`

bigfootjon pushed a commit that referenced this pull request


          Remove RTR from ReactProfiler-test (#28407)

3ef7cbf

## Summary

Internal cleanup of ReactTestRenderer

## How did you test this change?

`yarn test packages/react/src/__tests__/ReactProfiler-test.internal.js`

DiffTrain build for commit eebdbf4.

eps1lon added a commit to vercel/next.js that referenced this pull request


          Update React from 14898b6a9 to c3048aab4.

fc58bbc

### React upstream changes

- facebook/react#28643
- facebook/react#28628
- facebook/react#28361
- facebook/react#28513
- facebook/react#28299
- facebook/react#28617
- facebook/react#28618
- facebook/react#28621
- facebook/react#28614
- facebook/react#28596
- facebook/react#28625
- facebook/react#28616
- facebook/react#28491
- facebook/react#28583
- facebook/react#28427
- facebook/react#28613
- facebook/react#28599
- facebook/react#28611
- facebook/react#28610
- facebook/react#28606
- facebook/react#28598
- facebook/react#28549
- facebook/react#28557
- facebook/react#28467
- facebook/react#28591
- facebook/react#28459
- facebook/react#28590
- facebook/react#28564
- facebook/react#28582
- facebook/react#28579
- facebook/react#28578
- facebook/react#28521
- facebook/react#28550
- facebook/react#28576
- facebook/react#28577
- facebook/react#28571
- facebook/react#28572
- facebook/react#28560
- facebook/react#28569
- facebook/react#28573
- facebook/react#28546
- facebook/react#28568
- facebook/react#28562
- facebook/react#28566
- facebook/react#28565
- facebook/react#28559
- facebook/react#28508
- facebook/react#20432
- facebook/react#28555
- facebook/react#24730
- facebook/react#28472
- facebook/react#27991
- facebook/react#28514
- facebook/react#28548
- facebook/react#28526
- facebook/react#28515
- facebook/react#28533
- facebook/react#28532
- facebook/react#28531
- facebook/react#28407
- facebook/react#28522
- facebook/react#28538
- facebook/react#28509
- facebook/react#28534
- facebook/react#28527
- facebook/react#28528
- facebook/react#28519
- facebook/react#28411
- facebook/react#28520
- facebook/react#28518
- facebook/react#28493
- facebook/react#28504
- facebook/react#28499
- facebook/react#28501
- facebook/react#28496
- facebook/react#28471
- facebook/react#28351
- facebook/react#28486
- facebook/react#28490
- facebook/react#28488
- facebook/react#28468
- facebook/react#28321
- facebook/react#28477
- facebook/react#28479
- facebook/react#28480
- facebook/react#28478
- facebook/react#28464
- facebook/react#28475
- facebook/react#28456
- facebook/react#28319
- facebook/react#28345
- facebook/react#28337
- facebook/react#28335
- facebook/react#28466
- facebook/react#28462
- facebook/react#28322
- facebook/react#28444
- facebook/react#28448
- facebook/react#28449
- facebook/react#28446
- facebook/react#28447
- facebook/react#24580
- facebook/react#28514
- facebook/react#28548
- facebook/react#28526
- facebook/react#28515
- facebook/react#28533
- facebook/react#28532
- facebook/react#28531
- facebook/react#28407
- facebook/react#28522
- facebook/react#28538
- facebook/react#28509
- facebook/react#28534
- facebook/react#28527
- facebook/react#28528
- facebook/react#28519
- facebook/react#28411
- facebook/react#28520
- facebook/react#28518
- facebook/react#28493
- facebook/react#28504
- facebook/react#28499
- facebook/react#28501
- facebook/react#28496
- facebook/react#28471
- facebook/react#28351
- facebook/react#28486
- facebook/react#28490
- facebook/react#28488
- facebook/react#28468
- facebook/react#28321
- facebook/react#28477
- facebook/react#28479
- facebook/react#28480
- facebook/react#28478
- facebook/react#28464
- facebook/react#28475
- facebook/react#28456
- facebook/react#28319
- facebook/react#28345
- facebook/react#28337
- facebook/react#28335
- facebook/react#28466
- facebook/react#28462
- facebook/react#28322
- facebook/react#28444
- facebook/react#28448
- facebook/react#28449
- facebook/react#28446
- facebook/react#28447
- facebook/react#24580

This was referenced Apr 19, 2024

Update React from 14898b6a9 to c3048aab4 vercel/next.js#64798

Merged

Cleanup replayFailedUnitOfWorkWithInvokeGuardedCallbackand enableProfilerNestedUpdateScheduledHook #28891

Merged

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

CLA Signed React Core Team