[Typescript][Babel] decorators and fields issue

[nulladdict]

Hi, thanks for the awesome work on mobx-keystone
I am really excited to move from MST for TS compatibility reasons

However, I've ran into an issue with decorators and class fields
The condensed example is: (sandbox link with full example)

@model("model")
class ExampleModel extends Model({
  /* ... */
}) {
  @modelFlow // this line throws at runtime
  asyncAction = _async(function*() { /* ... */ });
}

At the time modelFlow runs asyncAction field hasn't been defined yet
I believe the root of the problem is Babel's handling of decorators and properties (more info here)
However in TS case the above doesn't work (probably because of this babel issue)

In this case we can use mobx's decorate helper and plain assignment call of model() decorator (see commented out code in the example) but it's a little verbose

Considering that a lot of people (and CRA) nowadays use Babel to transpile their Typescript maybe decorators are not the way to go
Other concern is that decorators proposal has been pretty much redone from scratch and would not be compatible with "legacy" Typescript one

My suggestion would be getting rid of @model in favour of "name" argument of Model({ }) and shipping function alternatives of modelFlow and modelAction

Overall I'm not that sold on having decorators in mobx-keystone which is the only downside in comparison with mobx-state-tree I've experienced so far
decorate approach is alright but for me DX takes a big hit in that case

[xaviergonz]

Hey, thanks for the bug report. Non legacy babel decorator support should be fixed in the recently released 0.26.4 (let me know if it is not!).

Sadly @model cannot be moved just as a name of Model since it needs to change the final extended class, the closest it could get is:

class _M extends Model(...) {}
export const M = model(_M, "name")

or similar

for the others (modelAction and modelFlow) it shouldn't be too difficult to change them to something like

{
  someAction = modelAction("actionName", () => {....})
}

however it has a few downsides:

• it won't be able to get the action name automatically, so it would need to be specified again (duplicated)
• all actions would need to be specified as a class field rather than class method, so it wouldn't use the prototype (meaning each instance would have a copy of the function), which is a bit slower

I guess a way to avoid those two cons would be to turn model into a decorate alike thing:

class _M extends Model({...}) {
  someAction() {...}
  someFlow = _async(function *() {...})
}

export const M = model(_M, {
  someAction: modelAction,
  someFlow: modelFlow
})

I don't particularly like that syntax since that puts the "decorations" far from the decorated thing, but I see how that can be useful for people that don't want to deal with decorators. Would that last decorate idea be useful to you or is the fix to make it work with babel new decorators good enough?

[nulladdict]

Thanks for the quick response

The 0.26.4 fix works and is good enough for me 👍

The decorate idea is not as clean as decorators, but looking at the wider picture, I believe it would be viable alternative for people, who don't want to use decorators yet
However they can already use this approach:

const M = model("M")(_M)
decorate(M, { // from mobx
  someAction: modelAction,
  someFlow: modelFlow
})

I believe we can close this issue since bug is fixed and decorators will eventually make its way into the standard

[xaviergonz]

Thanks for testing! Closing until there are more requests to support this then