This is less of an issue and more of a question on how you handle things.
React docs says to put ajax-requests in componentDidMount. But I would also like to cancel the request in componentWillUnmount as that data is not needed if the component isn't there. And to do so, I need a reference to the promise created in componentDidMount. I thought to achieve this by putting it on state, like so:
componentDidMount() {
this.setState({ serverRequest: this.props.fetcData() });
}
componentWillUnmount() {
const { serverRequest } = this.state;
if (serverRequest.isPending()) serverRequest.cancel();
}
(This is using bluebird btw, not native promises)
And this works, but the linter complains about the setState-call in componentDidMount.
I tracked down the issue where the rule was added (#345 & #346), and while the OP states that it follows your style guide, I can't find any reference to that rule in your docs.
So, my question is: Do you have any other way of keeping around references to async stuff created in componentDidMount such as ajax-requests, setTimeouts etc to be able to clean them up in componentWillUnmount? Or should I just disable the rule for that particular line?
Why not put the setState call in componentWillMount? That would be the inverse of componentWillUnmount
As the docs state that they belong in componentDidMount:
If you want to integrate with other JavaScript frameworks, set timers using
setTimeoutorsetInterval, or send AJAX requests, perform those operations in this method.
Touch茅 :-) according to https://github.com/yannickcr/eslint-plugin-react/blob/master/docs/rules/no-did-mount-set-state.md, "Updating the state after a component mount will trigger a second render() call and can lead to property/layout thrashing."
However, since setState uses ===, i'm not sure if it's the best place to put a promise. What about an instance property?
Just go this.serverRequest = ...?
Sure. You'll want to make sure it's managed in the appropriate places, in case props change and whatnot, but otherwise that seems like it would be fine.
Assigning to this works perfectly, and it is indeed buggy to use state. Thanks for the help!
Ref facebook/react#5870
@SimenB Can you elaborate on this.serverRequest = ...?
@ericgrosse you can see the full example in the React docs: https://facebook.github.io/react/tips/initial-ajax.html
Anything beyond that?
I lost "this" inside of componentDidMount() where I ran a Stomp (ActiveMQ) topic listener.
Inside that listener "this" became undefined.
An uneducated trick that worked was to save "this" into a variable "thus" before entering that listener
and I could use it inside the message handler doing stuff such as "thus.forceUpdate()" and similar.
@mobimation or you could use an arrow function, and just use this directly
Thanks @ljharb will try
@SimenB I don't understand either! The eslint-plugin-react says
Prevent usage of setState in componentDidMount (no-did-mount-set-state).
Anyone can explain?
Doing setState in componentDidMount will cause a visible render flash.
However, when doing server-rendering, anything that requires a browser environment must go in componentDidMount, since that runs client-only, but componentWillMount runs on both client and server.
This is an OK rule to disable.
If this is an OK rule to disable, is the benefit really worth the inconvenience of the overrides? I would not consider it a general "best practice" to not use setState in componentDidMount.
If you have problematic render flashes you can work around that with various methods, and these methods often will not include moving setState from componentDidMount, but another fix, leaving you with a fix as well as the eslint-disable.
There is also a scene that you must change your state depending on the what dom looks like. As a result, you must detect the dom element after mounting, which means you have to write the this.setState in componentDidMount.
I don't think this rule should be enabled by default. As the docs state:
Calling setState() in this method will trigger an extra rendering, but it is guaranteed to flush during the same tick. This guarantees that even though the render() will be called twice in this case, the user won鈥檛 see the intermediate state.
So it causes no visible side-effects.
Use this pattern with caution because it often causes performance issues.
This could justify configuring it as a warning, but certainly not as an error.
It can, however, be necessary for cases like modals and tooltips when you need to measure a DOM node before rendering something that depends on its size or position.
And it can be necessary.
It's definitely necessary for server rendering; I'm going to reopen this and change the default.
I've spent so long time because I have 'this.setState' on componentDidMount. It didn't work so I moved it to componentWillMount and now it's working.
I don't think that this rule makes any sense when using async/await.
Is there another, smarter way to do this?
async componentDidMount() {
try {
const response = await wpQuery();
const { headers, posts } = response;
this.setState({
posts,
maxPages: headers['x-wp-totalpages'],
maxPosts: headers['x-wp-total'],
});
} catch (error) {
this.setState({
error: 'Things went wrong, and the logic for error handling is irrelevant',
});
}
}
}
If it were a synchronous function, I'd understand a warning, and I'd probably want to see that warning.
@k1sul1 this guide forbids async/await currently, but either way, the rule has already been disabled.
setState is perfectly fine in componentDidMount, per https://github.com/airbnb/javascript/issues/684#issuecomment-340598319 and https://github.com/airbnb/javascript/commit/44dbd0bdc41d08eb5de8ad698099ae44240f4b0d - so no further comments on this issue are needed.
@ljharb about the rule having been disabled, setState still triggers no-did-mount-state even after ugrading these packages. Am I missing something?
````javascript
"eslint": "^4.14.0",
"eslint-config-airbnb": "^16.1.0",
@Sowed you're not missing anything; https://github.com/airbnb/javascript/commit/44dbd0bdc41d08eb5de8ad698099ae44240f4b0d has not yet been released. Wait for the next (major) version.
Doing setState in componentDidMount will cause a visible render flash.
For those still referring to this thread, it's worth pointing out this is no longer the case:
Calling setState() in this method will trigger an extra rendering, but it will happen before the browser updates the screen. This guarantees that even though the render() will be called twice in this case, the user won鈥檛 see the intermediate state
https://reactjs.org/docs/react-component.html#componentdidmount
This has not been released yet or am I missing something?
@eluchsinger nope, not yet - you can check that https://github.com/airbnb/javascript/commit/44dbd0bdc41d08eb5de8ad698099ae44240f4b0d is only in master and no released tags.
@ljharb @penx I find the language of the docs for componentDidMount() confusing. The method is invoked after the DOM is updated ("component...inserted into the tree") but you can setState immediately in componentDidMount() and essentially re-update the DOM again (now twice) "before the browser updates the screen". How is that possible? Perhaps you can update the DOM multiple times in one tick and the screen won't update until after the tick. Meanwhile, I gather, you can do things like measure the new DOM nodes, still in the same tick, before those DOM nodes are on the screen. Just guessing.
Because setState doesn鈥檛 update the DOM, it asks react to queue up a dom change - react batches them together.
@ljharb you're saying setting a state variable in a component constructor, then modifying it componentDidMount, will be batched together and executed as a single DOM change? I thought componentDidMount executed after a DOM change (namely component mounting). Then again, when the docs say "inserted into the tree" (for componentDidMount) I understand that to mean the DOM, but I don't think it's clear. The docs say that cDM can be used to measure the DOM, which certainly sounds like the DOM is already modified. Further, I understand "render" in react to mean create react elements in the virtual DOM, as opposed to DOM modification, as opposed to browser paint. But I find it difficult to ascertain exactly when these three things happen exactly in the lifecycle. I appreciate your comments on these issues!
Not will be, might be. You鈥檙e right tho, that cDM does run after the first DOM update, so setState in cDM will result in at least two updates total.
@ljharb okay, then I understand it to mean that you can modify the DOM multiple times in one tick, and then the browser will paint the current (last) state.
For anyone interested there's some more discussion of it here: https://twitter.com/dan_abramov/status/981712092611989509?lang=en. Though I'd love to find more info on it.
If your server requests take time you don't want to site around with a blank page. Async componentDidMount is the exact correct place to put server requests. Even if you get a page flash, that is your update coming back from the server. This error makes not sense because there is no better alternative.
@nickjuntilla whether cDM is async or not is immaterial; I agree that server requests belong there since that's the first chance you get when using SSR to do browser-only things - hence this issue being closed in https://github.com/airbnb/javascript/issues/684#issuecomment-340598319.
Most helpful comment
As the docs state that they belong in
componentDidMount: