Tomasz Mularczyk
Tomasz Mularczyk

Reputation: 36179

Should Promises be avoided in React components?

I've recently came across this error in React:

warning.js:36 Warning: setState(...): Can only update a mounted or mounting component. This usually means you called setState() on an unmounted component. This is a no-op. Please check the code for the BillingDetails component.

After digging I found out that this is caused because I do setState in unmounted component like this:

componentWillMount() {
    this.fetchBillings(this.props.userType);
}

componentWillReceiveProps({ userType }) {
    if (this.props.userType !== userType) {
        this.fetchBillings(userType);
    }
}

fetchBillings = userType => {
    switch (userType) {
        case USER_TYPE.BRAND:
            this.props.fetchBrandBillings()
                .then(() => this.setState({ isLoading: false }));
            return;
        default:
    }
};

fetchBillings is a redux-axios action creator which returns a promise

export const fetchBrandBillings = () => ({
    type: FETCH_BRAND_BILLINGS,
    payload: {
        request: {
            method: 'GET',
            url: Endpoints.FETCH_BRAND_BILLINGS,
        },
    },
});

The problem is that when user moves fast on site, component can be unmounted at the time promise resolves.

I found out lot of places around the project where I do something like this:

componentWillMount() {
    const { router, getOrder, params } = this.props;
    getOrder(params.orderId).then(action => {
        if (action.type.endsWith('FAILURE')) {
            router.push(`/dashboard/campaign/${params.campaignId}`);
        }
    })
}

and now I begin to think that using Promises in components could be anti-pattern as component can be unmounted at any time...

Upvotes: 2

Views: 655

Answers (1)

John Weisz
John Weisz

Reputation: 31934

The problem is that when user moves fast on site, component can be unmounted at the time promise resolves.

Since native promises are not interruptible, this is completely natural and should be expected at all times. You can overcome this in various ways, but you will ultimately need to track whether the component is still mounted, one way or another, and just don't do anything when the promise resolves/rejects if it's not.

Also, from the docs regarding componentWillMount:

Avoid introducing any side-effects or subscriptions in this method.

Considering this, I'd suggest using componentDidMount for initiating your fetch instead. Overall:

componentDidMount() {
    this._isMounted = true;
    this.fetchBillings(this.props.userType);
}

componentWillReceiveProps({ userType }) {
    if (this.props.userType !== userType) {
        this.fetchBillings(userType);
    }
}

componentWillUnmount() {
    this._isMounted = false;
}

fetchBillings = userType => {
    switch (userType) {
        case USER_TYPE.BRAND:
            this.props.fetchBrandBillings().then(() => {
                if (this._isMounted) {
                    this.setState({ isLoading: false });
                }
            });
            return;
        default:
    }
};

Additionally, although this is not directly related to your question, you will need to consider that you will have multiple parallel fetch calls running in parallel, leading to a data race. That is, the following is just waiting to happen at any time:

start fetch0
start fetch1
finish fetch1 -> update
...
finish fetch0 -> update

To avoid this, you can track your requests with a timestamp.

Upvotes: 2

Related Questions