babel: v7 plugin-proposal-class-properties transform flow type annotation to undefined
Bug Report
Current Behavior
@babel/plugin-proposal-class-properties
transform flow type annotation to a property assigning undefined.
this may caught statement order unexpected error.
Input Code
let code = `
class B {}
class Logger extends B {
log: {
error: boolean
}
silent: boolean
log = {
error: true
}
get silent(): boolean {
return this._silent;
}
set silent(val: boolean) {
this._silent = !!val;
if (!this.log) {
throw new Error('#log should not be nil.')
}
Object.keys(this.log).forEach(k => { /* do something... */ })
}
constructor() {
super()
}
}
`
let r = require("@babel/core").transform(code, {
"presets": [ "@babel/preset-flow" ],
"plugins": [
[ "@babel/plugin-proposal-class-properties", { "loose": true } ]
]
})
console.log(r.code)
ouput =>
class B {}
class Logger extends B {
get silent() {
return this._silent;
}
set silent(val) {
this._silent = !!val;
if (!this.log) {
throw new Error('#log should not be nil.') // <----- will throw if the annotation property assigning undefined
}
Object.keys(this.log).forEach(k => { /* do something... */ });
});
}
constructor() {
super();
this.log = void 0; <<<<<----
this.silent = void 0; <<<<<---- with setter defined.
this.log = {
error: true
};
}
}
Expected behavior/code
class B {}
class Logger extends B {
get silent() {
return this._silent;
}
set silent(val) {
this._silent = !!val;
if (!this.log) {
throw new Error('#log should not be nil.')
}
Object.keys(this.log).forEach(k => { /* do something... */ });
}
constructor() {
super();
this.log = {
error: true
};
}
}
Environment
- Babel version(s): [e.g. v6.0.0, v7.0.0-beta.55]
- Node/npm version: [e.g. Node 8/npm 5]
- OS: [e.g. OSX 10.13.4]
- Monorepo [e.g. yes/no/Lerna]
- How you are using Babel:
js
Possible Solution
Should strip the ClassProperty
type of TypeAnnotation
.
About this issue
- Original URL
- State: closed
- Created 6 years ago
- Reactions: 26
- Comments: 25 (8 by maintainers)
Commits related to this issue
- Fix type annotation properties assigning to undefined. (#8417) — committed to allex/babel by allex 6 years ago
- Fix type annotation properties assigning to undefined. (#8417) — committed to allex/babel by allex 6 years ago
- Fix type annotation undefine, strip no initial properties. (#8417) — committed to allex/babel by allex 6 years ago
- Fix type annotation undefine, strip no initial properties. (#8417) — committed to allex/babel by allex 6 years ago
- Fix type annotation undefine, strip no initial properties. (#8417) — committed to allex/babel by allex 6 years ago
- fix(react): Ensure flow babel plugin runs before class properties plugin there is an issue with flow and class properties where values are undefined when they should not https://github.com/babel/bab... — committed to xing/hops by robin-drexler 6 years ago
- fix(react): Ensure flow babel plugin runs before class properties plugin there is an issue with flow and class properties where values are undefined when they should not https://github.com/babel/bab... — committed to xing/hops by robin-drexler 6 years ago
- fix(react): ensure flow babel plugin runs before other plugins There is an issue with flow and class properties where values are undefined when they should not https://github.com/babel/babel/issues/... — committed to untool/untool by robin-drexler 6 years ago
- feat(react): remove flow support There is an issue with flow and class properties where values are undefined when they should not babel/babel#8417 Flow support will be moved to hops — committed to untool/untool by robin-drexler 6 years ago
- feat(react): remove flow support There is an issue with flow and class properties where values are undefined when they should not babel/babel#8417 Flow support will be moved to hops — committed to untool/untool by robin-drexler 6 years ago
- feat(react): remove flow support There is an issue with flow and class properties where values are undefined when they should not babel/babel#8417 Flow support will be moved to hops — committed to untool/untool by robin-drexler 6 years ago
- fix(react): Ensure flow babel plugin runs before class properties plugin there is an issue with flow and class properties where values are undefined when they should not https://github.com/babel/bab... — committed to xing/hops by robin-drexler 6 years ago
- Add: react-native Button component Fix: Weird error from Babel/Flow interaction cf. https://github.com/babel/babel/issues/8417#issuecomment-430007587 — committed to kiwicom/orbit by deleted user 6 years ago
So, as much as this is personally annoying, b/c our existing codebase was written based on the existing Flow transform (i.e., no initial
undefined
value), it does appear the current babel@7 treatment is spec compliant (emphasis added):This sort of potential conflict comes with the territory of pre-spec transpilation and supra/meta-language constructs (i.e., Flow).
However, it raises the issue that there is, at least FWICT, now no suitable means to add a class property flow annotation except through a flow comment (which I would prefer to avoid if possible)… The conflict is unfortunate, but I don’t think it’s reasonable to break from language compliance in favor of flow compliance. Flow will necessarily have to evolve/change to address this edge case.
Best Recommendation for those in my/this situation is @lxcid’s recommendation to ensure
@babel/plugin-transform-flow-strip-types
runs (removingfoo: bar;
) prior to@babel/plugin-proposal-class-properties
in the plugin ordering. You’ll effectively be writing spec non-compliant code, but I assume Flow will address the issue with an easy/simple and hopefully automatic upgrade/codemod in the near future.See other discussion here: #8762 https://github.com/babel/babel/pull/8584 https://github.com/facebook/flow/issues/6811
I came across this issue using react-native 0.56 and babel v7.0.0-beta.47 as well. In my case, an instance property gets overwritten with
void 0
after already being set by the base class constructor. While I’m not sure that this is the best solution for solving this issue, I fixed the issue for my case by adding a line from the 6.x branch that skips initialisation for all unset instance properties. Here is the patch I used for beta.47:I’m having this issue too while running
jest
, in particular with example files like: https://github.com/facebook/react-native/blob/master/Libraries/Animated/src/createAnimatedComponent.js, in where type definition and declaration is being made with flow. If you have a look at the file pointed above, you will find:As you can see, some of the property type definitions have default values too, as for example
_invokeAnimatedPropsCallbackOnMount
, but, lets focus on_setComponentRef
, which is a function. If you keep looking at the code below, in particular the classconstructor()
method:The method
_setComponentRef
is being defined later onAnimatedComponent
source code, but, at the time this file gets compiled by babel, and@babel/preset-flow
is applied,_setComponentRef
gets initialized withvoid 0
, it does’t matterloose: true
orloose: false
, in the end its value will bevoid 0
. Having that in mind, at the timethis._setComponentRef = this._setComponentRef.bind(this);
runs,this._setComponentRef
has been set withvoid 0
, which causes:It seems something in the process is ignoring
_setComponentRef
definition later in the code, and only stays with_setComponentRef: Function
, which is indeed translated into_setComponentRef = void 0
because it does not specify any default value, but in the end this is not correct, because it should have been initialized to the method definition_setComponentRef(c) { ... }
that is located later in the code, as displayed above.If I remove
_setComponentRef: Function;
at the top ofAnimatedComponent
the error disappears, because_setComponentRef
gets initialized as expected, pointing to the method defined later in the code.Any thoughts?
Thanks.
Luckily with Flow there is a third solution but not with TypeScript 😦
@aliceriot Would you be able to do
to sidestep the issue?
I wonder if that behavior was intentionally left out, but based on the comments I would assume that this is not spec compliant and a regression.
I resolved it as described in
https://github.com/facebook/react-native/issues/20150#issuecomment-417858270
yes, I’m also prefer to strip these annotations without initial value. below diffs is my localized fixes:
This is fixed by https://github.com/babel/babel/pull/11178, which allows you to use
declare
in front of class fields to mark then as type-only and thus don’t initialize them toundefined
.Wow, I just literally spent 5 hours figuring out why my react-native iOS app was not doing any network request. It just didn’t resolve or reject. Kudo’s for the people here mentioning that
@babel/plugin-proposal-class-properties
is the dealbreaker.@loganfsmyth It’s not possible. because
Immutable.Record
create a class that initialize all fields asgetter
s andsetter
s when asetter
throws.Record#asMutable()
) . (And lets say it’s okay, it will break upmobx
☹️)But thanks, I’ll use the first option for now. But I think in
loose
mode it would be better when there are only type annotations, it will just strip them instead of doingthis.x = void 0