Migrating from mobx, useEffect dependency from store property

[mogadanez]

Im widely use

  useEffect(()=>{
  ....
  },[localStateProp, someObservablePropFromStore])

but its not working in mobx6, seems now I should rewrite it to use with autorun.
is it any way to work in old way ?

[danielkcz]

Can you elaborate with a fuller example? Hard to say what are you are trying to achieve. Ideally a runnable minimal reproduction.

[mogadanez]

class CategoryStore {
    categoryItems = [];
 constructor (){
        makeAutoObservable(this, {
            getProductsByCategoryId: action
        })
    }
   async getProductsByCategoryId(categoryId){
.....
  runInAction(() => {
                    this.categoryItems.replace(result)
                })
    
   }
}

let component = observer({categoryId, filter}=>{
  const {categoryStore } = useStores()
  const [categories, setCategories] = useState([])
  const { categoryItems } = categoryStore


  useEffect( ()=>{
    if ( categoryId ) {
      (async () => {
        await categoryStore.getProductsByCategoryId(categoryId) 
      })()
    }
  }, [categoryId] )

  useEffect( ()=>{
          if (categoryItems && childCategories ) {
          let result = //....some calculations
          setCategories(result)
          }
  }, [categoryId, categoryItems] )

})

in this example second useEffect fired once on start( but items is not yet loaded), but not fired when items is loaded( categoryItems changed )
It works if I put autorun inside it, but its not clear why I should do it.

[danielkcz]

I really recommend you to read https://mobx.js.org/understanding-reactivity.html thoroughly. This is a classic mindset issue. MobX is mutable which means you always get the same object reference. If you use that reference in useEffect deps, it won't trigger because it's a same object. Consider removing those deps completely and let that effect run on every render, potentially detect changes inside it if you need so.

[mweststrate]

categoryItems is not being used by your render, so `observer` doesn't track it, causing the effect not to be re-evaluated. It can be fixed by slicing categoryItems before using it as dep, which would be correct from react perspective, but I think it is a weird mental model, as it directly couples your effects with rendering, which is why we recommend using autorun instead which decouples the execution of your effect from the fact whether your component re-rendered or not.

…

On Mon, Nov 30, 2020 at 10:01 AM Aleksandr Popov ***@***.***> wrote: class CategoryStore { categoryItems = []; constructor (){ makeAutoObservable(this, { getProductsByCategoryId: action }) } async getProductsByCategoryId(categoryId){ ..... runInAction(() => { this.categoryItems.replace(result) }) } } let component = observer({categoryId, filter}=>{ const {categoryStore } = useStores() const [categories, setCategories] = useState([]) const { categoryItems } = categoryStore useEffect( ()=>{ if ( categoryId ) { (async () => { await categoryStore.getProductsByCategoryId(categoryId) })() } }, [categoryId] ) useEffect( ()=>{ if (categoryItems && childCategories ) { let result = //....some calculations setCategories(result) } }, [categoryId, categoryItems] ) }) in this example second useEffect fired once on start( but items is not yet loaded), but not fired when items is loaded( categoryItems changed ) It works if I put autorun inside it, but its not clear why I should do it. — You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub <#2652 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAN4NBANGSO6CULGL4CB3BDSSNUQNANCNFSM4UG2W7NQ> .

[mogadanez]

Ok, autorun is probably ok, and works fine.

can I combine useEffect deps for incoming params and local state ?

like

let component = observer(({componentParam} )=> {
  let { matchParam }= useParams();
  const [componentState, setComponentState] = useState([])
  const {categoryStore } = useStores()
  const { categoryItems } = categoryStore


  useEffect(
      () =>
          autorun(() => {
                  // somehow use componentParam, componentState matchParam and categoryItems 
            }
          }),
      [componentParam, matchParam, componentState]
  )

will this work? or autorun can also watch over componentParam, matchParam, componentState and I can skip them from deps?

I understand that it probably not best way, I just trying to find a way how to work with legacy code without complete rewriting

[danielkcz]

In the case of autorun you generally want to avoid any deps ([]) because MobX is the one guarding for changes. Any effect rerun would mean recreating autorun without the reason.

[mogadanez]

But if I provide additional deps - what side effects I can catch?

Its now academic questions to understand how it works

[danielkcz]

Adding deps makes sense only in case the effect depends on non-observable value. I suppose in your case it's componentParam? And I guess useParams is not working with observables too. However, in that case, consider if you really need to rerun the side effect based on those changes. Sometimes it might make more sense to split into more effects. There is no golden bullet here.

[mweststrate]

If you create a new effect by changing deps, the old autorun is disposed (that is why returning the autorun is important) and a new one is set up. So yes, that works as expected.

…

On Mon, Nov 30, 2020 at 12:17 PM Daniel K. ***@***.***> wrote: Adding deps makes sense only in case the effect depends on non-observable value. I suppose in your case it's componentParam? And I guess useParams is not working with observables too. However, in that case, consider if you really need to rerun the side effect based on those changes. Sometimes it might make more sense to split into more effects. There is no golden bullet here. — You are receiving this because you commented. Reply to this email directly, view it on GitHub <#2652 (reply in thread)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAN4NBH4YSP7EFKMPYVOTZ3SSOEONANCNFSM4UG2W7NQ> .