-
-
Notifications
You must be signed in to change notification settings - Fork 4.5k
chore: use $$props
directly where possible
#9813
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
Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
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.
Once code style comment, otherwise good to go
if ( | ||
!state.analysis.accessors && | ||
!(state.analysis.runes ? binding.reassigned : binding.mutated) && | ||
!binding.initial | ||
) { | ||
return b.member(b.id('$$props'), node); | ||
} | ||
|
||
if ( | ||
!(state.analysis.immutable ? binding.reassigned : binding.mutated) && | ||
!binding.initial && | ||
!state.analysis.accessors | ||
) { | ||
return b.call(node); | ||
} |
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.
These are almost identical, which makes it a bit hard to understand what's the difference. Also, these checks are identical to those in other places, which makes them prone to get out of sync. I think it would be better to have a function uses_prop_source
(one for legacy, one for runes mode, because they are slightly different) and then use them in legacy/runes files at the respective locations.
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.
ah, they actually are identical in practice — consolidated them. agree that it would be good to have a uses_prop_source
check eventually, I was just holding off for now because it may make sense to have a separate prop_fallback
helper for props with fallback values but no updates; looking into that currently
Now that
$$props
is always an object, it makes no sense to hide props behind a function call:We still need
prop_source
for reassigned (or mutated, in mutable mode) props or those with initial values, though I expect there's room for tweaks there as wellBefore 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