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

feat(nuxt): share asyncData between calls - #5738

Closed
OhB00 wants to merge 9 commits into
nuxt:mainfrom
OhB00:main
Closed

feat(nuxt): share asyncData between calls#5738
OhB00 wants to merge 9 commits into
nuxt:mainfrom
OhB00:main

Conversation

@OhB00

@OhB00OhB00 commented Jul 5, 2022

Copy link
Copy Markdown
Contributor

Had a look at nuxt/nuxt#13910 and #5078 and made some changes to improve readability, and fix the race condition mentioned in the issue.

I have also implemented the change suggested in the other pull request to share asyncData between calls.

🔗 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)

📚 Description

  1. I have refactored useAsyncData to use more async await, this (hopefully) makes it much more readable + manageable.
  2. I have eliminated some dead code from various code paths, _nuxtOnBeforeMountCbs are no longer touched at all server side, data caching is handled top level rather than within refresh,
  3. refresh was simplified with the concept of 'tagging along', if a promise is found for a key, use that instead of calling the handler again.
  4. asyncData is sync'd between calls, for example, if you had a useUserData composable, that displays user data in two separate components, calling refresh will change both.
  5. As a result of the previous point various bugs related to no sync are resolved (#4758 and likely others)
  6. Added a force option to refresh, this seems to make more sense then what was provided in the documentation originally, this may be a breaking change.
  7. Added some tests for useAsyncData, as it's fairly easy to break.
  8. Refreshing on the server side updates the cache, this seemed logical.

📝 Checklist

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

@netlify

netlifyBot commented Jul 5, 2022

Copy link
Copy Markdown

Deploy Preview for nuxt3-docs canceled.

NameLink
🔨 Latest commit3b19532
🔍 Latest deploy loghttps://app.netlify.com/sites/nuxt3-docs/deploys/62c5b2713793c200081ec037

@OhB00OhB00 changed the title Share asyncData between requestsShare asyncData between callsJul 5, 2022
@OhB00OhB00 changed the title Share asyncData between callsfeat(nuxt): share asyncData between callsJul 5, 2022
@danielroe
danielroe self-requested a review July 5, 2022 20:08
@OhB00

OhB00 commented Jul 6, 2022

Copy link
Copy Markdown
ContributorAuthor

3rd times a charm

@antfuantfu left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Please revert the formatting changes before we could proceed reviewing.

@OhB00OhB00 left a comment

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

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

Reverted formatting changes

@pi0

pi0 commented Aug 30, 2022

Copy link
Copy Markdown
Member

Thanks for your nice PR @OhB00 and sorry it took long to proceed.

I have splitted your changes into smaller PR and issues (next times, please consider doing this. this way we can quickly iterate over smaller changes)

  1. I prefer to avoid using await in this particular utility because we only have current component/nuxt context available until first await and this utility returns a hybrid promise interface
  2. Merged via perf(nuxt): tree-shake asyncData client logic from server #7056
  3. Iterating over with some refactors.
  4. (main fix) Done via fix(nuxt): use shared state for asyncData #7055 -- probably needs iteration to handle edge cases like ones @danielroe mentioned in fix(nuxt): use shared state for asyncData #7055 (comment)
  5. (same as 4)
  6. Moved to discussion Force refresh for useAsyncData and useFetch nuxt#14746
  7. Merged via fix(nuxt): use shared state for asyncData #7055
  8. (same as 3)

Thanks again!

Sign up for freeto subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

3.xbugSomething isn't workingenhancementNew feature or request❗ p4-importantPriority 4: bugs that violate documented behavior, or significantly impact perf

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants

@OhB00@pi0@antfu@danielroe