-
Notifications
You must be signed in to change notification settings - Fork 34
FIX: :for loop reactivity
#541
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
base: dev
Are you sure you want to change the base?
FIX: :for loop reactivity
#541
Conversation
Test Results: ✅ PASSEDRun at: 2025-11-20T16:28:19.296Z Summary: |
|
@HadiAlMarzooq awesome, this looks good! Thanks for contributing this. I'm wondering why this change would ensure the tracking isn't being missed though 🤔 - maybe it happens with certain transpiled code? just for my own sanity, do you have a clear reproduction scenario of when the issue happens? |
…rlying properties
|
Thanks @michielvandergeest , I'll try to give a clear reproduction scenario and what we discovered during debugging: Reproduction ScenarioThe issue occurs when a component has a prop that starts as an empty array In our app, we have a // Row.js
export default Blits.Component('Row', {
props: ['cards'],
computed: {
processedCards() {
return this.cards.map(card => {
// ... process card data
return processedCard
})
}
},
template: `
<ContentCard
:for="(card, index) in $processedCards"
:key="$card.id"
/>
`
})The Problem: Initially What We Found During DebuggingAfter deep debugging, I discovered the root cause is in how the effect tracks dependencies when using computed properties. When the effect was registered with a specific key filter like This means the effect was only tracking the computed property name itself, not the underlying reactive properties that the computed depends on. When The FixBy changing the effect registration to use I also added explicit access to underlying props/state for direct properties (not computed) to ensure tracking is established even when arrays start empty, though the main fix is the Known LimitationDuring testing, I discovered a limitation: Computed properties that depend on component state (not props) do not work reactively in
Workaround: Use props instead of state for arrays that are used in computed properties within Test ResultsI created a test page (
The fix is minimal and focused - it just changes how the effect tracks dependencies, ensuring computed property dependencies are properly tracked for props without affecting other functionality. |
Fixes #540
The
:forloop directive wasn't reactively tracking array property access when arrays transitioned from empty to populated. The effect generator passedcomponent.itemsdirectly as a function argument (forloops[1](component.items, ...)), causing the reactive proxy'sgethandler to potentially miss the dependency tracking during argument evaluation. By storing the property in a local variable (const collection = component.items) before passing it to the forloop function, we ensure the property access occurs explicitly during effect execution with the correctcurrentEffectandcurrentKeycontext, guaranteeingtrack()is invoked and the dependency is registered. This addresses the same issue mentioned in v0.9.4 CHANGELOG, which suggests the previous fix didn't fully resolve the tracking mechanism. All tests updated and passing (920 tests).