-
-
Notifications
You must be signed in to change notification settings - Fork 4.5k
feat: allow $inspect reactivity map, set, date #11164
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
feat: allow $inspect reactivity map, set, date #11164
Conversation
🦋 Changeset detectedLatest commit: 9d68231 The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
I like the idea! I think we can expand it to autogenerate this symbol field in dev mode onto classes using runes. For example if you write class Counter {
count = $state(0)
double = $derived(this.count * 2)
} then in dev mode the compiler will autogenerate the symbol for that class and return the count signal from it. I'm not fully sure yet what's the best API for the inspect symbol. Should it invoke the signals? Should it return an array of signals the inspect function can then traverse? |
fcdfd35
to
07b32dc
Compare
Could you go one step further, and instead of making I've been thinking about how to redefine the Then when you need the raw Set (or whatever), you could call unstate on it, which could call an I think this could also be used to enable |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just need to fix the linting issue :)
I think this can be done in a follow up, as it might widely implications for what we do with ownership too. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'd like to discuss what the symbol contains exactly and whether or not it makes sense to auto generate that first (see my comment above)
I think this can be done in a follow up, as it might widely implications for what we do with ownership too.
Then let's discuss that first and think about it holistically. We can always incrementally work towards that once we figured out a path, but I'd like to first see if we're going in the right direction
@dummdidumm What I mean is that there have been many complaints about |
5eb625b
to
d38f89e
Compare
ok to proceed, we can always change this later, after a bit of discussion we concluded it doesn't affect other stuff directly right now
@tanhauhau You have some linting issues. |
@trueadm thanks. i've fixed them |
This reverts commit 43d13e9.
@Rich-Harris This was working fine when I reviewed it, I believe it's likely because of the other PR that ensures we don't duplicate data landed (#11200). I wonder what API is used for introspection of Maps for dev tools |
Relates to #11151.
Currently the
$inspect
runes work based ondeep_read
the state through properties and getters. This seems to work withReactiveSet
andReactiveMap
because it has aget size()
getter.And it doesn't work with the
ReactiveDate
because it has neither state properties nor getters.This PR attempts to fix this by introduce a
Symbol('$inspect')
method to the reactivity objects. This allow the reactivity objects to implements its own logic to trigger the inspect callbacks.We could export the
Symbol('$inspect')
to allow it to work for user defined reactivity object.Before submitting the PR, please make sure you do the following
feat:
,fix:
,chore:
, ordocs:
.Tests and linting
pnpm test
and lint the project withpnpm lint