Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Re-design model architecture #45

Open
Kashuab opened this issue Jun 1, 2023 · 1 comment
Open

Re-design model architecture #45

Kashuab opened this issue Jun 1, 2023 · 1 comment
Assignees

Comments

@Kashuab
Copy link
Owner

Kashuab commented Jun 1, 2023

Instead of automatically generating "user-editable" models, instead provide a modular mechanism for allowing resolved model instances to be "made into" other things. For example:

// Note: no "UserModel". EditableUser is manually implemented.
class EditableUser {
  // Choose what fields you want from the UserBaseModel
  name: string;

  constructor(user: UserBaseModel) {
    this.name = user.name;

    makeAutoObservable(this);
  }

  setName(name: string) {
    this.name = name;
  }
}

// Imagine this data comes from a query.
const user = new UserBaseModel({ id: 'wow', name: 'Dave' });

// user doesn't have any built-in mutator methods. You need to "cast" it:
import { cast } from 'mobx-depot';

const editableUser = cast(user, EditableUser);

console.log(editableUser.name); // -> "Dave"
console.log(editableUser.email); // TS error, otherwise undefined

editableUser.setName('Jeff');

console.log(editableUser.name); // -> "Jeff"
console.log(user.name); // -> "Dave"

// Imagine this happened in the RootStore via query/mutation response
user.name = "Bob";

console.log(editableUser.name); // -> 'Bob'



// ... Somewhere else in a component very far, far away...

const editableUserTwo = cast(user, EditableUser);
console.log(editableUserTwo  === editableUser ); // -> true

The as method should memoize by instance and class. For example, if two separate modules both called as(EditableUser) on the same user, they should in turn get the same EditableUser instance.

The problem

After implementing mobx-depot in a new app with my team, we discovered many use-cases where we wanted to store information about a model that didn't have to do with the GraphQL schema. We basically wanted to add some local state that was easily accessible. However, it felt odd to add it to the "model" just for the sake of accessibility.

We ended up making other classes and passing model instances around to be able to reference them. This made our state management quite a pain, having to instantiate things and remembering what we were supposed to be working with.

"Can I change the model's data? No, cause then something else may update unintentionally. Okay, let's make some sort of view model store so that components can selectively choose if they want dirty or real data. (some time later...) I want to change data. Do I change it in the model? The view model? Or classes X and/or Y?"

Needless to say, this sucked. We want to just "wrap"/"cast" an instance into something that has the methods/state we want. But we also need the identity to start with the model instance. The "what do I need" question should always be the model instance.

The solution

  1. Stop generating "user-editable" models

These only encourage developers to dump a bunch of random crap that various components may exclusively need. This is an anti-pattern that should be avoided. Had I not other developers I was working with, I most likely would've fallen into this pit.

This could also remove the need for sub-classing, so we shouldn't need makeModelObservable.

  1. Allow developers to "cast" a model instance into something else that has what they need

Models should only contain information that the server delivers. If you need to store local state that's related to a model, create a class that's compatible with the instance's model and cast it when you need to.

  1. When models are casted, stabilize the new instances

The first time a model is casted into something else, store a reference to it, so any next time it's casted the same way again the reference is the same. For instance, if components A and B both require an EditableUser, they should receive the same instance when casting the same model instance.

  1. When a casted model's state changes, do not update the original instance. However, its state should receive updates from the original instance

In the case of an EditableUser, performing this.name = "Jeff" shouldn't update the original user that was used to cast.
That being said, if a mutation was dispatched which resulted in the user.updatedAt updating with "real" data, the editableUser should receive that update.


Other notes

Can this refactor avoid subclassing entirely? The makeModelObservable workaround will never be encouraged by the MobX team. Crap like this will make developers want to avoid mobx-depot.

Plus, with the removal of user-editable models, this will require the base classes to be observables. When casting, how are the additional state and actions going to be made observable? Does it generate an entirely new observable? It probably should, otherwise we'd have to amend the annotations on the existing model instance (and what if a non-casted instance is being interacted with? under-the-hood it could have a bunch of random crap.)

@KashubaK KashubaK self-assigned this Jun 3, 2023
KashubaK added a commit that referenced this issue Jun 3, 2023
@KashubaK
Copy link
Collaborator

KashubaK commented Jun 3, 2023

Implemented in #46

@KashubaK KashubaK mentioned this issue Jun 3, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants