Open fratzinger opened 1 year ago
I think these are good calls. We might want to keep using defineProperty
for __isClone
to make it non-iterable (so it won't be sent to the API). Alternatively we keep showing people how to remove the attribute in a before-all hook on the client.
Not calling instanceDefaults by default, use this.init(data) instead
This is the exact direction I wanted to take the constructor. :+1:
Thanks for reviewing and your comments! And thanks for your open mind on these changes! Highly appreciated.
We might want to keep using
defineProperty
for__isClone
to make it non-iterable (so it won't be sent to the API).
Good point! I forgot that defineProperty
is enumerable: false
by default. Because it was explicitly in feathers-vuex. As of now, I filter it through cleanData
see source
I'll change it to defineProperty
. That makes so much sense. Also for serialization. But the point stays, I think: don't set __isClone
conditionally in the constructor depending on the options but make it false
by default.
As of now, I filter it through cleanData
Yes, this is how I was doing it until a few months ago. I appreciate not having the extra setup step if we can avoid it.
Also for serialization. But the point stays, I think: don't set __isClone conditionally in the constructor depending on the options but make it false by default.
Good plan
Setup Store
BaseModel:
ModelDefineOptions
removed.__isClone
as class property predefined, not withdefineProperty
__isClone: false
by default. Simply make ittrue
instore.clone(...)
instanceDefaults
by default, usethis.init(data)
insteadconstructor(data)
withinstanceDefaults(data)
andinit(data)
has the problem, thatdata
always is on top and therefore need to be merged last.instanceDefaults
was called inBaseModel
. Callingthis.init
would callinstanceDefaults
two times.clientAlias / models / registerClient
Chore