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

Add legacy decorator support for TypeScript files #5642

Closed
Timer opened this issue Oct 31, 2018 · 9 comments
Closed

Add legacy decorator support for TypeScript files #5642

Timer opened this issue Oct 31, 2018 · 9 comments

Comments

@Timer
Copy link
Contributor

Timer commented Oct 31, 2018

Oops, looks like we forgot to turn legacy decorators on:

Support for the experimental syntax 'decorators-legacy' isn't currently enabled (24:3)

I'm surprised there hasn't been a complaint yet!

@metawave
Copy link

@Timer Used craco so far to turn decorators on (use it for mobx), but if cra would support it, that would be great!

@smithdalec
Copy link

Looking forward to this going in 2.1.1 - the decorators are the one thing preventing me from migrating to react-scripts from react-scripts-ts

@ghost
Copy link

ghost commented Oct 31, 2018

I've just hit this, trying to port an app using MobX.

@metawave
Copy link

metawave commented Nov 4, 2018

@AndyCJ Warning if you use MobX with @observable decorators on class properties. Babel 7 doesn't support class property annotations yet! This results in no change detection from MobX on those, so your observers don't get notified when values have changed. You have to enable the @babel/plugin-proposal-class-properties to overcome this. You can do it without ejecting using something like craco or another config overloading lib/tool. Maybe this could be added to CRA as well in addition to the decorators already added by @Timer in #5659 ? Without class property decorators the decorators support is not really complete.

@ianschmitz
Copy link
Contributor

@metawave that plugin should already be included in CRA

@ghost
Copy link

ghost commented Nov 5, 2018

@metawave That package was added 2 months ago: https://github.com/facebook/create-react-app/blame/fbfa21621ae3c2a66e63daeca1a98beb09860227/packages/babel-preset-react-app/package.json#L21

Unless I'm missing something?

EDIT: Interestingly I currently have a @MobXReact.observer export class MapView extends React.Component which isn't calling render when I change a @MobX.observable.
I'm currently trying to get to the bottom of the cause.
The symptom certainly matches what you say, but the config does appear to load that babel plugin.

EDIT 2: It was a mistake on my part, I was dereferencing an observable in a render callback, which isn't monitored by @MobXReact.observer.
So @babel/plugin-proposal-class-properties appears to be working fine.

@metawave
Copy link

metawave commented Nov 5, 2018

@AndyCJ

I do see a difference when compiling. On a vanilla project created with cli, the decorators on class fields don't get transpiled:

/***/ "./src/state.ts":
/*!**********************!*\
  !*** ./src/state.ts ***!
  \**********************/
/*! exports provided: default */
/***/ (function(module, __webpack_exports__, __webpack_require__) {

"use strict";
__webpack_require__.r(__webpack_exports__);
/* harmony export (binding) */ __webpack_require__.d(__webpack_exports__, "default", function() { return State; });
/* harmony import */ var _Users_marcel_Source_test2_node_modules_babel_runtime_helpers_esm_classCallCheck__WEBPACK_IMPORTED_MODULE_0__ = __webpack_require__(/*! ./node_modules/@babel/runtime/helpers/esm/classCallCheck */ "./node_modules/@babel/runtime/helpers/esm/classCallCheck.js");
/* harmony import */ var mobx__WEBPACK_IMPORTED_MODULE_1__ = __webpack_require__(/*! mobx */ "./node_modules/mobx/lib/mobx.module.js");

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 = '';
};

for typescript file

import { observable } from 'mobx';

export default class State {
  @observable text: string;

  constructor() {
  	this.text = '';
  }
}

Only when configuring babel to use this plugin, the resulting code is complete:

/*!**********************!*\
  !*** ./src/state.ts ***!
  \**********************/
/*! exports provided: default */
/***/ (function(module, __webpack_exports__, __webpack_require__) {

"use strict";
__webpack_require__.r(__webpack_exports__);
/* harmony export (binding) */ __webpack_require__.d(__webpack_exports__, "default", function() { return State; });
/* harmony import */ var _Users_marcel_Source_test2_node_modules_babel_runtime_helpers_esm_initializerDefineProperty__WEBPACK_IMPORTED_MODULE_0__ = __webpack_require__(/*! ./node_modules/@babel/runtime/helpers/esm/initializerDefineProperty */ "./node_modules/@babel/runtime/helpers/esm/initializerDefineProperty.js");
/* harmony import */ var _Users_marcel_Source_test2_node_modules_babel_runtime_helpers_esm_classCallCheck__WEBPACK_IMPORTED_MODULE_1__ = __webpack_require__(/*! ./node_modules/@babel/runtime/helpers/esm/classCallCheck */ "./node_modules/@babel/runtime/helpers/esm/classCallCheck.js");
/* harmony import */ var _Users_marcel_Source_test2_node_modules_babel_runtime_helpers_esm_applyDecoratedDescriptor__WEBPACK_IMPORTED_MODULE_2__ = __webpack_require__(/*! ./node_modules/@babel/runtime/helpers/esm/applyDecoratedDescriptor */ "./node_modules/@babel/runtime/helpers/esm/applyDecoratedDescriptor.js");
/* harmony import */ var _Users_marcel_Source_test2_node_modules_babel_runtime_helpers_esm_initializerWarningHelper__WEBPACK_IMPORTED_MODULE_3__ = __webpack_require__(/*! ./node_modules/@babel/runtime/helpers/esm/initializerWarningHelper */ "./node_modules/@babel/runtime/helpers/esm/initializerWarningHelper.js");
/* harmony import */ var mobx__WEBPACK_IMPORTED_MODULE_4__ = __webpack_require__(/*! mobx */ "./node_modules/mobx/lib/mobx.module.js");

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);

/***/ }),

@Timer
Copy link
Contributor Author

Timer commented Nov 5, 2018

Hmm, that sounds like a bug. Can you file a new issue please?

@metawave
Copy link

metawave commented Nov 7, 2018

done! #5741

@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