Eslint-plugin-react: Should react/no-did-mount-set-state trigger on ES2017 async componentDidMount?

[bartolkaruza]

The rule react/no-did-mount-set-state triggers error on async componentDidMount function for a deferred setState. Basically, this.setState() is being called here after an async operation has been completed. It is not re-rendering straight after mounting, but only after some data has been retrieved from a server. Does it make sense to error on this?

async componentDidMount() {
    try {
      await someAsyncOperation();
    } catch (e) {
      this.setState({errorMessage: 'Failed to load data!'});
    }
  }

[ljharb]

Duplicate of/related to #56

[bartolkaruza]

I agree it is related, but the fix for #56 at https://github.com/yannickcr/eslint-plugin-react/commit/70fef7194783c1603f2b7d89e698647706aab658 will not cover the async await case. Could similar logic be added to any calls to setState after an await within the componentDidMount body?

[ljharb]

Yes, that seems reasonable.

[floriancargoet]

I believe that unfortunately, we can't be sure that a given statement is after an await statement:

async componentDidMount() {
    try {
      if (condition) {
        await someAsyncOperation();
      }
      this.setState({ ... }); // here
    } catch (e) {
      this.setState({ ... });
    }
  }

Depending on the condition, the line marked // here can be executed synchronously or asynchronously.

We can verify this by executing this snippet:

async function f(condition) {
    if (condition) {
      await 1;
    }
    console.log("after condition");
}
// will log "after call", then "after condition", proving that the execution was asynchronous
f(true); 
console.log("after call");

// will log "after condition", then "after call", proving that the execution was synchronous
f(false); 
console.log("after call");

[ljharb]

No, the spec for await means that the line after the await will be async no matter what is returned.

[floriancargoet]

I haven't written anything about a "returned" value. I'm not saying that await somethingSync() is synchronous.
I'm talking about a not executed await expression. Note that the await line is in an if statement.
I was surprised by the results, I was expecting an async execution even if the await line isn't executed.
Please run my example in the Chrome console and see for yourself.

[ljharb]

ahh, i see what you mean. sure, async functions execute synchronously until the first await, and if that code path isn't traversed, it will remain sync.

[floriancargoet]

Exactly. Because of that we can't always statically determine if a setState() is called asynchronously or not. I don't see how we could implement the suggested rule except for the simple cases (e.g. a setState immediately after an await).

[ljharb]

You're right; we'd have to treat a setState as sync unless all possible code paths included at least one await.

[tgroutars]

What about an option to never trigger when componentDidMount is async ?
It's clearly not perfect, lets the user shoot himself in the foot but less so than turning off the rule altogether

[slorber]

I would also like such an option. When componentDidMount is async it's rarely the case that the user would try to do a setState before having waited for some promise. The other alternative for me is to simply disable this rule, as it's annoying for the devs using async/await

[ljharb]

Rarely isn’t the same as never; it’s better to overwarn than underwarn here imo.

[slorber]

@ljharb not saying to make it a default, just an option to opt-out for overwarn

[ljharb]

I’d prefer to fix the default so it properly warns.

[frederikhors]

@ljharb any news on this? Can I help? What do you mean with

I’d prefer to fix the default so it properly warns.

@ljharb?

Can we use setState in an async componentDidMount() after an await something()?

[ljharb]

There's no news; the state of this issue is that it is still awaiting a PR.