Skip to content
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

DevTools: Add support for use(Promise) #28277

Closed
wants to merge 2 commits into from

Conversation

eps1lon
Copy link
Collaborator

@eps1lon eps1lon commented Feb 8, 2024

Summary

Adds support for Devtools to display the fulfilled value of promises passed to use.

The other cases would suspend so I don't think we need to handle them in devtools anyway.

How did you test this change?

Added new cases to shell under FunctionWithUse:

Screen.Recording.2024-02-08.at.16.22.21.mov

@facebook-github-bot facebook-github-bot added CLA Signed React Core Team Opened by a member of the React Core Team labels Feb 8, 2024
@react-sizebot
Copy link

react-sizebot commented Feb 8, 2024

Comparing: 04b5992...931780c

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.64 kB 176.64 kB = 55.02 kB 55.02 kB
oss-experimental/react-dom/cjs/react-dom.production.min.js = 178.63 kB 178.63 kB = 55.60 kB 55.60 kB
facebook-www/ReactDOM-prod.classic.js = 591.73 kB 591.73 kB = 104.44 kB 104.44 kB
facebook-www/ReactDOM-prod.modern.js = 575.51 kB 575.51 kB = 101.53 kB 101.53 kB
test_utils/ReactAllWarnings.js Deleted 67.02 kB 0.00 kB Deleted 16.42 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 +1.16% 25.26 kB 25.55 kB +0.44% 6.76 kB 6.79 kB
oss-stable-semver/react-debug-tools/cjs/react-debug-tools.development.js +1.16% 25.26 kB 25.55 kB +0.44% 6.76 kB 6.79 kB
oss-stable/react-debug-tools/cjs/react-debug-tools.development.js +1.16% 25.26 kB 25.55 kB +0.44% 6.76 kB 6.79 kB
oss-experimental/react-debug-tools/cjs/react-debug-tools.production.min.js +1.05% 7.83 kB 7.91 kB +0.71% 2.82 kB 2.84 kB
oss-stable-semver/react-debug-tools/cjs/react-debug-tools.production.min.js +1.05% 7.83 kB 7.91 kB +0.71% 2.82 kB 2.84 kB
oss-stable/react-debug-tools/cjs/react-debug-tools.production.min.js +1.05% 7.83 kB 7.91 kB +0.71% 2.82 kB 2.84 kB
test_utils/ReactAllWarnings.js Deleted 67.02 kB 0.00 kB Deleted 16.42 kB 0.00 kB

Generated by 🚫 dangerJS against 931780c

@eps1lon eps1lon marked this pull request as ready for review February 8, 2024 15:47
@eps1lon eps1lon requested review from acdlite and hoxyq February 8, 2024 15:47
Copy link
Contributor

@hoxyq hoxyq left a comment

Choose a reason for hiding this comment

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

Oh wow, this is interesting. I wonder how this is going to work if component is not wrapped with Suspense and usable is a pending Promise at the moment of hooks inspection.

Planning to test it with browser extension later today / early tomorrow, will follow-up

@eps1lon eps1lon force-pushed the feat/devtools/use-promise branch from f7d5353 to 931780c Compare February 8, 2024 17:48
@hoxyq
Copy link
Contributor

hoxyq commented Feb 9, 2024

Oh wow, this is interesting. I wonder how this is going to work if component is not wrapped with Suspense and usable is a pending Promise at the moment of hooks inspection.

Planning to test it with browser extension later today / early tomorrow, will follow-up

In order to parse hooks tree of component X, DevTools is calling the render function of X:

try {
ancestorStackError = new Error();
renderFunction(props);
} catch (error) {
handleRenderFunctionError(error);
} finally {
readHookLog = hookLog;
hookLog = [];
// $FlowFixMe[incompatible-use] found when upgrading Flow
currentDispatcher.current = previousDispatcher;
}
const rootStack = ErrorStackParser.parse(ancestorStackError);
return buildTree(rootStack, readHookLog, includeHooksSource);

In case if promise is in pending state at this stage, our patched use hook will throw usable, once render function is called.

We should probably also update this code and handleRenderFunctionError implementation to support this case with use.

@eps1lon
Copy link
Collaborator Author

eps1lon commented Feb 12, 2024

Better approach in #28297. Just going to cherry-pick the example from the devtools shell.

@eps1lon eps1lon closed this Feb 12, 2024
@eps1lon eps1lon deleted the feat/devtools/use-promise branch February 12, 2024 12:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CLA Signed React Core Team Opened by a member of the React Core Team
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants