-
-
Notifications
You must be signed in to change notification settings - Fork 4.5k
{#key} block #5397
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
{#key} block #5397
Conversation
b8c041a
to
fa94a5f
Compare
fa94a5f
to
a0f0e8a
Compare
If you're keying on an atomic immutable value, and you update that value to be the same as it already is, does that trigger the contents to be re-created? I would hope not. Is this already handled elsewhere in the code, in that the keying value will never even be marked as dirty? Do you think this PR would benefit from a test for this situation? What initially caught my eye, though, when I was looking at this PR was the |
Good point. I wonder what do you think about passing expression such as {#key obj.foo}
<div>{obj.foo}</div>
{/key}
{#key foo + bar}
<div />
{/key}
{#key foo(bar)}
<div />
{/key} which means we need to evaluate the expression when it's dirty and compare the value to determine should trigger the re-creation. which mean that if writing {#key [foo, bar]}
<div />
{/key} will always recreate if |
It looks like this partially attempts to add support for animation inside {#key foo}
<div animate:bar/>
{/key} still throws the validation error "An element that use the animate directive must be the immediate child of a keyed each block". Do |
i wonder what are the use cases for animation in if the key condition changed, elements are destroyed and recreated, |
I'm not sure, I couldn't think of one off the top of my head, but this PR contains a validation check that returns the message "An element that use the animate directive must be the sole child of a key block" when it fails - so I'm guessing then that this is mostly a copy-paste from some other code? If this wasn't an intended feature of your PR, I'd vote this animation-related stuff gets stripped out entirely for the time being. |
yes. that's copy paste code from |
Does this contain an analogous fix to the one in #5447? Edit: Maybe that question doesn't really make sense... |
Yup. just to be sure, I added another test case that is analogous to #5447, where the variables in the key are not referenced in the template |
Co-authored-by: Conduitry <[email protected]>
Closes #1469
Before submitting the PR, please make sure you do the following
Tests
npm test
and lint the project withnpm run lint