Allow async blocks in `to(Error|Warn)Dev` #25338

eps1lon · 2022-09-28T09:54:10Z

Summary

Allows async blocks in toWarnDev and toErrorDev blocks e.g.

await expect(async () => {
   await act(async () => {})
}).toWarnDev('Some warning')

Note that this means that expect(() => act()).toWarnDev is now async and should be either voided

-expect(() => act()).toWarnDev()
+expect(() => { act() }).toWarnDev()

or awaited.

-expect(() => act()).toWarnDev()
+await expect(() => act()).toWarnDev()

This will enable using toErrorDev in ReactDOMServerIntegration tests and allows explicitly asserting a warning we previously ignored in tests for useSyncExternalStore that should probably not exist in the first place?

How did you test this change?

CI

eps1lon · 2022-09-28T09:54:31Z

packages/use-sync-external-store/src/__tests__/useSyncExternalStoreShared-test.js

+        });
+      }).toWarnDev(
+        ReactFeatureFlags.enableUseRefAccessWarning
+          ? ['Warning: App: Unsafe read of a mutable value during render.']


Intended or should this be a TODO?

sizebot · 2022-09-28T09:59:03Z

Comparing: 15557fa...28a97ad

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	=	154.35 kB	154.35 kB	=	48.99 kB	48.99 kB
oss-experimental/react-dom/cjs/react-dom.production.min.js	=	156.28 kB	156.28 kB	=	49.61 kB	49.61 kB
facebook-www/ReactDOM-prod.classic.js	=	533.20 kB	533.20 kB	=	94.44 kB	94.44 kB
facebook-www/ReactDOM-prod.modern.js	=	518.30 kB	518.30 kB	=	92.26 kB	92.25 kB
facebook-www/ReactDOMForked-prod.classic.js	=	533.20 kB	533.20 kB	=	94.44 kB	94.44 kB

Significant size changes

Includes any change greater than 0.2%:

(No significant changes)

Generated by 🚫 dangerJS against 28a97ad

eps1lon · 2022-09-28T10:31:06Z

scripts/jest/matchers/toWarnDev.js

+          typeof result.then === 'function'
+        ) {
+          // `act` returns a thenable that can't be chained.
+          // Once `act(async () => {}).then(() => {}).then(() => {})` works


Should act() be chainable?

gnoff

fwiw I think this is a great change. maybe we should get someone else to also weigh in thought before merging?

eps1lon · 2022-11-19T10:22:21Z

fwiw I think this is a great change. maybe we should get someone else to also weigh in thought before merging?

Definitely. Code is a bit of a mess due to the nature of the change. So ideally either there is an existing pattern I should adhere to or others are fine with it.

Also have some questions that are revealed by this change that I'd like to touch upon (chainable act, uSES warnings).

eps1lon · 2022-12-01T10:26:27Z

Ok to merge as agreed during sync.

Summary: Three problems popped up during the sync: - facebook/react@07f46ecf2 breaks breaks tests - facebook/react@6fb8133ed breaks fbsource tests. I added a workaround and created a test for the team that owns the test. - https://fb.workplace.com/groups/flowlang/permalink/1198137807458547/ enables local type interference in fbsource but not in github React repo and some code breaks. Addressed in facebook/react#26064 This sync includes the following changes: - **[17f6912a4](facebook/react@17f6912a4 )**: Add flow types to ReactFiberHooks ([#25752](facebook/react#25752)) //<Samuel Susla>// - **[f101c2d0d](facebook/react@f101c2d0d )**: Remove Reconciler fork (2/2) ([#25775](facebook/react#25775)) //<Jan Kassens>// - **[420f0b7fa](facebook/react@420f0b7fa )**: Remove Reconciler fork (1/2) ([#25774](facebook/react#25774)) //<Jan Kassens>// - **[3ba7add60](facebook/react@3ba7add60 )**: Allow async blocks in `to(Error|Warn)Dev` ([#25338](facebook/react#25338)) //<Sebastian Silbermann>// - **[fa11bd6ec](facebook/react@fa11bd6ec )**: [ServerRenderer] Add option to send instructions as data attributes ([#25437](facebook/react#25437)) //<mofeiZ>// - **[e98225485](facebook/react@e98225485 )**: Add ref cleanup function ([#25686](facebook/react#25686)) //<Samuel Susla>// - **[15557fa67](facebook/react@15557fa67 )**: [Fix] properly track `useId` use in StrictMode in development ([#25713](facebook/react#25713)) //<Josh Story>// - **[8a23def32](facebook/react@8a23def32 )**: Resubmit Add HydrationSyncLane ([#25711](facebook/react#25711)) //<Tianyu Yao>// - **[2655c9354](facebook/react@2655c9354 )**: Fizz Browser: fix precomputed chunk being cleared on Node 18 ([#25645](facebook/react#25645)) //<Jimmy Lai>// - **[c08d8b804](facebook/react@c08d8b804 )**: Revert "Add SyncHydrationLane" ([#25708](facebook/react#25708)) //<Tianyu Yao>// - **[56ffca8b9](facebook/react@56ffca8b9 )**: Add Bun streaming server renderer ([#25597](facebook/react#25597)) //<Colin McDonnell>// - **[f31005d6a](facebook/react@f31005d6a )**: Add SyncHydrationLane ([#25698](facebook/react#25698)) //<Tianyu Yao>// - **[f284d9faf](facebook/react@f284d9faf )**: Track ThenableState alongside other hooks //<Andrew Clark>// - **[6b4c0314e](facebook/react@6b4c0314e )**: Check thenable instead of thenableState //<Andrew Clark>// - **[33e3d2878](facebook/react@33e3d2878 )**: Reuse hooks when replaying a suspended component //<Andrew Clark>// - **[4387d752d](facebook/react@4387d752d )**: Allow more hooks to be added when replaying mount //<Andrew Clark>// - **[5eb78d0a0](facebook/react@5eb78d0a0 )**: Pass ThenableState to replaySuspendedUnitOfWork //<Andrew Clark>// - **[4a2d86bdd](facebook/react@4a2d86bdd )**: Don't reset work loop until stack is unwound //<Andrew Clark>// - **[9dfbd9fa9](facebook/react@9dfbd9fa9 )**: use: Don't suspend if there are pending updates //<Andrew Clark>// - **[44c4e6f4d](facebook/react@44c4e6f4d )**: Force unwind work loop during selective hydration ([#25695](facebook/react#25695)) //<Andrew Clark>// - **[7b17f7bbf](facebook/react@7b17f7bbf )**: Enable warning for defaultProps on function components for everyone ([#25699](facebook/react#25699)) //<Sebastian Markbåge>// - **[6fb8133ed](facebook/react@6fb8133ed )**: Turn on string ref deprecation warning for everybody (not codemoddable) ([#25383](facebook/react#25383)) //<Sebastian Silbermann>// - **[07f46ecf2](facebook/react@07f46ecf2 )**: Turn on key spread warning in jsx-runtime for everyone ([#25697](facebook/react#25697)) //<Sebastian Markbåge>// - **[d65b88d03](facebook/react@d65b88d03 )**: Eagerly initialize an mutable object for instance.refs ([#25696](facebook/react#25696)) //<Sebastian Markbåge>// - **[c343f8025](facebook/react@c343f8025 )**: [react-float] feature detect getRootNode ([#25689](facebook/react#25689)) //<Jan Kassens>// - **[e1dd0a2f5](facebook/react@e1dd0a2f5 )**: Remove recoverable error when a sync update flows into a dehydrated boundary ([#25692](facebook/react#25692)) //<Sebastian Markbåge>// - **[c54e3541b](facebook/react@c54e3541b )**: [DevTools] bug fix for Hydrating fibers ([#25663](facebook/react#25663)) //<Mengdi Chen>// Changelog: [General][Changed] - React Native sync for revisions d1e35c7...17f6912 jest_e2e[run_all_tests] Reviewed By: makovkastar Differential Revision: D42804802 fbshipit-source-id: 6a9f00724cc73378025bbd04edb2d17760a87280

Summary: Three problems popped up during the sync: - facebook/react@07f46ecf2 breaks breaks tests - facebook/react@6fb8133ed breaks fbsource tests. I added a workaround and created a test for the team that owns the test. - https://fb.workplace.com/groups/flowlang/permalink/1198137807458547/ enables local type interference in fbsource but not in github React repo and some code breaks. Addressed in facebook/react#26064 This sync includes the following changes: - **[17f6912a4](facebook/react@17f6912a4 )**: Add flow types to ReactFiberHooks ([facebook#25752](facebook/react#25752)) //<Samuel Susla>// - **[f101c2d0d](facebook/react@f101c2d0d )**: Remove Reconciler fork (2/2) ([facebook#25775](facebook/react#25775)) //<Jan Kassens>// - **[420f0b7fa](facebook/react@420f0b7fa )**: Remove Reconciler fork (1/2) ([facebook#25774](facebook/react#25774)) //<Jan Kassens>// - **[3ba7add60](facebook/react@3ba7add60 )**: Allow async blocks in `to(Error|Warn)Dev` ([facebook#25338](facebook/react#25338)) //<Sebastian Silbermann>// - **[fa11bd6ec](facebook/react@fa11bd6ec )**: [ServerRenderer] Add option to send instructions as data attributes ([facebook#25437](facebook/react#25437)) //<mofeiZ>// - **[e98225485](facebook/react@e98225485 )**: Add ref cleanup function ([facebook#25686](facebook/react#25686)) //<Samuel Susla>// - **[15557fa67](facebook/react@15557fa67 )**: [Fix] properly track `useId` use in StrictMode in development ([facebook#25713](facebook/react#25713)) //<Josh Story>// - **[8a23def32](facebook/react@8a23def32 )**: Resubmit Add HydrationSyncLane ([facebook#25711](facebook/react#25711)) //<Tianyu Yao>// - **[2655c9354](facebook/react@2655c9354 )**: Fizz Browser: fix precomputed chunk being cleared on Node 18 ([facebook#25645](facebook/react#25645)) //<Jimmy Lai>// - **[c08d8b804](facebook/react@c08d8b804 )**: Revert "Add SyncHydrationLane" ([facebook#25708](facebook/react#25708)) //<Tianyu Yao>// - **[56ffca8b9](facebook/react@56ffca8b9 )**: Add Bun streaming server renderer ([facebook#25597](facebook/react#25597)) //<Colin McDonnell>// - **[f31005d6a](facebook/react@f31005d6a )**: Add SyncHydrationLane ([facebook#25698](facebook/react#25698)) //<Tianyu Yao>// - **[f284d9faf](facebook/react@f284d9faf )**: Track ThenableState alongside other hooks //<Andrew Clark>// - **[6b4c0314e](facebook/react@6b4c0314e )**: Check thenable instead of thenableState //<Andrew Clark>// - **[33e3d2878](facebook/react@33e3d2878 )**: Reuse hooks when replaying a suspended component //<Andrew Clark>// - **[4387d752d](facebook/react@4387d752d )**: Allow more hooks to be added when replaying mount //<Andrew Clark>// - **[5eb78d0a0](facebook/react@5eb78d0a0 )**: Pass ThenableState to replaySuspendedUnitOfWork //<Andrew Clark>// - **[4a2d86bdd](facebook/react@4a2d86bdd )**: Don't reset work loop until stack is unwound //<Andrew Clark>// - **[9dfbd9fa9](facebook/react@9dfbd9fa9 )**: use: Don't suspend if there are pending updates //<Andrew Clark>// - **[44c4e6f4d](facebook/react@44c4e6f4d )**: Force unwind work loop during selective hydration ([facebook#25695](facebook/react#25695)) //<Andrew Clark>// - **[7b17f7bbf](facebook/react@7b17f7bbf )**: Enable warning for defaultProps on function components for everyone ([facebook#25699](facebook/react#25699)) //<Sebastian Markbåge>// - **[6fb8133ed](facebook/react@6fb8133ed )**: Turn on string ref deprecation warning for everybody (not codemoddable) ([facebook#25383](facebook/react#25383)) //<Sebastian Silbermann>// - **[07f46ecf2](facebook/react@07f46ecf2 )**: Turn on key spread warning in jsx-runtime for everyone ([facebook#25697](facebook/react#25697)) //<Sebastian Markbåge>// - **[d65b88d03](facebook/react@d65b88d03 )**: Eagerly initialize an mutable object for instance.refs ([facebook#25696](facebook/react#25696)) //<Sebastian Markbåge>// - **[c343f8025](facebook/react@c343f8025 )**: [react-float] feature detect getRootNode ([facebook#25689](facebook/react#25689)) //<Jan Kassens>// - **[e1dd0a2f5](facebook/react@e1dd0a2f5 )**: Remove recoverable error when a sync update flows into a dehydrated boundary ([facebook#25692](facebook/react#25692)) //<Sebastian Markbåge>// - **[c54e3541b](facebook/react@c54e3541b )**: [DevTools] bug fix for Hydrating fibers ([facebook#25663](facebook/react#25663)) //<Mengdi Chen>// Changelog: [General][Changed] - React Native sync for revisions d1e35c7...17f6912 jest_e2e[run_all_tests] Reviewed By: makovkastar Differential Revision: D42804802 fbshipit-source-id: 6a9f00724cc73378025bbd04edb2d17760a87280

facebook-github-bot added the CLA Signed label Sep 28, 2022

eps1lon commented Sep 28, 2022

View reviewed changes

eps1lon force-pushed the test/async-toWarnDEv branch 2 times, most recently from feadce1 to a620062 Compare September 28, 2022 10:26

eps1lon commented Sep 28, 2022

View reviewed changes

eps1lon marked this pull request as ready for review September 28, 2022 10:32

eps1lon requested review from gaearon and acdlite September 28, 2022 10:32

eps1lon mentioned this pull request Sep 28, 2022

Upgrade jest to v29 #21575

Closed

1 task

gnoff approved these changes Nov 18, 2022

View reviewed changes

eps1lon added 4 commits November 28, 2022 20:02

Allow async to(Error|Warn)Dev

81cc91c

Work around act's thenable

549ed4c

Identify warning in useSyncExternalStoreShared-test

f485c5e

Resolve breaking changes

28a97ad

eps1lon force-pushed the test/async-toWarnDEv branch from a620062 to 28a97ad Compare November 28, 2022 19:03

eps1lon merged commit 3ba7add into facebook:main Dec 1, 2022

eps1lon deleted the test/async-toWarnDEv branch December 1, 2022 10:26

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Allow async blocks in `to(Error|Warn)Dev` #25338

Allow async blocks in `to(Error|Warn)Dev` #25338

eps1lon commented Sep 28, 2022 •

edited

Loading

eps1lon Sep 28, 2022

sizebot commented Sep 28, 2022 •

edited

Loading

eps1lon Sep 28, 2022

gnoff left a comment

eps1lon commented Nov 19, 2022

eps1lon commented Dec 1, 2022

Allow async blocks in to(Error|Warn)Dev #25338

Allow async blocks in to(Error|Warn)Dev #25338

Conversation

eps1lon commented Sep 28, 2022 • edited Loading

Summary

How did you test this change?

eps1lon Sep 28, 2022

Choose a reason for hiding this comment

sizebot commented Sep 28, 2022 • edited Loading

Critical size changes

Significant size changes

eps1lon Sep 28, 2022

Choose a reason for hiding this comment

gnoff left a comment

Choose a reason for hiding this comment

eps1lon commented Nov 19, 2022

eps1lon commented Dec 1, 2022

Allow async blocks in `to(Error|Warn)Dev` #25338

Allow async blocks in `to(Error|Warn)Dev` #25338

eps1lon commented Sep 28, 2022 •

edited

Loading

sizebot commented Sep 28, 2022 •

edited

Loading