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

TypeScript decorators: decorators on class properties don't get transpiled #5741

Closed
metawave opened this issue Nov 7, 2018 · 3 comments
Closed

Comments

@metawave
Copy link

metawave commented Nov 7, 2018

When using class properties decorators, the compiler doesn't generate the nessecary Object.defineProperty calls to make real properties, making the use of mobx or libraries which depend on decorators useless.

Steps to reproduce:

  1. use latest create-react-app (im my case 2.1.1) to create a new project with typescript support: npx create-react-app myproject --typescript
  2. Add mobx, mobx-react to the project yarn add mobx mobx-react
  3. Create a model class with @observable properties
  4. Decorate a React component with the @observer decorator and add the Model class as an instance to this component's props and use it in the render() method

Expected:
The observer get's notified, when property on the model has changed and therefore re-rendered

Actual:
The observer doesn't get notified

What went wrong:
The decorators on properties of the class don't get generated as javascript, therefore the changed notification doesn't get fired and the observer doesn't get it.

Reason:
The reason is that in transpilation to javascript the decorators on class propeties/fields don't get generated.

var State = function State() {
  Object(_Users_marcel_Source_test2_node_modules_babel_runtime_helpers_esm_classCallCheck__WEBPACK_IMPORTED_MODULE_0__["default"])(this, State);
  this.text = '';
};

instead of

var _class, _descriptor;
var State = (_class = function State() {
 Object(_Users_marcel_Source_test2_node_modules_babel_runtime_helpers_esm_classCallCheck__WEBPACK_IMPORTED_MODULE_1__["default"])(this, State);
 Object(_Users_marcel_Source_test2_node_modules_babel_runtime_helpers_esm_initializerDefineProperty__WEBPACK_IMPORTED_MODULE_0__["default"])(this, "text", _descriptor, this);
  this.text = '';
}, (_descriptor = Object(_Users_marcel_Source_test2_node_modules_babel_runtime_helpers_esm_applyDecoratedDescriptor__WEBPACK_IMPORTED_MODULE_2__["default"])(_class.prototype, "text", [mobx__WEBPACK_IMPORTED_MODULE_4__["observable"]], {
  configurable: true,
  enumerable: true,
  writable: true,
  initializer: null
})), _class);

Steps to make it work:

  1. add craco to the project yarn add @craco/craco
  2. add class properties and decorators babel plugin also to this project yarn add --dev @babel/plugin-proposal-class-properties @babel/plugin-proposal-decorators
  3. add following craco.config.js to the project:
module.exports = {
    babel: {
        plugins: [
            ["@babel/plugin-proposal-decorators", { legacy: true }],
            ["@babel/plugin-proposal-class-properties", { loose: true }]
        ]
    }
};
  1. change react-scripts start to craco start in package.json, scripts.

Now it should work like expected.

Demo Project:
I've created a sample really quick and dirty project on github, with both variants, a non-working (on master) and a working (on branch working):
https://github.com/metawave/property-decorator-demo/

@lucax88x
Copy link

lucax88x commented Nov 9, 2018

Just migrated from the CRA 1 with microsoft typescript to CRA 2 --typescript (BEAUTIFUL, thanks).

but this issue is preventing me to continue unlucky.

@ianschmitz
Copy link
Contributor

ianschmitz commented Nov 13, 2018

So i did some digging into this. It appears that @babel/plugin-transform-flow-strip-types is interfering with things. If i use your sample repo and hardcode isFlowEnabled = false, the issue goes away. I'm not super familiar with the usage of opts in this file, as it appears that it's always an empty object, which means both isFlowEnabled, and isTypeScriptEnabled are true.

cc @Timer

EDIT: Moving @babel/plugin-transform-flow-strip-types to be defined after @babel/plugin-proposal-class-properties also seems to work. Not sure on the negative effects of this.

@Timer
Copy link
Contributor

Timer commented Nov 13, 2018

We can make flow only work on .js files @ianschmitz, see how I did decorators for TS (in our preset).

@lock lock bot locked and limited conversation to collaborators Jan 9, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

4 participants