Make Model and Datastore immutable

transitive-bullshit commented 9 months ago

make model and datastore immutable
remove previous accessors in favor of readonly public properties
fix a bug that previously existed if you cloned an existing model with debug: true, it would add the debug events twice; now all event handlers get deduped while retaining a stable ordering
AbstractModel.extend and Datastore.extend now support the old clone functionality as well as modifying any params
the types were a bit tricky in several places, but overall I'm happy with where it ended up

@rileytomasek one note: the reason I had to remove the class methods for merge* was because we're now calling some of these in the constructor of subclasses before super is called, so the previous method which was nicer for typing no longer worked. I tried a few diff variations but ended up using what I think is a much cleaner replacement for deepMerge and mergeEvents, which is a slight variation on the underlying package to cleanly handle undefined parameters as empty objects. Also added unit tests for these changes.

One downside to this approach is that it's difficult to reset events/params/context to an empty object, but I don't think this is too important since the user can still override individual fields on these objects to be undefined.

TODO: add unit tests which verify immutability after .extend().

vercel[bot] commented 9 months ago

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name	Status	Preview	Comments	Updated (UTC)
dexter	✅ Ready (Inspect)	Visit Preview	💬 Add feedback	Apr 21, 2024 2:46pm

transitive-bullshit commented 9 months ago

@rileytomasek fixed the missing context deepMerge in datastores and addressed #18 as well.

rileytomasek commented 5 months ago

@transitive-bullshit do you remember what’s left to complete if we wanted to merge this?

transitive-bullshit commented 5 months ago

@rileytomasek this has been ready to review & merge for awhile :) I fixed the one missing issue after you took a quick pass initially.

rileytomasek commented 5 months ago

@transitive-bullshit i saw this, and no commit message that appeared to address it, which is what prompted the qeustion:

TODO: add unit tests which verify immutability after .extend().

rileytomasek commented 4 months ago

@transitive-bullshit i made it so events and context are fully cleared if an empty object is passed instead of being undefined and added some tests.

i'm ready to merge and release this if you're on board with the changes.

transitive-bullshit commented 4 months ago

Looks good to me 💪

dexaai / dexter

Make Model and Datastore immutable #17