Named hooks
bvaughn opened this issue · 5 comments
This issue has been relocated to: facebook/react#16474
The problem
One common piece of feedback about DevTools hooks integration is that hooks have no name and can be confusing. Consider the following example:
function useSomeCustomHook() {
const [foo, setFoo] = useState(true);
const [bar, setBar] = useState(false);
// ...
}
function Example() {
const baz = useSomeCustomHook();
// ...
}
Currently in DevTools the above component would be displayed as follows:
SomeCustomHook:
State: true
State: false
This information isn't as rich as we would prefer.
The next question is often: "can you use the name of the variable the hook return value is assigned to?" but this is tricky because DevTools doesn't actually have any way to access that variable. (Even if DevTools has a handle on the Example
function above, how would it access the useSomeCustomHook
function?)
The proposal
The solution to this would be some form of user-defined metadata (preferably generated by a code transform). Building on the precedent of the useDebugValue
hook (facebook/react#14559), we might introduce a new no-op hook e.g. useDebugName
.
The above example could make use of this hook like so:
function useSomeCustomHook() {
const [foo, setFoo] = useState(true);
useDebugName("foo"); // injected by Babel transform
const [bar, setBar] = useState(false);
useDebugName("bar"); // injected by Babel transform
// ...
}
function Example() {
const baz = useSomeCustomHook();
// ...
}
DevTools could then display something like:
SomeCustomHook:
State (foo): true
State (bar): true
Implementation details
The new useDebugName
hook might be a noop hook provided by React (similar to useDebugValue
) or it could even be an export from the (soon to be released react-debug-hooks
package). The key concerns would be that:
- It has no effect (and adds no overhead) when DevTools is not present.
- Not calling it at all (or only calling it for some hooks) should not break or corrupt anything.
DevTools could override the no-op useDebugName
implementation before inspecting a component and automatically associate the provided name with the most recently called native hook.
For example, the following code should only result in one named hook (the second useState
call).
const [foo, setFoo] = useState(true);
const [bar, setBar] = useState(false);
useDebugName("bar"); // injected by Babel transform
const [baz, setBaz] = useState(true);
Being able to support sparse name metadata would be important for third party code (that might not be transformed to supply the metadata).
A code transform would be ideal for this scenario because manual annotation would probably be cumbersome. This could also be marketed as a DEV-only transform so as not to bloat production bundles with display names. We might even try to detect the env and throw if it isn't DEV (like facebook/react#15939).
Further considerations
Custom hooks?
In some cases, custom hooks might also be ambiguous. Consider the useSubscription
hook (facebook/react#15022):
function Example() {
const foo = useSubscription(...);
const bar = useSubscription(...);
// ...
}
Currently in DevTools the above component would be displayed as follows:
Subscription: "some value"
State: Object
Subscription: "some other value"
State: Object
Maybe the value alone (provided by useDebugValue
) could be enough to uniquely identify the hook, but I suspect in many cases it might not be sufficient. Should we then use useDebugName
for custom hooks as well?
I think it would be more fragile given the way our custom hooks detection logic is implemented. Custom hooks are not identified until after a component has finished rendering. In order for us to associate names with custom hooks, we would need to maintain a stack of names. This could lead to potential mismatches though in the event that useDebugName
was called more (or fewer) times than there are custom hooks.
For example, consider the following code:
function useSomeCustomHook() {
const [foo, setFoo] = useState(true);
useDebugName("foo");
useDebugName("effectively ignored");
const [bar, setBar] = useState(false);
const [baz, setBaz] = useState(false);
useDebugName("baz");
// ...
}
The proposed implementation of useDebugName
would be robust enough to handle naming "foo" and "baz" states and leaving "bar" as anonymous state hook. If we were maintaining a stack of names however, this discrepency would be more difficult to manage.
Perhaps there is a clever solution to this problem. I would probably suggest leaving it out of the initial implementation though and only revisiting if we determine it's a necessary feature.
Alternatives considered
Pass debug name as an additional (unused) parameter
An alternative approach to calling a separate hook for naming purposes would be to pass the display name as an additional parameter to the native hook, e.g.:
function useSomeCustomHook() {
const [foo, setFoo] = useState(true, "foo");
const [bar, setBar] = useState(false, "bar");
// ...
}
function Example() {
const baz = useSomeCustomHook();
// ...
}
Pros:
- Less code.
- Does not introduce a new hook.
Cons:
- It requires knowledge about the arity of native hooks. Ror example
useReducer
has optional parameters that the transform (or manual code) would need to be aware of to avoid a runtime error. - It would not be possible to support naming custom hooks (if that's something we decided to do).
Load source code (with source maps) and parse for name
We could use an extension API like Resource.getContent
to load the source code (including custom hooks) and parse it determine the hook/variable names. Essentially this would work like the proposed transform above, but at runtime.
Pros:
- Does not require a Babel transform step. ("Just works")
- Does not potentially bloat production builds (if transform is used incorrectly).
Cons:
- Adds additional async loading (complexity) to suspense cache used for hooks inspection.
- May have difficulty parsing certain code patterns (e.g. Babel's destructuring transform) unless we embed a full parser.
Call toString
on the function component and parse for name
A possible 80/20 variant of the above proposal would be to simply call toString
on the function component and parse any top-level hooks.
Pros:
- Does not require a Babel transform step. ("Just works")
- Does not potentially bloat production builds (if transform is used incorrectly).
- Does not require any additional asynchronous code.
Cons:
- Only supports top-level hooks (used directly within the function).
- May have difficulty parsing certain code patterns (e.g. Babel's destructuring transform) unless we embed a fullp parser.
Use a Babel transform to leave an inline comment (and call toString
to search for it)
Rather than inserting a call to a new custom hook, our code transform could just insert an inline comment with the name. We could then parse the code to find the inline comment, e.g.:
function Example() {
/* hook:foo:Example.react.js:3 */
const foo = useSubscription(...);
/* hook:bar:Example.react.js:5 */
const bar = useSubscription(...);
// ...
}
Pros:
- Does not potentially bloat production builds (if transform is used incorrectly).
- Potentially sidesteps difficulty of parsing certain code patterns (e.g. Babel's destructuring transform).
Cons:
- Only supports top-level hooks (used directly within the function).
- Still requires an explicit transform step.
One thing that useDebugName
could do is, create an exception and record the stack depth. renderWithHooks
itself would also record its own stack depth. If the stack depth is recorded along with the name argument, it should be possible to tell whether the call came from a custom hook or from the component.
The problem is that renderWithHooks
would have to unconditionally measure its own depth before rendering the component, whether or not the component has hooks, which is a performance cost. The alternative is to only measure it if the component called useDebugName
at least once, but at that point a second pass over the hook list would be needed to bind the names to the correct depth.
This could potentially break with strange setups like the regenerator
transform, but I don't think any component or custom hook is allowed to be async
or a generator function.
This is really well thought-out! I think my issue with the useDebugName
hook, even though it follows precedent, is that it seems you -always- want to have this hook defined for future devs who need to debug the code. If this is the case, making this a separate hook that you need to learn how to combine with your normal hooks seems more error prone.
I believe adding an -arity argument to the original react provided hooks to name them, and then a useDebugName
hook for custom hooks (because at this point if you are making custom hooks we can assume you have the requisite knowledge to incorporate this correctly), would be the best way forward.
Then docs can reference:
const [count, setCount] = React.useState(0, { name: 'count' });
and people will catch on that the name is for debugging purposes in the DevTools.
- I think it might be more future-proof to name it
useDebugInfo()
and take the hook function name (which is always required) as one of possible parameters and the assignment as another, for example, you might add source address (line, column) to jump to in the code editor. - Have you considered calling this debug before the hook that's being named, not after?
For custom hooks it will naturally build a stack:
function Component() {
useDebugInfo({fn:"useSubscription",lhs:"const {something}"});
const {something} = useSubscription();
}
After some more thought, if we did have a useDebugName
hook, someone could just release a package with wrappers around useState
and other built-in react hooks that include useDebugName
and useDebugValue
. I changed my mind and vote on the custom hook.
This repository is being merged into the main React repo (github.com/facebook/react). As part of this, I am moving all issues to that repository as well and closing them here.
This issue has been relocated to:
facebook/react#16474