Array of Objects don't request an update

[knuspertante]

Expected Behaviour

ComponentOne should rerender when todo array changed.

Actual Behaviour

Changing the todo array in store class does not request a rerender in lit

Reproduce Scenario (including but not limited to)

https://codesandbox.io/s/brave-panini-ozg2f?file=/src/component-one.ts

Steps to Reproduce

Switch from "second" page to "home" page and back again

Or add ${testString} after ${todoListTemplate} in ComponentOne render return statement.

Platform and Version

Node 14.16.0 adobe/lit-mobx 1.0.1 lit 1.01 mobx 6.3.2

[knuspertante]

I found the reason:

"Make sure that an initial value is assigned to all state properties before calling makeAutoObservable(), otherwise the property will not be observable. This is a MobX limitation since we can’t use the TypeScript configuration "useDefineForClassFields": true due to conflict with Lit decorators mentioned above."

For more information https://vaadin.com/docs/latest/fusion/application/state-management

Is it possible to add this to readme?

Thx and nice work!

[StephenHaney]

Agreed the readme should probably mention that useDefineForClassFields must be set to false for Lit.

However, this conflicts with MobX's requirement that useDefineForClassFields be set to true. You can still make MobX work with it set to false, but you need to be more careful to always assign values to your observable properties (if you don't assign an initial value, the property will not be observable). ~~And as far as I can tell, you can't use decorator syntax with MobX if useDefineForClassFields is `false.~~ Edit: decorators still work either way.

There's a thread here from a member of the Lit team asking MobX about this: https://github.com/mobxjs/mobx/discussions/3216

[benjamind]

Apologies I missed both these comments somehow.

@justinfagnani do you have any more insight as to the right outcome here given the conflicting requirements of mobx and lit in this case?

[justinfagnani]

I think until standard decorators are available, or TypeScript adds the accessor keyword, we have a few workarounds:

1. Always assign an initial value to observable properties, even if undefined. This creates an instance property on the object that MobX can see and make observable.
2. Turn useDefineForClassFields on and use declare for reactive fields. Example
3. Turn useDefineForClassFields on and use declare for reactive fields and use the static properties field instead of decorators.

[benjamind]

1 seems the simpler thing to suggest to consumers of the library?

So probably an update to the README to indicate why this shows up (due to the mismatch in useDefineForClassFields between MobX and Lit decorators. I guess the recommendation is to keep useDefineForClassFields to false so that lit works, and then just recommend our users always assign an initial value for observable properties then?

[StephenHaney]

Thanks for all the info to everyone... I'll use option 2 personally as I'm not using reactive fields (or rarely) ~~That's probably the decision point — if you want to use MobX decorators, you have to use useDefineForClassFields set to true (as far as I can tell in testing).~~ Edit: part about decorators wasn't accurate.

[benjamind]

This is whats confusing me, we're using both Mobx and Lit decorators at Adobe right now with this, so I'm unsure what we did to support that :-)

[StephenHaney]

Oh interesting, what's your useDefineForClassFields setting? And are you on MobX 6? I'll keep toying around to see if I can figure out more as well.

[benjamind]

On one app we're on Mobx5, but on the other we're on Mobx6 and have not specified a value for useDefineForClassFields, though I suspect we have always initialized properties with a default value.

[StephenHaney]

Ok I need to walk back the bit about MobX decorators — I created a simple repro from scratch and with useDefineForClassFields set to false, MobX decorators still work as long as you assign an initial value to properties. Must be something else in my more complicated app that's breaking with useDefineForClassFields set to false. Editing my previous comments.

So my only observed drawback to the false config with MobX is that you must assign initial values to fields or they aren't made observable.

[benjamind]

Thanks for hte update, closing this out then as not reproducible.