onSuccess not called by useDelete/deleteOne in pessimistic mode

[yelworc]

[This is a v4 bug, which I hope is still supported, since it's mentioned in the issue template? I did not have time to check if the issue exists in v5 as well.]

What you were expecting:

I'm using the deleteOne function returned by useDelete with the (default) pessimistic mutation mode, and onSuccess and onError callbacks supplied in the options object. One of these callbacks should be invoked after the delete request responds, corresponding to the response code.

What happened instead:

When the DELETE request fails, onError is invoked as expected. When the request succeeds though, onSuccess is not called. It is only called when I change the mutation mode to optimistic.

Steps to reproduce:

Repro: https://stackblitz.com/edit/github-718bhh69?file=src%2Fposts%2FPostList.tsx

Check the post list view; the delete button labeled "ERROR" will fail, logging "ERROR!" in the console. The button labeled "SUCCESS" however will not log "SUCCESS!", even though it does delete the post.

Environment

• React-admin version: 4.16.20
• Last version that did not exhibit the issue (if applicable): did not try v5
• Browser: Vivaldi 7.3.3635.11
• Stack trace (in case of a JS error): n/a

[fzaninotto]

Thanks for the report. This is very weird because we have a story for that case and it works in the 4.X branch:

https://react-admin-storybook-git-4x-marmelab.vercel.app/?path=/story/ra-core-dataprovider-usedelete-pessimistic--success-case

We also have a unit test, and it passes:

react-admin/packages/ra-core/src/dataProvider/useDelete.spec.tsx

Lines 171 to 205 in 5536a23

	it('when pessimistic, executes success side effects on success', async () => {
	const onSuccess = jest.fn();
	const dataProvider = testDataProvider({
	delete: () => Promise.resolve({ data: { id: 1 } } as any),
	});
	let localDeleteOne;
	const Dummy = () => {
	const [deleteOne] = useDelete(
	'foo',
	{
	id: 1,
	previousData: { id: 1, bar: 'bar' },
	},
	{ onSuccess }
	);
	localDeleteOne = deleteOne;
	return <span />;
	};
	render(
	<CoreAdminContext dataProvider={dataProvider}>
	<Dummy />
	</CoreAdminContext>
	);
	localDeleteOne('foo', {
	id: 1,
	previousData: { foo: 456 },
	});
	await waitFor(() => {
	expect(onSuccess).toHaveBeenCalledWith(
	{ id: 1 },
	{ id: 1, previousData: { foo: 456 }, resource: 'foo' },
	{ snapshot: [] }
	);
	});
	});

It is also a widely used features, and we've never seen an error report on that feature, that hasn't changed for a very long time (in the 4.x branch).

I looked at your repro and saw nothing unusual, so I don't understand why it works in our tests and not in your tests.

Could you try defining the side effects when calling the hook rather than calling the deleteOne function?

-   const [del, { isLoading }] = useDelete();
+       const options = {
+           mutationMode: 'pessimistic', // with optimistic mode, onSuccess is called
+           onSuccess: () => { console.log('SUCCESS!'); },
+           onError: () => { console.log('ERROR!'); },
+       };
+   const [del, { isLoading }] = useDelete(undefined, undefined, options);
    const mutate = () => {
        const params = { 
            id: fail ? 'INVALID_ID' : record.id, 
            previousData: record,
        };
-       const options = {
-           mutationMode: 'pessimistic', // with optimistic mode, onSuccess is called
-           onSuccess: () => { console.log('SUCCESS!'); },
-           onError: () => { console.log('ERROR!'); },
-       };
        console.log('DELETE', params, options)
-      return del('posts', params, options);
+      return del('posts', params);
    };   

[yelworc]

Thx for looking into it! Yeah, when I supply the options (or all 3 parameters) in the hook call, it does indeed work as expected. That's not a workaround I can easily use in my actual code though, since it's a generic component that applies common side effects to various mutations.

However, the unit test you linked still works when I change it the same way, i.e. just call useDelete() without arguments, and add the 3rd parameter to the localDeleteOne() call. Puzzling 😄

[fzaninotto]

The only difference I see is the react-query version (3.34 on our unit test 3.39 in your stackblitz). Can you try downgrading to see if it fixes the issue?

[yelworc]

Hmm, no difference with react-query@3.34.20, I'm afraid. And the other way round, that useDelete unit test also passes with react-query@3.39.3 😑