quarrant / mobx-persist-store

Persist and rehydrate observable properties in mobx store.
268 stars 14 forks source link

Persisting domain object with a dependency on parent store #81

Open ekiwi111 opened 2 years ago

ekiwi111 commented 2 years ago

What's the preferred pattern to follow in the case of persisting a store with one to many relationship to its dependent domain objects?

E.g. example store

export class ShopStore {
  shopName: string | null = null;
  products: ObservableMap<Product['id'], Product> = observable.map();

  constructor(rootStore: RootStore) {
    makeAutoObservable(this, { rootStore: false }, { autoBind: true });
    makePersistable(this, {
      name: 'ShopStore',
      properties: ['shopName', 'products'],
      storage: window.localStorage,
    });
  }

 createProduct() {
    const product = new Product(this);
    this.products.set(product.id, product);
    return product;
  }
}

And its domain items:

export class Product {
  shopStore: ShopStore;
  id: string;

  constructor(appStore: ShopStore, id = v4()) {
    makeAutoObservable(this, { shopStore: false }, { autoBind: true });
    this.shopStore = appStore;
    this.id = id;
  }
}

In this example it'd throw the following error when persistence is triggered:

Converting circular structure to JSON
--> starting at object with constructor 'ShopStore'
| property 'products' -> object with constructor 'Array'
| index 2 -> object with constructor 'Array'
| index 1 -> object with constructor 'Product'
--- property 'shopStore' closes the circle
codeBelt commented 2 years ago

This is similar to https://github.com/quarrant/mobx-persist-store/issues/41 and https://github.com/quarrant/mobx-persist-store/issues/55 where you are trying to save a Map Object with references to classes. So basically you are doing this:

JSON.stringify(new Map(['productId', new Product(appStore)]);

JSON.stringify is trying to convert all the objects to JSON but Product has a reference to ShopStore and there is a circular dependency (Product > ShopStore > Product > ShopStore etc.). Every time JSON.stringify converts a Product to JSON it finds ShopStore and tries to convert that but then it finds more Product in products which is a property on ShopStore and the cycle never ends.

It is very hard to persist classes and/or Map's because if they are converted to JSON, then how do you rehydrate JSON back to classes and Map's. You would have to give makePersistable a custom storage controller that knows to turn the JSON back to classes and finally within a Map.

As of right now there is not a perfect solution for what you are trying to do. Persisting simple objects and arrays is currently the easiest way because in the end you are working with JSON.

quarrant commented 2 years ago

@codeBelt Thank you for your explanation. It's a problem with circular dependencies and architecture. If you want to resolve this just remove circular dependencies.

For example, if you want create relation between to classes you may do it with any primitive identifier

class Shop {
    constructor(public readonly name: string) {}
}

class Product {
    constructor(public readonly id: string, public readonly shopName: string) {}
}

const shops = new Map([["appStore", new Shop("appStore")]]);

const appStoreShop = shops.get("appStore")!;

const product_1 = new Product("1234", appStoreShop.name);
const product_2 = new Product("5678", appStoreShop.name);

const productShop = shops.get(product_1.shopName);

This example uses an architecture without circular dependencies. I hope it helps you =)

ekiwi111 commented 2 years ago

Is it possible to exclude a property at the class level from being persisted? e.g. makeAutoObservable(this, { rootStore: false }); as an example in 'mobx'.

It'd be great if there'd be a similar functionality in mobx-persist-store, e.g. in Product:

// something like
persistableConfig(this, {
    exclude: ['appStore']
});
quarrant commented 2 years ago

Nope, but I’ll think about it

ekiwi111 commented 2 years ago

Is there a way to perform a dynamic check on persistance? e.g. in isPersisting?

quarrant commented 2 years ago

isPersisting checks only top-level objects, not each nested level. We didn’t do this kind of work because it may be a really hard work.

For example, if I have a nested array with nested objects there are each object has a nested array I need to loop each array to find which property I want to exclude before persisting…

Anyway, I think your case is a good example what we couldn’t do with this library, because it will lead to low performance.

I know an approach how do it better, but not in this library. I need to think about it moreeeee

ThaJay commented 1 year ago

@ekiwi111 I just did this:


export class AppStore implements AppStoreType {
  constructor() {
    makeAutoObservable(this)

    makePersistable(this, {
      name: 'AppStore',
      properties: Object.keys(this).filter(item => item !== 'dataStore') as (keyof this)[]
    })
  }
...
}
ThaJay commented 1 year ago

But now I have the problem where some stores have other, smaller stores as properties, but those smaller stores also sometimes reference dataStore which references many other things.

It would be awesome to have a global ignore array in configurePersistable settings! Then we could just put our parent store names there and let it do its awesome thing.

I will try making them private, because I have read that makePersistable does not pick up privates but I'm not sure that's actually a feature so I'd be very happy to read about the recommended way of doing this.

Even if making parent stores private should or does not help here, it's a good idea because they should never be accessed from anywhere else but inside the class where they were defined.