reduxjs / redux-toolkit

The official, opinionated, batteries-included toolset for efficient Redux development
https://redux-toolkit.js.org
MIT License
10.71k stars 1.17k forks source link

Problem when updating from 1.9 to 2.0 #4142

Open venelinpetrov opened 8 months ago

venelinpetrov commented 8 months ago

Hello team! :wave:

We decided to update and take advantage of all the new goodies in redux-toolkit 2, but encountered a problem in our cypress tests.

I tracked down the issue to the autoBatch option that is now 'auto' by default.

Here is a very simple cypress test that fails, because the Processing payment... overlay never appears.

it('Credit / Debit card', () => {
    cy.clicks('next-button');
    cy.delayRequest({ url: managerMockApi.lease.payBalance.url });
    cy.clicks('pay-btn');
    cy.gets('status-overlay-text').should('have.text', 'Processing payment...'); // <<<<<< FAILS
    cy.wait('@lease.payBalance').then(({ request }) => {
        expect(request.body.amount).to.equal(7235);
        expect(request.body.paymentMethod.methodType).to.equal('CREDIT_CARD');
    });
    cy.contains('Payment successful');
});

This overlay should appear while the query for @lease.payBalance is in flight, but it seem like we never get to that "loading state".

Adding

enhancers: (getDefaultEnhancers) =>
    getDefaultEnhancers({
        autoBatch: { type: 'tick' },
    }),

fixes the issue, which leads me thinking the problem is indeed with how we use RTK.

Note: the tests are only failing (inconsistently) in github actions (we use cypress-io/github-action@v6). Locally, everything works fine.

I know this is a bit of a long shot, but can you please advice at least where to start looking at?

Thanks!

markerikson commented 8 months ago

What is the actual UI code that is trying to display this text? What state is it looking for? What sequence of logic triggers showing that text?

venelinpetrov commented 8 months ago

We have several mutation endpoints and a hook that abstracts all these. Example:

export const usePay = ({
...
}: UsePay) => {
    const [data, setData] = useState<PayResponse>();
    const [
        payLease,
        {
            data: payLeaseData,
            status: payLeaseStatus,
            reset: resetPayLease,
            isLoading: isPayLeaseLoading,
        },
    ] = usePayLeaseMutation();

    const [
        payMerch,
        {
            data: payMerchData,
            status: payMerchStatus,
            reset: resetPayMerch,
            isLoading: isPayMerchLoading,
        },
    ] = usePayShopCartMutation();

    const [
        payUpfrontDiscount,
        {
            data: payUpfrontDiscountData,
            status: payUpfrontDiscountStatus,
            reset: resetPayUpfrontDiscount,
            isLoading: isPayUpfrontDiscountLoading,
        },
    ] = usePayUpfrontDiscountMutation();

    const [status, setStatus] = useState<QueryStatus>(QueryStatus.uninitialized);

    useObserve(() => {
        setStatus(payLeaseStatus);
        setData(payLeaseData);
    }, [payLeaseStatus]);

    useObserve(() => {
        setStatus(payMerchStatus);
        setData(payMerchData);
    }, [payMerchStatus]);

    useObserve(() => {
        setStatus(payUpfrontDiscountStatus);
        setData(payUpfrontDiscountData);
    }, [payUpfrontDiscountStatus]);

    return {
        data,
        status,
        isLoading:
            isPayLeaseLoading ||
            isPayMerchLoading ||
            isPayUpfrontDiscountLoading ||
        payLease: useCallback(...),
        payMerch: useCallback(...),
        payUpfrontDiscount: useCallback(...),

    };
};

The hook returns several things along with the status variable that is of type @reduxjs/toolkit/query.QueryStatus. Then, another component, <StatusIndicator status={status} /> takes the status and decides what to display based on it.

switch (payStatus) {
    case QueryStatus.rejected:
        return (...);
    case QueryStatus.pending:
        return (
            <StatusOverlay text="Processing payment..." /> // <<<< This is the piece in question
        );
    case QueryStatus.fulfilled: {...}
    case QueryStatus.uninitialized:
    default:
        return null;
}
markerikson commented 8 months ago

Hmm. First observation is that I believe we have the status field marked as @deprecated - it exists, but it's more for internal usage, and you should rely on the derived booleans instead.

Second, this feels like the mutation is resolving fast enough that the fulfilled action is dispatched before the UI has a chance to re-render. How is this mutation request being handled in the test environment?

venelinpetrov commented 8 months ago

Thanks for the answer, can you suggest on how to replace status in this case?

How is this mutation request being handled in the test environment?

Mock response with cy.intercept(),