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 _$afterCreateRoot hook to handle new roots in dev #1067

Merged
merged 2 commits into from
Jun 26, 2022

Conversation

thetarnav
Copy link
Contributor

Summary

Solid's internal reactivity graph has this problem where roots created with createRoot don't have to be added to its detachedOwner object — it's enough that they are able to get to the context walking upwards.
But not for devtools, all the createRoot calls are effectively invisible for a debugger that wants to walk downwards to analyze the tree. And roots are very common — components such as <For>, <Index> and <Suspense>; primitives that want to control disposal like createLazyMemo; and even render function that is creating the main branch of the reactive tree.

Having a special hook that would fire with every newly created root would let us handle all of those cases.

Then it could be used easily for tracking:

import { registerRoot } from "solid-devtools";
window._$afterCreateRoot = (root) => {
   registerRoot(root);
}

Related discussion: #860
And devtools issue: thetarnav/solid-devtools#15

How did you test this change?

Added a new test case to dev.spec.ts

@coveralls
Copy link

Pull Request Test Coverage Report for Build 2534785309

  • 3 of 3 (100.0%) changed or added relevant lines in 1 file are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage decreased (-0.02%) to 88.99%

Totals Coverage Status
Change from base Build 2482635498: -0.02%
Covered Lines: 1254
Relevant Lines: 1337

💛 - Coveralls

@ryansolid ryansolid merged commit 06873b0 into solidjs:main Jun 26, 2022
@ryansolid
Copy link
Member

Thanks.

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