easy-peasy icon indicating copy to clipboard operation
easy-peasy copied to clipboard

Problems with computed properties not refreshing

Open Jussinevavuori opened this issue 5 years ago • 5 comments

Hello. I am currently working first time with this library and am loving it. That said, I have ran into a problem with the library.

I have a React application, and as a simplified version, I am showing a list of items, which can be filtered by date. The items are in their own model, while the date interval filter is lifted to its own model. My store looks roughly something like this.

// interval.model.ts
// ...
const intervalModel: IntervalModel = {
  startDate: new Date(/* ... */),
  endDate: new Date(/* ... */),

  // ... actions to set start and end dates
  // ... computed properties depending on start and end dates
}
// items.model.ts
// ...
const itemsModel: ItemsModel = {
  items: [],
  filteredItems: computed([
    state => state.items,
    (state, storeState) => storeState.interval.startDate,
    (state, storeState) => storeState.interval.endDate
  ],
  (items, startDate, endDate) => return items.filter(item => {
    return isBetween(item.date, startDate, endDate)
  })
}
// store.ts

export const store = createStore({
  items: itemsModel,
  interval: intervalModel,
})

For some reason the filteredItems property is not updating when startDate and endDate are updated in intervalModel, however other computed properties depending within intervalModel depending on those properties are updating.

I did manage to create a workaround and that was by using nested models and nesting the interval model within the transactions model. I however think this solution is not as elegant as having everything on the top level. Especially when the time interval will be an important part of my application and should be on the top level.

My solution roughly (interval.model.ts file is unchanged)

// items.model.ts
// ...
const itemsModel: ItemsModel = {
  items: [],
  interval: intervalModel,
  filteredItems: computed(
  (state) => return state.items.filter(item => {
    return isBetween(item.date, state.interval.startDate, state.interval.endDate)
  })
}
// store.ts

export const store = createStore({
  items: ItemsModel
})

Jussinevavuori avatar Sep 01 '20 14:09 Jussinevavuori

Hi @Jussinevavuori 👋

Happy to hear you are enjoying the library, but sorry that you are having some difficulties.

This may be a bug. It would be really helpful to me if you could create a small reproducible example within CodeSandbox to allow me to debug this for you.

Thanks!

ctrlplusb avatar Sep 04 '20 07:09 ctrlplusb

After long trying to reproduce the bug, I managed to find the real reason (spoilers: it was my code, not yours).

Instead of using the filteredItems property directly, I had another computed property depending on filteredItems, which is already a computed property in itself. Let's use filteredCount as an example here - filteredCount looked like this.

// items.model.ts
// ...
const itemsModel: ItemsModel = {
  items: [],
  filteredItems: computed([
    state => state.items,
    (state, storeState) => storeState.interval.startDate,
    (state, storeState) => storeState.interval.endDate
  ],
  (items, startDate, endDate) => return items.filter(item => {
    return isBetween(item.date, startDate, endDate)
  }),
  filteredCount: computed(state => state.filteredItems.length),
}

Having filteredCount be defined like this causing it to not update when storeState.interval.startDate was updated, causing filteredItems to update. My intuition was that the updates would 'cascade', if you will, and cause filteredCount to also update when storeState.interval.startDate updates. It seems that was not the case.

Updating filteredCount to the following fixed the problem. Here I manually tell both filteredItems and filteredCount to update when storeState.interval.startDate updates. (Now you also have to provide the third type argument, StoreModel to the definition of filteredCount inside the ItemsModel interface definition.)

// items.model.ts
// ...
const itemsModel: ItemsModel = {
  items: [],
  filteredItems: computed([
    state => state.items,
    (state, storeState) => storeState.interval.startDate,
    (state, storeState) => storeState.interval.endDate
  ],
  (items, startDate, endDate) => return items.filter(item => {
    return isBetween(item.date, startDate, endDate)
  }),
  filteredCount: computed([
    state => state.filteredItems,
    (state, storeState) => storeState.interval.startDate,
    (state, storeState) => storeState.interval.endDate
  ],state => state.filteredItems.length),
}

Here is a CodeSandbox where the bug is visible in src/itemsModel.ts. It seems the bug was a different bug that I initially described in the issue, and this might not even be a bug, maybe just confusing behaviour.

Thank you for your quick reply.

Jussinevavuori avatar Sep 05 '20 11:09 Jussinevavuori

Hi @Jussinevavuori,

So I updated your example to resolve the specific computed property and it appears to work as expected now:

https://codesandbox.io/s/young-sun-qxz0q?file=/src/itemsModel.ts

I need to dig into this a bit more. I may need to describe this in the docs, stating that if you depend on computed property from another that it is best resolve it directly. Thinking off the top of my head it is likely that the immutability trigger that causes the update won't be triggered without doing this because of the nature of computed properties.

ctrlplusb avatar Sep 25 '20 14:09 ctrlplusb

Great! Thank you.

Jussinevavuori avatar Sep 25 '20 15:09 Jussinevavuori

@ctrlplusb I had an issue with persistence and a computed similar to this initial topic in the v4.

had a very simple computed that looked like

valid: computed(state => state.userId !== null),

Before the render logic of my app I have an await for the persist.

await store.persist.resolveRehydration();

but it was never updating the computed state after the hydration. I added some logging to valid and basically saw it do.

null // value of userId in computed
null // value of userId in computed
store persisted // log after the await
false // value of valid state, called in the app render

After reading this I switch

valid: computed(
    [
      state => state.userId
    ],
    userId => userId !== null
  ),

and the problem resolved itself. When I get a chance i'll see if I can get a reproduction in a sandbox for you.

jchamb avatar Oct 28 '20 15:10 jchamb

I believe this will be resolved by the most recent updates to computed.

ctrlplusb avatar Sep 16 '22 06:09 ctrlplusb