Skip to content

Conversation

@OrbisK
Copy link
Member

@OrbisK OrbisK commented Aug 10, 2025

🔗 Linked issue

📚 Description

Based on @huang-julien #32531 - swapping exposed AbortController for exposed AbortSignal + adding timeout to asyncData.

Opening this as PR to better discuss some of the POC changes.

@bolt-new-by-stackblitz
Copy link

Review PR in StackBlitz Codeflow Run & review this pull request in StackBlitz Codeflow.

Copy link
Member

@huang-julien huang-julien left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm

@huang-julien
Copy link
Member

huang-julien commented Aug 10, 2025

I think we still need to cancel previous promise (if still running) with the last signal 👀

@OrbisK
Copy link
Member Author

OrbisK commented Aug 10, 2025

I think we still need to cancel previous promise (if still running) with the last signal 👀

I will add this.

Maybe you could share your thoughts on how we should propagate this error:

https://github.com/OrbisK/nuxt/blob/ea9dfb1ec9e35c6ae5d137e66abf220ba04105b7/packages/nuxt/src/app/composables/asyncData.ts#L709

(+ this should be mergedSignal.reason not abortController.signal.reason - fixing this)

@OrbisK
Copy link
Member Author

OrbisK commented Aug 10, 2025

I think we still need to cancel previous promise (if still running) with the last signal 👀

Isnt this done here?

https://github.com/OrbisK/nuxt/blob/ea9dfb1ec9e35c6ae5d137e66abf220ba04105b7/packages/nuxt/src/app/composables/asyncData.ts#L682-L684

@OrbisK
Copy link
Member Author

OrbisK commented Aug 10, 2025

With the current implementation, the status is error while the new promise is pending. I will check if we can prioritise the pending status over the aborted error.

@OrbisK
Copy link
Member Author

OrbisK commented Aug 10, 2025

With the current implementation, the status is error while the new promise is pending. I will check if we can prioritise the pending status over the aborted error.

I have not found a clean solution yet. I will check again tomorrow. 😄

@OrbisK
Copy link
Member Author

OrbisK commented Aug 11, 2025

execute()
// status.value -> pending
execute()
// status.value -> pending
execute()
// status.value -> pending
// status.value -> success

execute()
// status.value -> pending
clear()
// status.value -> idle + requets aborted

const ac = new AbortController()
execute({signal: ac.signal})
// status.value -> pending
ac.abort(new WhatEverError())
// status.value -> error

execute({timeout: 10})
// status.value -> pending
// status.value -> error

@OrbisK
Copy link
Member Author

OrbisK commented Aug 11, 2025

ToDos:

@OrbisK OrbisK marked this pull request as ready for review August 12, 2025 07:51
@OrbisK OrbisK requested a review from danielroe as a code owner August 12, 2025 07:51
@danielroe danielroe requested a review from huang-julien August 12, 2025 09:41
@danielroe danielroe merged commit 2bc1827 into nuxt:fix/32530 Aug 12, 2025
6 of 7 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants