quarrant / mobx-persist-store

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

Is better encapsulation of entity properties possible? #89

Closed nathan-alden-sr closed 2 years ago

nathan-alden-sr commented 2 years ago

Normally, most examples and such describe entities that have public mutable fields, like this:

class Todo {
  public id: string;
  public title: string;
  public isFinished: boolean;
}

The issue with this is it violates encapsulation by exposing internals to callers. I'm wondering if MobX supports a classic DDD design like this that encourages mutation through methods and not by modifying fields directly:

import { makeAutoObservable } from "mobx";

export default class Todo {
  public readonly id;
  private _title = "";
  private _isFinished = false;

  public constructor(id: string, title: string) {
    makeAutoObservable(this);

    this.id = id;
    this.title = title;
  }

  public get isFinished() {
    return this._isFinished;
  }

  public get title() {
    return this._title;
  }

  public set title(title: string) {
    // Complex validation logic omitted for brevity
    this._title = title;
  }

  public toggle() {
    this._isFinished = !this._isFinished;
  }
}
nathan-alden-sr commented 2 years ago

I create a small demo app using the above entity. I can create new Todo instances just fine, as you'd expect; however, hydrating them fails, I suspect because of the lack of public mutable fields. Is there anyway to control hydration so I don't have to compromise encapsulation?

This screenshot is taken after hard-refreshing the browser so my Todo instance is hydrated from localforage:

image

nathan-alden-sr commented 2 years ago

I discovered that I could provide my own StorageController to makePersistable. If I can do this, I can write my own hydration logic for my entities. Unfortunately, no matter what signatures I use for the three methods, mobx-persist-store keeps reporting:

mobx-persist-store: Todos does not have a valid storage adaptor. Make sure the storage controller has 'getItem', 'setItem' and 'removeItem' methods."

const storageController: StorageController = {
  async getItem<T>(key: string) {
    const storedItem = await localForage.getItem(key);
    const value: T = storedItem as T;

    console.log(value);

    return value;
  },
  async removeItem(key) {
    await localForage.removeItem(key);
  },
  async setItem(key, value) {
    await localForage.setItem(key, value);
  }
};

class TodoStore {
  public todos: Todo[] = [];

  public constructor() {
    makeAutoObservable(this);

    makePersistable(
      this,
      {
        name: "Todos",
        properties: ["todos"],
        storage: storageController,
        stringify: false
      },
      {
        delay: 200
      }
    );
  }
}

I investigated the internal isStorageControllerLike function here and it seems like it doesn't handle async functions, which toString as [object AsyncFunction]. Does this mean I can't use a custom StorageController with localForage?

nathan-alden-sr commented 2 years ago

I was able to work around this by defining the functions as non-async:

const storageController: StorageController = {
  // eslint-disable-next-line @typescript-eslint/promise-function-async
  getItem<T>(key: string) {
    return localForage.getItem(key).then(storedItem => {
      const value: T = storedItem as T;

      console.log(value);

      return value;
    });
  },
  // eslint-disable-next-line @typescript-eslint/promise-function-async
  removeItem(key) {
    return localForage.removeItem(key);
  },
  // eslint-disable-next-line @typescript-eslint/promise-function-async
  setItem(key, value) {
    return localForage.setItem(key, value);
  }
};

Optimally, the library would natively support asynchronous functions since libraries like localForage are very popular.

nathan-alden-sr commented 2 years ago

Closing this because my questions have changed.