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

[WIP]: Injecting parse function from extension to devtools #91

Closed
wants to merge 7 commits into from

Conversation

vibhorgupta-gh
Copy link
Collaborator

Requesting review for this commit.

How it's intended to work:

injectHookVariableNamesFunction returns a promise that resolves to the new hook object after 2 seconds (to mock for the potential delay in parsing) which gets passed down via context for rendering. As per the last discussion, the following happens:

  1. A new resource type hookResource is declared (with the default fetch argument for createResource simply returning a promise that resolves to the initial hook object of the original inspectedElement object
  2. If prop injectHookVariableNamesFunction is present, this function is called and the promise for new hooks obtained is written to the new hook resource
  3. Inside the function getInspectedElement we read the hook resource to extract the new hooks objects and set it in the inspectedElement object

How it's working:

We aren't able to handle promises properly along with the suspense and resource approach. Observations from looking in cache.js:

  • For default cases (cases where we don't write anything to a resource) within accessResult, a .then() is called on a thenable and a pending status with the value as the thenable is returned
  • For cases where we have already called resource.write(key, val) and object with resolved state and value as val is set and accessResult simply returns this object

In this approach, we're writing the promise of the new hook object as the value with the inspectedElement as the key in the hookResource and while reading from this resource, fulfilled promise object (containing the desired new hook object) is returned, instead of the actual object. However, if we don't write anything to the hookResource and trigger the default case (which returns the original hooks for now), things work as expected and hook array is returned on reading the resource.
Screenshots of the inspectedElement for both these cases follow:

  1. We write the promise of parsed names returned by injectHookVariableNamesFunction and then try to read It from the resource. This results in the hook key being a promise object.

Screenshot 2020-11-10 at 2 21 48 AM

  1. We don't write the newly obtained promise which forces the default case while reading from hookResource, and it perfectly returns an array of hooks, set in inspectedElement

Screenshot 2020-11-10 at 2 19 25 AM

Note: The approach in the concerned commit works if we don't return a promise from injectHookVariableNamesFunction, instead just return a dummy hook object (for experimentation purposes) and write that to hookResource.

Maybe we're missing something really nuanced or something very basic? It feels like we have some degree of understanding of how suspense and createResource work, but there still are gaps in our knowledge and need nudging in the right direction

@vibhorgupta-gh vibhorgupta-gh changed the title Injecting parse function from extension to devtools [WIP]: Injecting parse function from extension to devtools Nov 10, 2020
@bvaughn
Copy link
Collaborator

bvaughn commented Nov 10, 2020

First thing I noticed when trying this branch out is that it throws an error whenever I selected a component (even one that has hooks):

Uncaught Error: hooks.map is not a function

Call stack:

    at InnerHooksTreeView (webpack-internal:///../react-devtools-shared/src/devtools/views/Components/InspectedElementHooksTree.js:126:16)
    at renderWithHooks (webpack-internal:///../../build/node_modules/react-dom/cjs/react-dom.development.js:2545:157)
    at mountIndeterminateComponent (webpack-internal:///../../build/node_modules/react-dom/cjs/react-dom.development.js:2883:872)
    at beginWork (webpack-internal:///../../build/node_modules/react-dom/cjs/react-dom.development.js:3200:93)
    at HTMLUnknownElement.callCallback (webpack-internal:///../../build/node_modules/react-dom/cjs/react-dom.development.js:657:119)
    at Object.invokeGuardedCallbackDev (webpack-internal:///../../build/node_modules/react-dom/cjs/react-dom.development.js:677:45)
    at invokeGuardedCallback (webpack-internal:///../../build/node_modules/react-dom/cjs/react-dom.development.js:696:126)
    at beginWork$1 (webpack-internal:///../../build/node_modules/react-dom/cjs/react-dom.development.js:4139:1)
    at performUnitOfWork (webpack-internal:///../../build/node_modules/react-dom/cjs/react-dom.development.js:3934:270)
    at workLoopSync (webpack-internal:///../../build/node_modules/react-dom/cjs/react-dom.development.js:3924:30)

Component stack:

    at InnerHooksTreeView (webpack-internal:///../react-devtools-shared/src/devtools/views/Components/InspectedElementHooksTree.js:119:3)
    at div
    at InspectedElementHooksTree (webpack-internal:///../react-devtools-shared/src/devtools/views/Components/InspectedElementHooksTree.js:55:3)
    at div
    at InspectedElementView (webpack-internal:///../react-devtools-shared/src/devtools/views/Components/InspectedElementView.js:56:3)
    at div
    at InspectedElementWrapper (webpack-internal:///../react-devtools-shared/src/devtools/views/Components/InspectedElement.js:43:79)
    at Suspense
    at NativeStyleContextController (webpack-internal:///../react-devtools-shared/src/devtools/views/Components/NativeStyleEditor/context.js:63:3)
    at div
    at div
    at InspectedElementContextController (webpack-internal:///../react-devtools-shared/src/devtools/views/Components/InspectedElementContext.js:83:3)
    at OwnersListContextController (webpack-internal:///../react-devtools-shared/src/devtools/views/Components/OwnersListContext.js:68:3)
    at SettingsModalContextController (webpack-internal:///../react-devtools-shared/src/devtools/views/Settings/SettingsModalContext.js:34:3)
    at Components (webpack-internal:///../react-devtools-shared/src/devtools/views/Components/Components.js:59:81)
    at ErrorBoundary (webpack-internal:///../react-devtools-shared/src/devtools/views/ErrorBoundary.js:38:5)
    at PortaledContent (webpack-internal:///../react-devtools-shared/src/devtools/views/portaledContent.js:34:32)
    at div
    at div
    at ProfilerContextController (webpack-internal:///../react-devtools-shared/src/devtools/views/Profiler/ProfilerContext.js:43:3)
    at TreeContextController (webpack-internal:///../react-devtools-shared/src/devtools/views/Components/TreeContext.js:590:3)
    at SettingsContextController (webpack-internal:///../react-devtools-shared/src/devtools/views/Settings/SettingsContext.js:42:3)
    at ModalDialogContextController (webpack-internal:///../react-devtools-shared/src/devtools/views/ModalDialog.js:65:3)
    at DevTools (webpack-internal:///../react-devtools-shared/src/devtools/views/DevTools.js:87:3)

@bvaughn
Copy link
Collaborator

bvaughn commented Nov 10, 2020

I pushed an update to this branch that wires up the Suspense cache and resource stuff. Loading source maps and parsing AST is still a TODO, but this wires things up so that the dummy data flows all the way to the UI:
Screen Shot 2020-11-10 at 10 15 57 AM

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