mobx-state-tree icon indicating copy to clipboard operation
mobx-state-tree copied to clipboard

`types.Date` expects a number in postProcessSnapshot

Open mridulmeh opened this issue 4 years ago • 2 comments

Bug report

  • [x] I've checked documentation and searched for existing issues
  • [ ] I've made sure my project is based on the latest MST version
  • [x] Fork this code sandbox or another minimal reproduction.

Disclaimer : I am using version 3.17.3 since v4 does not state that this bug has been addressed and my mobx is of an older version.

Sandbox link or minimal reproduction code https://codesandbox.io/s/dazzling-poitras-5e4f3?file=/src/App.js

Describe the expected behavior types.date should not have number as its type in postProcessSnapshot

Describe the observed behavior When trying to change values in postProcessSnapshot, typescript throws error as it expects a number instead of a date. This is weird, because when using an older version (v1.4.0), it used to work fine and the issues I see in the repo don't address it directly. Please advise what kind of steps should be taken or if it is a bug that should be solved

image

mridulmeh avatar Jan 07 '21 10:01 mridulmeh

Base on mobx 4.15.7 and mobx-state-tree 3.17.3

postProcessSnapshot args type definition

export declare type ModelSnapshotType<P extends ModelProperties> = {
    [K in keyof P]: P[K]["SnapshotType"];
} & NonEmptyObject;

types.Date type definition

Date: import("../internal").IType<number | Date, number, Date>;

IType type definition

export interface IType<C, S, T> {
    /** omit **/
    /**
     * @deprecated use `SnapshotOut<typeof MyType>` instead.
     * @hidden
     */
    readonly SnapshotType: S;
    /** omit **/
}

So typescript infer the snapshot.date's type is number, maybe this is a bug

This is weird, because when using an older version (v1.4.0), it used to work fine and the issues I see in the repo don't address it directly.

v1.4.0 of what? Did you mean mobx-state-tree v1.4.0 ? @mridulmeh

iChenLei avatar Jan 17 '21 03:01 iChenLei

Hey @mridulmeh - I think the culprit is https://github.com/mobxjs/mobx-state-tree/blob/master/packages/mobx-state-tree/src/types/primitives.ts#L162, where we call getTime() on the stored value, which will return the integer of milliseconds, even though we claim to support Date object instances as well.

I'm going to label this as a bug, and would welcome help if you're still interested. If not, no worries, I'm sorry it took so long for us to get back to you here

coolsoftwaretyler avatar Jun 28 '23 04:06 coolsoftwaretyler