mobxjs / mobx

Simple, scalable state management.
http://mobx.js.org
MIT License
27.56k stars 1.78k forks source link

Document that makeAutoObservable for generators is not fool-proof #2492

Closed DeanIfalo closed 4 years ago

DeanIfalo commented 4 years ago

Hi guys, I tested makeAutoObservable automatically infers generators to be flows. (mobx 6.0.1) The source code is below.

import { autorun, flow, makeAutoObservable } from "mobx";

function delay(t: number) {
  return new Promise((resolve) => {
    setTimeout(resolve, t);
  });
}

class Doubler {
  value = 0;

  constructor(value: number) {
    this.value = value;

    makeAutoObservable(this);//!!
  }

  *increment() {
    yield delay(1000);
    return ++this.value;
  }
}

const doubler = new Doubler(10);

autorun(() => {
  console.log("value:", doubler.value);
});

doubler.increment();

I only get value: 10 printed once in the log, and the increment method will not trigger the notification that the value has changed. when I added the @flow decorator to the increment method, I got the same result.

if I use the same way as the flow + generator function example in this page and change the code to makeAutoObservable(this, { increment: flow });, I can get the expect result.

value: 10
value: 11

Is this a bug, or is it necessary to add annotation manually?

mweststrate commented 4 years ago

It shouldn't matter, in fact, even when not using flow at all it still should print twice. Would you mind putting this in a sandbox so that we can look at running code?

On Wed, Oct 7, 2020 at 9:03 AM DeanIfalo notifications@github.com wrote:

Hi guys, I tested makeAutoObservable automatically infers generators to be flows. (mobx 6.0.1) The source code is below.

import { autorun, flow, makeAutoObservable } from "mobx";

function delay(t: number) { return new Promise((resolve) => { setTimeout(resolve, t); }); }

class Doubler { value = 0;

constructor(value: number) { this.value = value;

makeAutoObservable(this);//!!

}

*increment() { yield delay(1000); return ++this.value; } }

const doubler = new Doubler(10);

autorun(() => { console.log("value:", doubler.value); });

doubler.increment();

I only get value: 10 printed once in the log, and the increment method will not trigger the notification that the value has changed. when I added the @flow https://github.com/flow decorator to the increment method, I got the same result.

if I use the same way as the flow + generator function example in this page https://mobx.js.org/actions.html#asynchronous-actions and change the code to makeAutoObservable(this, { increment: flow });, I can get the expect result.

value: 10 value: 11

Is this a bug, or is it necessary to add annotation manually?

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/mobxjs/mobx/issues/2492, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAN4NBDYC4SOVIZLVRTDWULSJQODTANCNFSM4SHBL4YA .

DeanIfalo commented 4 years ago

Reproduce the issue in a sandbox with Vallina Typescript template. (Edit) https://codesandbox.io/s/clever-mendeleev-7rvx1?file=/src/index.ts

DeanIfalo commented 4 years ago

@mweststrate https://codesandbox.io/s/purple-currying-kmhdz?file=/src/index.tsx Above with React Typescript template will run correctly. The result seems to depend on the typescript compiler. The js compiled with babel above runs correctly. By the way, I actually like to use babel to compile typescript.😊

mweststrate commented 4 years ago

Yes, there is currently no fool proof way in JavaScript to detect generators. so makeAutoObservable might not always detect them. Annotating manually with flow should work in any environment.

mweststrate commented 4 years ago

Updated docs

DeanIfalo commented 4 years ago

Hi @mweststrate, I found mobxjs/mobx-state-tree#279

Change the codesandbox project tsconfig.json to below can avoid this issue.

"compilerOptions": {
    "strict": true,
    "target": "ES6", //add
    "module": "commonjs",
...

Or modify the mobx source code through this suggestion.

Regards.