mobx-persist-store icon indicating copy to clipboard operation
mobx-persist-store copied to clipboard

Type issues when using multiple serializable properties

Open Tartiflettus opened this issue 2 years ago • 11 comments

Hello, first of all, great work on this package, the usage is intuitive, it's customizable, and it's not intrusive: one can use it on mobx stores basically as-is.

When using the package, I stumbled upon a possible issue when writing serializable properties. When using a single serializable property, everything works as intended, but when using 2 or more, the type checker complains about the arguments of the serializers.

Here is an example:

class Store {
  nb = 0;
  str = "0";

  constructor() {
    makeAutoObservable(this);
    makePersistable(this, {
      name: "Store",
      storage: window.localStorage,
      properties: [
        {
          key: "nb",
          serialize(v) { // v is deduced as this["nb" | "str"]
            return `${v}`;
          }
          deserialize(v) { // return type is deduced this["nb" | "str"]
            return parseFloat(v);
          }
        },
        {
          key: "str",
          serialize(v) { // same here for v
            return [v] as const;
          }
          deserialize(v) { // same here for return type
            return v[0];
          }
        }
      ]
    });
  }
}

After following the type declarations, it seems like the issue is that SerializableProperty is generic with 2 arguments: T (type of store), and P (key of store). So once in the properties array, these types are deduced in my example by Store and "nb" | "str".

While the enum deduction is correct when properties is defined as an array of the keys, it leads to this weird behaviour when using serializable properties, where all serializers must handle all used types.

From my perspective, it seems like the required behaviour would be to have each serializable property use it's own P, but after searching for a bit, I don't think that typescript allows this kind of thing :/. For now, I "solved" this by using a separate function that is correctly typed for each use, but degenerates its output type to use any (codesandbox).

What do you think about it, was this indeed not intended, or am I misusing the package?

In all cases have a nice day

Tartiflettus avatar Dec 19 '22 21:12 Tartiflettus

Great job! Thank for your issue. I’ll try to fix it on the weekend

quarrant avatar Dec 20 '22 04:12 quarrant

@viicky fixed in 1.1.3

quarrant avatar Dec 28 '22 07:12 quarrant

Thank you!

Tartiflettus avatar Jan 03 '23 09:01 Tartiflettus

I am still having the same issue in 1.1.3.

Here is a very simple example:

import { makeAutoObservable } from "mobx";
import { makePersistable } from "mobx-persist-store";

type Cookie = {
  name: string;
  isBaking: boolean;
};

export class ExampleStore {
  cookies = new Map<string, Cookie>();
  ovenSettings = {
    temperature: 200,
  };

  constructor() {
    makeAutoObservable(this, {}, {autoBind: true});
    makePersistable(this, {
      name: "CookieStore",
      properties: [
        {
          key: "ovenSettings",
          serialize: v => v,
          deserialize: v => v,
        },
        {
          key: "cookies",
          serialize: v => v,
          deserialize: v => v,
        },
      ],
    });
  }
}

Result:

index.ts:26:11 - error TS2322: Type '"cookies"' is not assignable to type '"ovenSettings"'.
index.ts:27:11 - error TS2322: Type '(v: this["cookies"]) => this["cookies"]' is not assignable to type '(value: this["ovenSettings"]) => any'.

dansimau avatar Jan 10 '23 21:01 dansimau

Okay I discovered it compiles fine in TS 4.4.4 (on CodeSandbox) but not in the latest version (4.9.4). It seems like the above error starts from TS 4.6 onwards.

dansimau avatar Jan 10 '23 21:01 dansimau

I’ll check it

quarrant avatar Jan 10 '23 21:01 quarrant

I get a similar issue when trying to mix string and serializable properties:

  constructor(store: RootStore) {
    this._store = store;
    makeAutoObservable(this, { _store: false }, { autoBind: true });
    makePersistable(this, {
      name: "GameStore",
      storage,
      properties: [
        "combat",
        "_lastUpdated",
        {
          key: "workees",
          serialize(value) {
            return value;
          },
          deserialize(_workees: Workee[]) {
            return _workees.map(workee => new Workee(workee));
          },
        },
      ],
    });
    log.info("initialized");
  }

This gives me an error on "combat" and "_lastUpdated"

Type '"combat"' is not assignable to type '"workees" | { key: "workees"; serialize: (value: this["workees"]) => any; deserialize: (value: any) => this["workees"]; }'

Type '"_lastUpdated"' is not assignable to type '"workees" | { key: "workees"; serialize: (value: this["workees"]) => any; deserialize: (value: any) => this["workees"]; }'

Redmega avatar Feb 04 '23 19:02 Redmega

Yes, this is still happening w "mobx-persist-store": "^1.1.3", and "typescript": "^5.1.3",

Any advice?

spencercap avatar Aug 09 '23 12:08 spencercap

I had a similar problem with mixed-up keys:

image

Currently I've worked around the problem using the explicit satisfies operator:

image

The type has been imported like this:

import { SerializableProperty } from 'mobx-persist-store/lib/esm2017/serializableProperty';

ArturBaybulatov avatar Nov 22 '23 22:11 ArturBaybulatov

any news on this?

Maxoos avatar May 04 '24 03:05 Maxoos