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

Add support for hooks inspection via devtools #2480

Merged
merged 5 commits into from Apr 20, 2020
Merged

Conversation

marvinhagemeister
Copy link
Member

@marvinhagemeister marvinhagemeister commented Apr 19, 2020

This PR makes it possible to inspect a components hook state which is needed for the devtools (preactjs/preact-devtools#143).

The way the devtools work is that they inspect a node lazily when the user selects it in the elements panel and not during rendering. So we need to guess which piece of state belongs to which hook at a later point. To do that I've introduced numbers as unique ids for each hook function.

Some of our hooks leverage other hooks to keep our size small and this PR will detect that and will pick the topmost "native" hook. So if useState calls useReducer internally, the hook will be identified as useState no matter what.

Originally I played around with storing that information on the _hooks property of a component, but that turns out to be a lot bigger than just passing it as an additional argument to options._hook. Besides the type, the callsite index of the current hooks is very relevant for the devtools to display them in the correct order.

To retrieve any custom hooks we need to do some awkward re-rendering of the inspected component. Custom hooks are just plain JavaScript function and we can be only made aware of those by inspecting the stack trace via the Error object.

But simply re-rendering poses another issue in that we need to make sure that it doesn't change the state of the current app in any way. We assume that the render function is pure and leverage a new options._skipEffects flag to bypass all scheduled effects.

For a longer description of the approach used in the devtools check this comment out: preactjs/preact-devtools#52 (comment)

Screenshot from 2020-04-19 09-33-13

@github-actions
Copy link

@github-actions github-actions bot commented Apr 19, 2020

Size Change: +151 B (0%)

Total Size: 39.3 kB

Filename Size Change
debug/dist/debug.js 3 kB +2 B (0%)
debug/dist/debug.umd.js 3.08 kB +1 B
hooks/dist/hooks.js 1.1 kB +50 B (4%)
hooks/dist/hooks.module.js 1.12 kB +43 B (3%)
hooks/dist/hooks.umd.js 1.18 kB +55 B (4%)
ℹ️ View Unchanged
Filename Size Change
compat/dist/compat.js 3.12 kB 0 B
compat/dist/compat.module.js 3.14 kB 0 B
compat/dist/compat.umd.js 3.17 kB 0 B
debug/dist/debug.module.js 2.98 kB 0 B
devtools/dist/devtools.js 185 B 0 B
devtools/dist/devtools.module.js 194 B 0 B
devtools/dist/devtools.umd.js 260 B 0 B
dist/preact.js 3.83 kB 0 B
dist/preact.min.js 3.83 kB 0 B
dist/preact.module.js 3.84 kB 0 B
dist/preact.umd.js 3.88 kB 0 B
test-utils/dist/testUtils.js 437 B 0 B
test-utils/dist/testUtils.module.js 439 B 0 B
test-utils/dist/testUtils.umd.js 515 B 0 B

compressed-size-action

@marvinhagemeister
Copy link
Member Author

@marvinhagemeister marvinhagemeister commented Apr 19, 2020

I seem to run into a few wrongly mangled properties. Need to investigate...

Never mind, I accidentally mutated something in the devtools 🙈

Copy link
Member

@JoviDeCroock JoviDeCroock left a comment

Awesome, I've been following the devtools part and this is amazing! Hats off

@@ -6,6 +6,9 @@ let currentIndex;
/** @type {import('./internal').Component} */
let currentComponent;

/** @type {number} */
let currentHook = 0;
Copy link
Member

@developit developit Apr 19, 2020

Choose a reason for hiding this comment

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

I'm wondering about the index association here: would it be feasible to instead set+pass a reference to the current hook function?

Preact/debug could then import * from hooks and do a reverse lookup to get the hook name:

import { * as hooks } from 'preact/hooks';

options._hook = (comp, index, type) = {
  let hookName;
  for (let i in hooks) if (hooks[i]===type) {
    hookName = i;
    break;
  }
  // Etc
};

Copy link
Member Author

@marvinhagemeister marvinhagemeister Apr 20, 2020

Choose a reason for hiding this comment

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

In the current architecture the whole options handling is injected by the devtools. Previously we did this in preact/debug, but it made us less flexible regarding bug fixes. Whenever there is a bug in the devtools bridge we had to tell users to upgrade their Preact version which feels wrong to me.

Because it's all handled by the extension in a central place we can easily roll out fixes for all supported Preact versions.

(For everyone reading this: We discussed various alternatives in Slack but came to the conclusion that the approach in this PR is the best one so far. We'll move forward with this)

@marvinhagemeister marvinhagemeister merged commit f272a86 into master Apr 20, 2020
4 of 5 checks passed
@marvinhagemeister marvinhagemeister deleted the hook-log branch Apr 20, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants