makeAutoObservable can't detect a generator after adding a default parameter value

[pasha-iwanov]

Hello!

I've faced the issue in a React Native (Expo) project, and it wasn't easy to find the reason. The problem is that in some configurations, generators can be detectable or not by makeAutoObservable depending on whether there are default parameter values or not.

Intended outcome:

Adding a default parameter value doesn't affect the behavior of makeAutoObservable.

import { makeAutoObservable } from "mobx";

const listStore = makeAutoObservable({
  *fetch(page) {
    console.log("fetch is called");
  },

  *fetchWithDefaultParameter(page = 0) {
    // it is never called
    console.log("fetchWithDefaultParameter is called");
  },
});

listStore.fetch(0);
listStore.fetch(1);

Actual outcome:

This code works and doesn't throw any exceptions, but the fetchWithDefaultParameter is never called now.

The reason is that Babel wraps the generator function in another function to set the default parameter value, and makeAutoObservable now can't detect that fetchWithDefaultParameter is a generator function. Instead it is detected as a normal function and is wrapped with action.

I know it's been discussed before (#2492) and there is documentation on that, but I still think this is worthy of a report and a discussion. This can be very confusing for a developer since the issue can appear at some point in project development, and a default parameter value is the last thing a developer would probably think about.

How to reproduce the issue:

https://codesandbox.io/p/sandbox/eloquent-ritchie-rh2357?file=%2Fsrc%2Findex.js%3A16%2C1

[urugator]

Bummer, native transpiles defaults like this:

function*foo(a=5) {};
// =>
function foo() {
      var a = arguments.length > 0 && arguments[0] !== undefined ? arguments[0] : 5;
      return function* () {}();
}

You may try to report it as a bug in native. Maybe it should produce:

function*foo() {
      var a = arguments.length > 0 && arguments[0] !== undefined ? arguments[0] : 5;
      return yield * function* () {}();
}

[pasha-iwanov]

@urugator Hmmm. But do the two options you provided behave the same way?

• The first (current native) transpiling result would assign the default parameter value during the generator function call.
• For the second, it would be assigned on the first .next() call.

Considering the fact that a default value can be not only a constant, I assume the current transpiling result is probably correct.

At this point, I think flow should be removed entirely from the makeAutoObservable magic and used explicitly only.

[pasha-iwanov]

What if MobX could warn a developer that the environment it works in can make generators undetectable by makeAutoObservable, and hint to wrap them in flow explicitly?

// isGenerator from mobx source code
function isGenerator(obj) {
  ...
}

function isGeneratorsDetectable() {
  function* generatorWithDefaultParameter(arg = 0) {
    return arg;
  }
  
  if (isGenerator(generatorWithDefaultParameter)) {
    return true;
  }

  return false;
}

Maybe It can be checked during the first makeAutoObservable call for the development environment?

[urugator]

But do the two options you provided behave the same way?

As you pointed out, they do not.

At this point, I think flow should be removed entirely from the makeAutoObservable magic and used explicitly only.

I don't have a strong opinion about it, but it would be a BC, so not much we can do about it atm.

make generators undetectable by makeAutoObservabl

You would have to consume mobx distribution that isn't compiled (contains generator syntax) and compile it with your compiler. I don't know if that's the case here, but generally it's not I think.

I think we could start with adding a warning to the documentation (that there is this specific problem on native and therefore we suggest using flow explicitely). PR welcome.

[pasha-iwanov]

@urugator Thank you for the answers! I'll create the documentation PR later this week.

Another option that comes to my mind is the following:

1. Make makeAutoObservable wrapping generators in flow configurable, set enabled by default.
2. Check for generator detectability on the top execution level (where Symbol, Map, and Set are checked for availability).
3. If the check fails, warn the developer and suggest disabling the automatic flow wrapping, promoting explicit flow use for the codebase.

The motivation is that now there is no obvious way to protect a developer (or a team) from the issue, and a warning can be at least something that might help here. Maybe this is overkill, though.

I would be happy to implement that!

[mweststrate]

@pasha-iwanov the problem is that step 2 generally doesn't work in libraries; because bundlers consume already down-compiled code, so it'd just check that we configured OUR compiler in this repo correctly, but not whether YOUR code is compiled correctly. This is also why we can't detect wrongly non standard configurations for class compilation etc.

However, if you could add a PR explaining how users could add a top-level check to their code that would be helpful for people running into this in the future. Thanks!