Skip to content

Commit

Permalink
[DevTools] Make Element Inspection Feel Snappy (facebook#30555)
Browse files Browse the repository at this point in the history
There's two problems. The biggest one is that it turns out that Chrome
is throttling looping timers that we're using both while polling and for
batching bridge traffic. This means that bridge traffic a lot of the
time just slows down to 1 second at a time. No wonder it feels sluggish.
The only solution is to not use timers for this.

Even when it doesn't like in Firefox the batching into 100ms still feels
too sluggish.

The fix I use is to batch using a microtask instead so we can still
batch multiple commands sent in a single event but we never artificially
slow down an interaction.

I don't think we've reevaluated this for a long time since this was in
the initial commit of DevTools to this repo. If it causes other issues
we can follow up on those.

We really shouldn't use timers for debouncing and such. In fact, React
itself recommends against it because we have a better technique with
scheduling in Concurrent Mode. The correct way to implement this in the
bridge is using a form of back-pressure where we don't keep sending
messages until we get a message back and only send the last one that
matters. E.g. when moving the cursor over a the elements tab we
shouldn't let the backend one-by-one move the DOM node to each one we
have ever passed. We should just move to the last one we're currently
hovering over. But this can't be done at the bridge layer since it
doesn't know if it's a last-one-wins or imperative operation where each
one needs to be sent. It needs to be done higher. I'm not currently
seeing any perf problems with this new approach but I'm curious on React
Native or some thing. RN might need the back-pressure approach. That can
be a follow up if we ever find a test case.

Finally, the other problem is that we use a Suspense boundary around the
Element Inspection. Suspense boundaries are for things that are expected
to take a long time to load. This shows a loading state immediately. To
avoid flashing when it ends up being fast, React throttles the reveal to
200ms. This means that we take a minimum of 200ms to show the props. The
way to show fast async data in React is using a Transition (either using
startTransition or useDeferredValue). This lets the old value remaining
in place while we're loading the next one.

We already implement this using `inspectedElementID` which is the async
one. It would be more idiomatic to implement this with useDeferredValue
rather than the reducer we have now but same principle. We were just
using the wrong ID in a few places so when it synchronously updated they
suspended. So I just made them use the inspectedElementID instead.

Then I can simply remove the Suspense boundary. Now the selection
updates in the tree view synchronously and the sidebar lags a frame or
two but it feels instant. It doesn't flash to white between which is
key.

DiffTrain build for [4ea12a1](facebook@4ea12a1)
  • Loading branch information
syi0808 committed Aug 1, 2024
1 parent 92282b2 commit 1180f1b
Show file tree
Hide file tree
Showing 41 changed files with 19,378 additions and 20,684 deletions.
31 changes: 13 additions & 18 deletions compiled/facebook-www/JSXDEVRuntime-dev.classic.js
Original file line number Diff line number Diff line change
Expand Up @@ -185,8 +185,8 @@ __DEV__ &&
-1 < x.stack.indexOf("\n at")
? " (<anonymous>)"
: -1 < x.stack.indexOf("@")
? "@unknown:0:0"
: "";
? "@unknown:0:0"
: "";
}
return "\n" + prefix + name + suffix;
}
Expand Down Expand Up @@ -598,13 +598,15 @@ __DEV__ &&
null === type
? (isStaticChildren = "null")
: isArrayImpl(type)
? (isStaticChildren = "array")
: void 0 !== type && type.$$typeof === REACT_ELEMENT_TYPE
? ((isStaticChildren =
"<" + (getComponentNameFromType(type.type) || "Unknown") + " />"),
(children =
" Did you accidentally export a JSX literal instead of a component?"))
: (isStaticChildren = typeof type);
? (isStaticChildren = "array")
: void 0 !== type && type.$$typeof === REACT_ELEMENT_TYPE
? ((isStaticChildren =
"<" +
(getComponentNameFromType(type.type) || "Unknown") +
" />"),
(children =
" Did you accidentally export a JSX literal instead of a component?"))
: (isStaticChildren = typeof type);
error(
"React.jsx: type is invalid -- expected a string (for built-in components) or a class/function (for composite components) but got: %s.%s",
isStaticChildren,
Expand Down Expand Up @@ -638,11 +640,7 @@ __DEV__ &&
hasValidKey(config) &&
(checkKeyStringCoercion(config.key), (children = "" + config.key));
hasValidRef(config) && warnIfStringRefCannotBeAutoConverted(config, self);
if (
(!enableFastJSXWithoutStringRefs &&
(!enableFastJSXWithStringRefs || "ref" in config)) ||
"key" in config
) {
if ("ref" in config || "key" in config) {
maybeKey = {};
for (var propName in config)
"key" !== propName &&
Expand Down Expand Up @@ -813,7 +811,6 @@ __DEV__ &&
disableDefaultPropsExceptForClasses =
dynamicFeatureFlags.disableDefaultPropsExceptForClasses,
enableDebugTracing = dynamicFeatureFlags.enableDebugTracing,
enableFastJSX = dynamicFeatureFlags.enableFastJSX,
enableRenderableContext = dynamicFeatureFlags.enableRenderableContext,
enableTransitionTracing = dynamicFeatureFlags.enableTransitionTracing,
renameElementSymbol = dynamicFeatureFlags.renameElementSymbol,
Expand Down Expand Up @@ -867,9 +864,7 @@ __DEV__ &&
specialPropKeyWarningShown;
var didWarnAboutStringRefs = {};
var didWarnAboutElementRef = {};
var enableFastJSXWithStringRefs = enableFastJSX && !0,
enableFastJSXWithoutStringRefs = enableFastJSXWithStringRefs && !1,
didWarnAboutKeySpread = {},
var didWarnAboutKeySpread = {},
ownerHasKeyUseWarning = {};
exports.Fragment = REACT_FRAGMENT_TYPE;
exports.jsxDEV = function (
Expand Down
31 changes: 13 additions & 18 deletions compiled/facebook-www/JSXDEVRuntime-dev.modern.js
Original file line number Diff line number Diff line change
Expand Up @@ -185,8 +185,8 @@ __DEV__ &&
-1 < x.stack.indexOf("\n at")
? " (<anonymous>)"
: -1 < x.stack.indexOf("@")
? "@unknown:0:0"
: "";
? "@unknown:0:0"
: "";
}
return "\n" + prefix + name + suffix;
}
Expand Down Expand Up @@ -595,13 +595,15 @@ __DEV__ &&
null === type
? (isStaticChildren = "null")
: isArrayImpl(type)
? (isStaticChildren = "array")
: void 0 !== type && type.$$typeof === REACT_ELEMENT_TYPE
? ((isStaticChildren =
"<" + (getComponentNameFromType(type.type) || "Unknown") + " />"),
(children =
" Did you accidentally export a JSX literal instead of a component?"))
: (isStaticChildren = typeof type);
? (isStaticChildren = "array")
: void 0 !== type && type.$$typeof === REACT_ELEMENT_TYPE
? ((isStaticChildren =
"<" +
(getComponentNameFromType(type.type) || "Unknown") +
" />"),
(children =
" Did you accidentally export a JSX literal instead of a component?"))
: (isStaticChildren = typeof type);
error(
"React.jsx: type is invalid -- expected a string (for built-in components) or a class/function (for composite components) but got: %s.%s",
isStaticChildren,
Expand Down Expand Up @@ -635,11 +637,7 @@ __DEV__ &&
hasValidKey(config) &&
(checkKeyStringCoercion(config.key), (children = "" + config.key));
hasValidRef(config) && warnIfStringRefCannotBeAutoConverted(config, self);
if (
(!enableFastJSXWithoutStringRefs &&
(!enableFastJSXWithStringRefs || "ref" in config)) ||
"key" in config
) {
if ("ref" in config || "key" in config) {
maybeKey = {};
for (var propName in config)
"key" !== propName &&
Expand Down Expand Up @@ -810,7 +808,6 @@ __DEV__ &&
disableDefaultPropsExceptForClasses =
dynamicFeatureFlags.disableDefaultPropsExceptForClasses,
enableDebugTracing = dynamicFeatureFlags.enableDebugTracing,
enableFastJSX = dynamicFeatureFlags.enableFastJSX,
enableRenderableContext = dynamicFeatureFlags.enableRenderableContext,
enableTransitionTracing = dynamicFeatureFlags.enableTransitionTracing;
dynamicFeatureFlags = dynamicFeatureFlags.renameElementSymbol;
Expand Down Expand Up @@ -863,9 +860,7 @@ __DEV__ &&
specialPropKeyWarningShown;
var didWarnAboutStringRefs = {};
var didWarnAboutElementRef = {};
var enableFastJSXWithStringRefs = enableFastJSX && !0,
enableFastJSXWithoutStringRefs = enableFastJSXWithStringRefs && !1,
didWarnAboutKeySpread = {},
var didWarnAboutKeySpread = {},
ownerHasKeyUseWarning = {};
exports.Fragment = REACT_FRAGMENT_TYPE;
exports.jsxDEV = function (
Expand Down
2 changes: 1 addition & 1 deletion compiled/facebook-www/REVISION
Original file line number Diff line number Diff line change
@@ -1 +1 @@
0117239720b6ea830376f4f8605957ccae8b3735
4ea12a11d1848c1398f9a8babcfbcd51e150f1d9
2 changes: 1 addition & 1 deletion compiled/facebook-www/REVISION_TRANSFORMS
Original file line number Diff line number Diff line change
@@ -1 +1 @@
0117239720b6ea830376f4f8605957ccae8b3735
4ea12a11d1848c1398f9a8babcfbcd51e150f1d9
79 changes: 41 additions & 38 deletions compiled/facebook-www/React-dev.classic.js
Original file line number Diff line number Diff line change
Expand Up @@ -272,8 +272,8 @@ __DEV__ &&
-1 < x.stack.indexOf("\n at")
? " (<anonymous>)"
: -1 < x.stack.indexOf("@")
? "@unknown:0:0"
: "";
? "@unknown:0:0"
: "";
}
return "\n" + prefix + name + suffix;
}
Expand Down Expand Up @@ -660,13 +660,15 @@ __DEV__ &&
null === type
? (isStaticChildren = "null")
: isArrayImpl(type)
? (isStaticChildren = "array")
: void 0 !== type && type.$$typeof === REACT_ELEMENT_TYPE
? ((isStaticChildren =
"<" + (getComponentNameFromType(type.type) || "Unknown") + " />"),
(children =
" Did you accidentally export a JSX literal instead of a component?"))
: (isStaticChildren = typeof type);
? (isStaticChildren = "array")
: void 0 !== type && type.$$typeof === REACT_ELEMENT_TYPE
? ((isStaticChildren =
"<" +
(getComponentNameFromType(type.type) || "Unknown") +
" />"),
(children =
" Did you accidentally export a JSX literal instead of a component?"))
: (isStaticChildren = typeof type);
error$jscomp$0(
"React.jsx: type is invalid -- expected a string (for built-in components) or a class/function (for composite components) but got: %s.%s",
isStaticChildren,
Expand Down Expand Up @@ -700,11 +702,7 @@ __DEV__ &&
hasValidKey(config) &&
(checkKeyStringCoercion(config.key), (children = "" + config.key));
hasValidRef(config) && warnIfStringRefCannotBeAutoConverted(config, self);
if (
(!enableFastJSXWithoutStringRefs &&
(!enableFastJSXWithStringRefs || "ref" in config)) ||
"key" in config
) {
if ("ref" in config || "key" in config) {
maybeKey = {};
for (var propName in config)
"key" !== propName &&
Expand Down Expand Up @@ -1202,7 +1200,6 @@ __DEV__ &&
disableDefaultPropsExceptForClasses =
dynamicFeatureFlags.disableDefaultPropsExceptForClasses,
enableDebugTracing = dynamicFeatureFlags.enableDebugTracing,
enableFastJSX = dynamicFeatureFlags.enableFastJSX,
enableRenderableContext = dynamicFeatureFlags.enableRenderableContext,
enableTransitionTracing = dynamicFeatureFlags.enableTransitionTracing,
renameElementSymbol = dynamicFeatureFlags.renameElementSymbol,
Expand Down Expand Up @@ -1319,9 +1316,7 @@ __DEV__ &&
didWarnAboutOldJSXRuntime;
var didWarnAboutStringRefs = {};
var didWarnAboutElementRef = {};
var enableFastJSXWithStringRefs = enableFastJSX && !0,
enableFastJSXWithoutStringRefs = enableFastJSXWithStringRefs && !1,
didWarnAboutKeySpread = {},
var didWarnAboutKeySpread = {},
ownerHasKeyUseWarning = {},
didWarnAboutMaps = !1,
userProvidedKeyEscapeRegex = /\/+/g,
Expand Down Expand Up @@ -1688,13 +1683,13 @@ __DEV__ &&
isArrayImpl(type)
? (typeString = "array")
: void 0 !== type && type.$$typeof === REACT_ELEMENT_TYPE
? ((typeString =
"<" +
(getComponentNameFromType(type.type) || "Unknown") +
" />"),
(i =
" Did you accidentally export a JSX literal instead of a component?"))
: (typeString = typeof type);
? ((typeString =
"<" +
(getComponentNameFromType(type.type) || "Unknown") +
" />"),
(i =
" Did you accidentally export a JSX literal instead of a component?"))
: (typeString = typeof type);
error$jscomp$0(
"React.createElement: type is invalid -- expected a string (for built-in components) or a class/function (for composite components) but got: %s.%s",
typeString,
Expand Down Expand Up @@ -1769,18 +1764,18 @@ __DEV__ &&
"forwardRef requires a render function but received a `memo` component. Instead of forwardRef(memo(...)), use memo(forwardRef(...))."
)
: "function" !== typeof render
? error$jscomp$0(
"forwardRef requires a render function but was given %s.",
null === render ? "null" : typeof render
)
: 0 !== render.length &&
2 !== render.length &&
error$jscomp$0(
"forwardRef render functions accept exactly two parameters: props and ref. %s",
1 === render.length
? "Did you forget to use the ref parameter?"
: "Any additional parameter will be undefined."
);
? error$jscomp$0(
"forwardRef requires a render function but was given %s.",
null === render ? "null" : typeof render
)
: 0 !== render.length &&
2 !== render.length &&
error$jscomp$0(
"forwardRef render functions accept exactly two parameters: props and ref. %s",
1 === render.length
? "Did you forget to use the ref parameter?"
: "Any additional parameter will be undefined."
);
null != render &&
null != render.defaultProps &&
error$jscomp$0(
Expand Down Expand Up @@ -1926,6 +1921,14 @@ __DEV__ &&
exports.unstable_useCacheRefresh = function () {
return resolveDispatcher().useCacheRefresh();
};
exports.unstable_useContextWithBailout = function (context, select) {
var dispatcher = resolveDispatcher();
context.$$typeof === REACT_CONSUMER_TYPE &&
error$jscomp$0(
"Calling useContext(Context.Consumer) is not supported and will cause bugs. Did you mean to call useContext(Context) instead?"
);
return dispatcher.unstable_useContextWithBailout(context, select);
};
exports.unstable_useMemoCache = useMemoCache;
exports.use = function (usable) {
return resolveDispatcher().use(usable);
Expand Down Expand Up @@ -1998,7 +2001,7 @@ __DEV__ &&
exports.useTransition = function () {
return resolveDispatcher().useTransition();
};
exports.version = "19.0.0-www-classic-01172397-20240716";
exports.version = "19.0.0-www-classic-4ea12a11-20240801";
"undefined" !== typeof __REACT_DEVTOOLS_GLOBAL_HOOK__ &&
"function" ===
typeof __REACT_DEVTOOLS_GLOBAL_HOOK__.registerInternalModuleStop &&
Expand Down
Loading

0 comments on commit 1180f1b

Please sign in to comment.