Devtools: Unwrap Promise in useFormState #28319

eps1lon · 2024-02-13T16:51:13Z

Summary

Instead of just showing plain "Promise" after an async action, we now unwrap the fulfilled value of the Promise.

How did you test this change?

added case to devtools fixture:

Screen.Recording.2024-02-13.at.17.49.29.mov

eps1lon · 2024-02-13T16:52:09Z

packages/react-debug-tools/src/ReactDebugHooks.js

+          // If this was an uncached Promise we have to abandon this attempt
+          // but we can still emit anything up until this point.
+          hookLog.push({
+            primitive: 'Unresolved',
+            stackError: new Error(),
+            value: thenable,
+            debugInfo:
+              thenable._debugInfo === undefined ? null : thenable._debugInfo,
+          });
+          throw SuspenseException;


Followed the same pattern as #28297.

Do we actually reach this with useFormStatus? Asking mainly because SuspenseException currently includes a message that's specific to use

Even if instrumented, the promise could still be pending.

react-sizebot · 2024-02-13T16:55:00Z

Comparing: 2e470a7...adea250

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	=	176.83 kB	176.83 kB	=	55.11 kB	55.11 kB
oss-experimental/react-dom/cjs/react-dom.production.min.js	=	178.83 kB	178.83 kB	=	55.69 kB	55.69 kB
facebook-www/ReactDOM-prod.classic.js	=	592.18 kB	592.18 kB	=	104.43 kB	104.43 kB
facebook-www/ReactDOM-prod.modern.js	=	575.46 kB	575.46 kB	=	101.43 kB	101.43 kB
oss-experimental/react-debug-tools/cjs/react-debug-tools.development.js	+4.36%	27.66 kB	28.87 kB	+2.69%	7.36 kB	7.56 kB
oss-stable-semver/react-debug-tools/cjs/react-debug-tools.development.js	+4.36%	27.66 kB	28.87 kB	+2.69%	7.36 kB	7.56 kB
oss-stable/react-debug-tools/cjs/react-debug-tools.development.js	+4.36%	27.66 kB	28.87 kB	+2.69%	7.36 kB	7.56 kB
oss-experimental/react-debug-tools/cjs/react-debug-tools.production.min.js	+3.43%	9.07 kB	9.38 kB	+2.89%	3.21 kB	3.31 kB
oss-stable-semver/react-debug-tools/cjs/react-debug-tools.production.min.js	+3.43%	9.07 kB	9.38 kB	+2.89%	3.21 kB	3.31 kB
oss-stable/react-debug-tools/cjs/react-debug-tools.production.min.js	+3.43%	9.07 kB	9.38 kB	+2.89%	3.21 kB	3.31 kB
test_utils/ReactAllWarnings.js	Deleted	66.26 kB	0.00 kB	Deleted	16.32 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
oss-experimental/react-debug-tools/cjs/react-debug-tools.development.js	+4.36%	27.66 kB	28.87 kB	+2.69%	7.36 kB	7.56 kB
oss-stable-semver/react-debug-tools/cjs/react-debug-tools.development.js	+4.36%	27.66 kB	28.87 kB	+2.69%	7.36 kB	7.56 kB
oss-stable/react-debug-tools/cjs/react-debug-tools.development.js	+4.36%	27.66 kB	28.87 kB	+2.69%	7.36 kB	7.56 kB
oss-experimental/react-debug-tools/cjs/react-debug-tools.production.min.js	+3.43%	9.07 kB	9.38 kB	+2.89%	3.21 kB	3.31 kB
oss-stable-semver/react-debug-tools/cjs/react-debug-tools.production.min.js	+3.43%	9.07 kB	9.38 kB	+2.89%	3.21 kB	3.31 kB
oss-stable/react-debug-tools/cjs/react-debug-tools.production.min.js	+3.43%	9.07 kB	9.38 kB	+2.89%	3.21 kB	3.31 kB
test_utils/ReactAllWarnings.js	Deleted	66.26 kB	0.00 kB	Deleted	16.32 kB	0.00 kB

Generated by 🚫 dangerJS against adea250

sebmarkbage · 2024-02-13T21:39:12Z

packages/react-debug-tools/src/ReactDebugHooks.js

+          // If this was an uncached Promise we have to abandon this attempt
+          // but we can still emit anything up until this point.
+          hookLog.push({
+            primitive: 'Unresolved',


This needs a different name and you need to exhaust this case in the stack frame cache. See https://github.com/facebook/react/blob/main/packages/react-debug-tools/src/ReactDebugHooks.js#L111-L117

Otherwise this will generate the wrong stack frame information and so it'll generate a messed up tree.

Probably should add some tests for it.

Did that just like use but it's not clear to me how I could test it. It was working fine in the fixture without the case as far as I could tell.

I guess I would need to simulate an update in getPrimitiveStackCache because we never actually call useFormState with a thenable?

This needs a different name

Can it just use FormState? I don't understand the purpose of using different primitive names for the same hook.

It's annoying because this is used both for the key in the cache map and each one will have different stack traces. So it needs to be unique per stack. However, it's also used for the display name.

Which is why I gave it a unique primitive but then aliased it back:

https://github.com/facebook/react/blob/main/packages/react-debug-tools/src/ReactDebugHooks.js#L795

We could just make two separate fields for the name and the key potentially.

Since both your cases are in the same Hook you can also just unify them so that they have the same new Error().stack and ideally the same hookLog.push(...).

Unified stack error and hook log. I have to fight flow analysis more when unifying the hook log push more unless I tie up value and error into a single type like { value: Awaited<S>, error: null } | { value: Thenable, error: mixed }.

* feat[devtools]: symbolicate source for inspected element ([hoxyq](https://github.com/hoxyq) in [#28471](#28471)) * refactor[devtools]: lazily define source for fiber based on component stacks ([hoxyq](https://github.com/hoxyq) in [#28351](#28351)) * fix[devtools/tree/element]: onClick -> onMouseDown to handle first click correctly ([hoxyq](https://github.com/hoxyq) in [#28486](#28486)) * [DOM] disable legacy mode behind flag ([gnoff](https://github.com/gnoff) in [#28468](#28468)) * Fix Broken Links In Documentation ([justindhillon](https://github.com/justindhillon) in [#28321](#28321)) * Update /link URLs to react.dev ([rickhanlonii](https://github.com/rickhanlonii) in [#28477](#28477)) * [tests] add support for @GATE pragma ([gnoff](https://github.com/gnoff) in [#28479](#28479)) * Devtools: Unwrap Promise in useFormState ([eps1lon](https://github.com/eps1lon) in [#28319](#28319)) * Add support for rendering BigInt ([eps1lon](https://github.com/eps1lon) in [#24580](#24580)) * Include server component names in the componentStack in DEV ([sebmarkbage](https://github.com/sebmarkbage) in [#28415](#28415))

Basically just port facebook#28319 to avoid merge conflicts of this PR with main

* feat[devtools]: symbolicate source for inspected element ([hoxyq](https://github.com/hoxyq) in [facebook#28471](facebook#28471)) * refactor[devtools]: lazily define source for fiber based on component stacks ([hoxyq](https://github.com/hoxyq) in [facebook#28351](facebook#28351)) * fix[devtools/tree/element]: onClick -> onMouseDown to handle first click correctly ([hoxyq](https://github.com/hoxyq) in [facebook#28486](facebook#28486)) * [DOM] disable legacy mode behind flag ([gnoff](https://github.com/gnoff) in [facebook#28468](facebook#28468)) * Fix Broken Links In Documentation ([justindhillon](https://github.com/justindhillon) in [facebook#28321](facebook#28321)) * Update /link URLs to react.dev ([rickhanlonii](https://github.com/rickhanlonii) in [facebook#28477](facebook#28477)) * [tests] add support for @GATE pragma ([gnoff](https://github.com/gnoff) in [facebook#28479](facebook#28479)) * Devtools: Unwrap Promise in useFormState ([eps1lon](https://github.com/eps1lon) in [facebook#28319](facebook#28319)) * Add support for rendering BigInt ([eps1lon](https://github.com/eps1lon) in [facebook#24580](facebook#24580)) * Include server component names in the componentStack in DEV ([sebmarkbage](https://github.com/sebmarkbage) in [facebook#28415](facebook#28415))

DiffTrain build for commit fb10a2c.

facebook-github-bot added CLA Signed React Core Team Opened by a member of the React Core Team labels Feb 13, 2024

eps1lon commented Feb 13, 2024

View reviewed changes

eps1lon requested review from hoxyq and sebmarkbage February 13, 2024 16:56

eps1lon marked this pull request as ready for review February 13, 2024 16:56

sebmarkbage reviewed Feb 13, 2024

View reviewed changes

eps1lon force-pushed the feat/devtools/unwrap-useformstate branch 2 times, most recently from f1738f4 to 13c058a Compare February 13, 2024 22:36

eps1lon changed the title ~~Devtools: Unwrap Promise in useFormStatus~~ Devtools: Unwrap Promise in useFormState Feb 15, 2024

Sebastian Silbermann added 4 commits February 15, 2024 11:19

Devtools: Unwrap Promise in useFormState

6dfb9e0

exhaust cases when creating stack frame cache

d7aa714

Use same primitive name in form state hook log

ba90061

Unify stack error

4e77339

eps1lon force-pushed the feat/devtools/unwrap-useformstate branch from 80ad00b to d3c3fdb Compare February 15, 2024 10:19

Unify hooklog push

adea250

eps1lon force-pushed the feat/devtools/unwrap-useformstate branch from d3c3fdb to adea250 Compare February 15, 2024 10:34

sebmarkbage approved these changes Feb 28, 2024

View reviewed changes

eps1lon merged commit fb10a2c into facebook:main Feb 28, 2024
36 checks passed

eps1lon deleted the feat/devtools/unwrap-useformstate branch February 28, 2024 22:53

hoxyq mentioned this pull request Mar 5, 2024

React DevTools 5.0.1 -> 5.0.2 #28496

Merged

eps1lon pushed a commit to rickhanlonii/react that referenced this pull request Mar 5, 2024

Port latest useFormState debug-tools impl

2489101

Basically just port facebook#28319 to avoid merge conflicts of this PR with main

ztanner mentioned this pull request Mar 18, 2024

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

Closed

gnoff mentioned this pull request Mar 25, 2024

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

Closed

ztanner mentioned this pull request Apr 2, 2024

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

Closed

EdisonVan pushed a commit to EdisonVan/react that referenced this pull request Apr 15, 2024

Devtools: Unwrap Promise in useFormState (facebook#28319)

0144744

bigfootjon pushed a commit that referenced this pull request Apr 18, 2024

Devtools: Unwrap Promise in useFormState (#28319)

4942d00

DiffTrain build for commit fb10a2c.

eps1lon mentioned this pull request Apr 19, 2024

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

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Devtools: Unwrap Promise in useFormState #28319

Devtools: Unwrap Promise in useFormState #28319

eps1lon commented Feb 13, 2024

eps1lon Feb 13, 2024

sebmarkbage Feb 13, 2024

react-sizebot commented Feb 13, 2024 •

edited

Loading

sebmarkbage Feb 13, 2024 •

edited

Loading

eps1lon Feb 13, 2024

eps1lon Feb 13, 2024

sebmarkbage Feb 14, 2024

eps1lon Feb 15, 2024

Devtools: Unwrap Promise in useFormState #28319

Devtools: Unwrap Promise in useFormState #28319

Conversation

eps1lon commented Feb 13, 2024

Summary

How did you test this change?

eps1lon Feb 13, 2024

Choose a reason for hiding this comment

sebmarkbage Feb 13, 2024

Choose a reason for hiding this comment

react-sizebot commented Feb 13, 2024 • edited Loading

Critical size changes

Significant size changes

sebmarkbage Feb 13, 2024 • edited Loading

Choose a reason for hiding this comment

eps1lon Feb 13, 2024

Choose a reason for hiding this comment

eps1lon Feb 13, 2024

Choose a reason for hiding this comment

sebmarkbage Feb 14, 2024

Choose a reason for hiding this comment

eps1lon Feb 15, 2024

Choose a reason for hiding this comment

react-sizebot commented Feb 13, 2024 •

edited

Loading

sebmarkbage Feb 13, 2024 •

edited

Loading