Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Class properties not working with TS 2.x and reflect-metadata #534

Closed
hccampos opened this issue Sep 2, 2016 · 3 comments
Closed

Class properties not working with TS 2.x and reflect-metadata #534

hccampos opened this issue Sep 2, 2016 · 3 comments

Comments

@hccampos
Copy link

hccampos commented Sep 2, 2016

Currently, when using Typescript 2.0 (targeting es2015 and running through babel to end as es5) and reflect-metadata (needed for InversifyJS), class properties don't work. For instance:

import {action} from 'mobx';

class Foo {
  @action doStuff = (): void => {
    console.log('wut!');
  }
}

If you try to call doStuff in strict mode, you will get an error:

Uncaught Error: [mobx] Invariant failed: It is not allowed to create or change state outside an `action` when MobX is in strict mode. Wrap the current method in `action` if this state change is intended

From what I have been able to determine, this is caused by the property not being properly defined, similar to what led to #343. Take a look at decorators.ts. In this case, customArgs is set, so the number of arguments is actually 4, hence why the property does not get defined.

By changing lines 51-56 to always define the property, it starts working again, but then in some cases (i.e. when not using reflect-metadata) the property will be set twice, which is not ideal.

Not really sure what the best way to fix this would be since I haven't had the chance to dig deeper into the workings of Reflect and mobx. In any case, I think we should totally come up with a better fix than checking for the number of arguments. Alternatively, the fix could go in Typescript or Reflect, albeit I am sure the path there will be much slower.

PS: Also tried configuring Typescript to target ES5 directly instead of going through babel, but the results are similar.

@mweststrate
Copy link
Member

@hccampos hmm maybe this could be mitigated by building mobx decorators on top of reflect-metadata in the first place. In an ideal world that might even remove the need for the current Mobx weirdness like __runInitializers? Could be quite an endeavour, so if somebody wants to investigate this direction that would be great!

@mweststrate
Copy link
Member

Hmm the problem is probably that then the constructor / class needs a decorator to interpret the meta-data and actually apply the decorators

@mweststrate
Copy link
Member

Closing this issue for now, prefer to not support more decorator use cases (it is messy enough already ;-)) until the standard and it's implementation are settled.

@hccampos in your case, @action.bound method() {} might solve this specific case :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants