Skip to content
This repository was archived by the owner on Apr 6, 2023. It is now read-only.

fix(nuxt): reuse asyncData promise with existing key#5078

Closed
cherrymarker wants to merge 5 commits intonuxt:mainfrom
cherrymarker:main
Closed

fix(nuxt): reuse asyncData promise with existing key#5078
cherrymarker wants to merge 5 commits intonuxt:mainfrom
cherrymarker:main

Conversation

@cherrymarker
Copy link
Copy Markdown

@cherrymarker cherrymarker commented May 20, 2022

🔗 Linked issue

nuxt/nuxt#13910

❓ Type of change

  • 📖 Documentation (updates to the documentation or readme)
  • 🐞 Bug fix (a non-breaking change that fixes an issue)
  • 👌 Enhancement (improving an existing functionality like performance)
  • ✨ New feature (a non-breaking change that adds functionality)
  • ⚠️ Breaking change (fix or feature that would cause existing functionality to change)

I suppose this is technically a breaking change if applications are dependant on the original broken behaviour.

📚 Description

I have modified useAsyncData to return the correct data (the original asyncdata from the first call) associated with a request in the case that another call is made to useAsyncData while the original promise is still pending.

This change allows you to call useAsyncData on a single key, within multiple components, and work with the data on the server side without subsequent calls resolving to pending and null.

Resolves nuxt/nuxt#13910 for now, more robust solution required in future.

📝 Checklist

  • I have linked an issue or discussion.
  • I have updated the documentation accordingly.

@pi0 pi0 changed the title fix: Temporary fix for https://github.com/nuxt/framework/issues/4758 fix(nuxt): reuse asyncData promise with existing key May 20, 2022
Comment thread packages/nuxt/src/app/composables/asyncData.ts Outdated
@cherrymarker
Copy link
Copy Markdown
Author

On further inspection this fix is a bit wonky, bear with

Comment thread packages/nuxt/src/app/composables/asyncData.ts Outdated
Comment thread packages/nuxt/src/app/composables/asyncData.ts Outdated
@danielroe
Copy link
Copy Markdown
Member

@cherrymarker Oops, sorry, missed your comment. Will hold any further comments for now.

@cherrymarker
Copy link
Copy Markdown
Author

While this fix "sort of" works, I have realised due to this line there may be problems

Object.assign(asyncDataPromise, asyncData)

The original asyncdata is still passed along.

A better solution would be to store the asyncData, and fetch it later IF the promise has already been found.

@cherrymarker
Copy link
Copy Markdown
Author

cherrymarker commented May 20, 2022

Updated to a better solution that works both client and server side.

Wasn't sure about the type definition in nuxt.ts but gave it a go.

@netlify
Copy link
Copy Markdown

netlify Bot commented May 23, 2022

Deploy Preview for nuxt3-docs canceled.

Name Link
🔨 Latest commit 37c69f2
🔍 Latest deploy log https://app.netlify.com/sites/nuxt3-docs/deploys/628bcbc9a6d2160008401d7d

@danielroe
Copy link
Copy Markdown
Member

I wonder if this would be an opportunity to share asyncData across calls, which would also address a related issue where pending can get out of sync. Thoughts?

@cherrymarker
Copy link
Copy Markdown
Author

I think that the design of useAsyncData is a bit messy and could be simplified by just using async methods.

Instead of data and error being stored on nuxt.payload it would be easier to store all of asyncData on payload instead and retrieve it on a matching key.

I think instead of having pending as a bool, a Promise that resolves when the handler is finished is far more useful and easier to use.

@atinux
Copy link
Copy Markdown
Member

atinux commented May 25, 2022

I think that the design of useAsyncData is a bit messy and could be simplified by just using async methods.

What do you have in mind?

Instead of data and error being stored on nuxt.payload it would be easier to store all of asyncData on payload instead and retrieve it on a matching key.

This makes sense since it will keep the scope of each one (error shall be used for global error handling)

I think instead of having pending as a bool, a Promise that resolves when the handler is finished is far more useful and easier to use.

Not 100% sure about this since most people use v-if="pending" for state handling.

@pi0
Copy link
Copy Markdown
Member

pi0 commented Aug 30, 2022

Hi @cherrymarker thanks for PR and sorry it took long to review.

We are iterating over this issue to fix related issues. Please check #5738 (comment) for a summary of changes and plans opened so far.

Feel free to try changes and share feedbacks via issue for any suggestions.

@pi0 pi0 closed this Aug 30, 2022
@danielroe danielroe added the 3.x label Jan 19, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

share useAsyncData state

4 participants