AFAICT there's still no real solution for this problem:
constructor() {
super();
this.onToggle = this.onToggle.bind(this); // Flow error: Cannot assign `this.onToggle.bind(...)` to `this.onToggle` because property `onToggle` is not writable.
}
The closest thing to official guidance I can find is a comment from @samwgoldman recommending this workaround:
(this: any).onToggle = this.onToggle.bind(this);
The above-linked issue was closed with no solution, except the workaround given in that comment. I've had to go searching for that comment several times over the last couple of years to remind myself what's the 'official' workaround.
The Flow docs for annotating React components still have no mention of binding methods on construction. If no solution to the root of the problem is in sight, I think it would be good to mention the binding issue and the workaround in the official docs.
I'm having this same issue. This is particularly annoying, and I don't know if it's just me, but it also messes up the syntax highlighting in my editor when I use this workaround.
I have the same problem here. As josh-degraw mentioned, the workaround is causing other problems with the editor but even new flow errors such as "missing property" in class.
I thought the recommendation was to declare your methods using property initialiser syntax?
onToggle = event => { /* ... */ };
as opposed to
onToggle(event) { /* ... */ }
I think this allows the methods to be bound in the constructor. Have I misunderstood?
@morcs yeah that seems like the best workaround (I didn't know about it when I posted this issue). I think the docs should mention that too. But I think that is currently still 'proposed' syntax and needs the transform-class-properties Babel plugin to work. So this should probably be noted in the docs, and it should probably also mention the other workaround ((this:any)...
) for people who can't use property initialisers.
if you have regular js classes, and in parent class you have this.f = () => {}
In child class, you cannot call super.f();
There's also this issue with arrow properties - you can't mock them in tests like this:
https://github.com/airbnb/enzyme/issues/1432#issuecomment-351670288
https://github.com/airbnb/enzyme/issues/1432#issuecomment-363889823
You can let flow know that you want a class property writable with a type declaration for that class property. Example:
constructor() {
super();
this.onToggle = this.onToggle.bind(this);
}
onToggle: () => void // <----- HERE
onToggle() {
// ... something something
}
If you run into this problem because you're not using a build step, and using Flow's comment syntax, the solution provided by @kbui-prosper also works with comment syntax:
constructor(props) {
super(props);
this.handleClick = this.handleClick.bind(this);
}
/*:: handleClick: () => void */
handleClick(event /*: SyntheticEvent<HTMLInputElement> */) {
// ... something something
}
I prefer the slightly terser alternative, which means you don't need to worry about re-typing your method definition in a type declaration:
constructor() {
super();
this.onToggle = this.onToggle.bind(this);
}
onToggle: Function;
onToggle() {
// ... something something
}
Is there a way in flow to import a class annotation like this contrived example?
type class Comp = {
props: {
name: string,
label: string,
onChange: (event:SyntheticEvent<>) => void,
},
state: {
active: boolean,
},
handleKeyPress: (e:SyntheticMouseEvent<>) => void,
};
class TextButton<Comp> extends PureComponent {
constructor(props:Props) {
super(props);
this.handleKeyPress = this.handleKeyPress.bind(this);
}
handleKeyPress(e) {
if (e.key === 'Enter') {
this.props.onChange(e);
}
}
render() {
return (
<button
name={this.props.name}
onKeyPress={(e) => this.handleKeyPress(e)}
>
{ this.props.label}
</button>
);
}
}
@danm am I wrong to thing that what you are talking about already exists in implements
syntax Interface Type?
What I want to know is what this error means — could somebody @flowtype of @facebook team comment on why class property declarations are covariant by default? What are we protecting except the obvious situation when I call this.thing()
but it not that thing()
anymore?
~This is not a solution per se but it may be better way to use function binding (feel free to correct me):~
/* don't do this */
<SomeInput onChange={this.handleChange.bind(this)} />
This is a no-no by React standards.
@ermik you don't want to do that, because then SomeInput
gets a different, !==
reference on every render. That needs to be bound in the constructor and stored on the instance to avoid this rendering performance footgun.
@ljharb there's a warning I did not see previously. I've amended my comment.
@ermik that has the identical risk. It must and only be constructor-bound to avoid this footgun (or bound in a clsss field)
Agreed. Don't bind in render methods :)
It is unnecessary to bind this to your method in the constructor
when calling to the event handler using an arrow function like this:
(e) => this.handleKeyPress(e)
The this
value inside the arrow function is taken from the enclosing lexical context
Example:
// @flow
import React from 'react';
type MyComponentPropsType = { show: boolean};
type MyComponentStateType = {|
show: boolean
|};
class MyComponent extends React.Component<MyComponentPropsType, MyComponentStateType> {
props: MyComponentPropsType;
state: MyComponentStateType;
handleToggle: (event: any) => void;
constructor(props: MyComponentPropsType) {
super(props);
this.state = { show: this.props.show };
}
handleToggle(event: any) {
// Do something with the event
// ....
this.setState(state => ({
show: !state.show
}));
}
render() {
return (
<button type="button" onClick={(e) => this.handleToggle(e)} >
<span> {this.state.show ? 'Hide' : 'Show'} </span>
</button>
);
}
}
export default MyComponent;
Flowtry
Babel _Requires to add the @babel/plugin-proposal-class-properties plugin_
@joselcc that has a performance impact, as now the function is repeated N times instead of being shared on the prototype. The only good solution is a constructor-hound instance method - never put arrow functions in class fields.
As this is a pretty common pattern, it would be great to see some progress on this. I would argue that it both clutters the code to annotate every method that needs to be bound, as well as it is hard to enforce in a large code base.
What about adding a configuration option to disable this rule?
I guess if you use hooks, you can avoid classes and don't need to worry about it any more?
We're on react-native, which just recently got hooks. And certainly, while new code would use hooks where applicable, there is still a large legacy code base with classes which will live on for a long time 😞
(this: any).funName
is given me a Unexpected token :Flow(ParseError)
. Does anyone knows what to do?
Thanks
@ThiagoMiranda ,
use this way
constructor(props) {
super(props);
this.handleClick = this.handleClick.bind(this);
}
/*:: handleClick: () => void */
handleClick(event /*: SyntheticEvent<HTMLInputElement> */) {
// ... something something
}
as @warpr mentioned above.
The best solution i've found so far:
````javascript
import { boundMethod } from 'autobind-decorator';
class MyClass extends Component
/* no constrcutor needed */
@boundMethod
handleClick( e ) {
/* */
}
}
````
See https://github.com/andreypopp/autobind-decorator#readme for details.
Be aware that you need to disable annotation warnings in .flowconfig (esproposal.decorators=ignore) and provide stub for autobind-decorator in flow-typed directory.
Most helpful comment
I thought the recommendation was to declare your methods using property initialiser syntax?
as opposed to
I think this allows the methods to be bound in the constructor. Have I misunderstood?