Fix flaky logout

[andrecasal]

submit(e.currentTarget) pushes the form's submission task to the macrotask queue in the event loop to process later. The click event bubbles up to <DropdownMenu.Portal> which will remove itself from the DOM by pushing the removal task to the macrotask queue.

The first task in the queue, the form's submission, is sent to the call stack and processed. This results in an asynchronous network request the browser asks the OS to make and await for and becomes associated with that DOM element.

While the browser waits for the OS to handle that network request, the second task in the macrotask queue, the removal of <DropdownMenu.Portal> and its <form> child, is sent to the call stack and processed.

This creates a race condition:

• If the network request is sent first, the logout will succeed (even if the response is not received/ignored).
• If the removal of <form> from the DOM (and the subsequent network request cancelation associated with it) succeeds first, the network request is canceled and the logout might fail, depending on whether the request has already been sent or not.

I think 😄

Fix
Detach the form submission from its DOM element:

<DropdownMenu.Item asChild>
	<Form
		action="/logout"
		method="POST"
		className="rounded-b-3xl px-7 py-5 outline-none radix-highlighted:bg-muted-500"
		onClick={e => {
			e.preventDefault()
			submit(null, { action: '/logout', method: 'POST' })
		}}
	>
		<button type="submit">Logout</button>
	</Form>
</DropdownMenu.Item>

Result

• No more canceled network requests seen in Playwright tests
• No more Form submission canceled because the form is not connected console warnings

Screenshots
Network tab:

Console:

[andrecasal]

Just noticed the logout POST request is still aborted. This only happens on Playwright tests.

//...
await page.getByRole('link', { name: user.name ?? user.username }).click()
await page.getByRole('menuitem', { name: /logout/i }).click()
await expect(page).toHaveURL(`/`) // 👈 This doesn't help

await page.goto('/login')
await expect(page).toHaveURL(`/login`)
//...

Edit: I'll open an issue just for test-related stuff.

[kentcdodds]

Is there a reason this is not the only line changed in the PR? Why can't we keep the action and method and use submit(e.currentTarget)?

[kentcdodds]

Whoops, never mind. I know why. I wonder if we could change this to e.stopPropagation to prevent the menu from closing?

[andrecasal]

Let me try that out

[andrecasal]

I'm not sure how the Radix UI internals work, but my suspicion is that, because the form takes up all of the <DropdownMenu.Item asChild>'s behavior, having an onClick on it is already too late, because it'll send the close comand to the <DropdownMenu.Portal>.

Adding onClick={e => e.stopPropagation()} to the button works 👌

[andrecasal]

Should be ok now 👌

✅ Detached from DOM, so no console warning
✅ Keyboard accessible
✅ Moved padding to button

[kentcdodds]

I believe I've solved it another way that I'm happier with. Can you try it out and let me know if this fixes it.

I also added a logout button to the profile page because otherwise you can't logout if JS doesn't load.

[kentcdodds]

Oh, I missed something: a61c713

[andrecasal]

Seems pretty solid 👌

[kentcdodds]

Awesome!

[kentcdodds]

Thank you for your help