framework icon indicating copy to clipboard operation
framework copied to clipboard

feat(nuxt): share `asyncData` between calls

Open OhB00 opened this issue 3 years ago • 2 comments

Had a look at https://github.com/nuxt/framework/issues/4758 and https://github.com/nuxt/framework/pull/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

#4758

❓ Type of change

  • [x] 📖 Documentation (updates to the documentation or readme)
  • [x] 🐞 Bug fix (a non-breaking change that fixes an issue)
  • [x] 👌 Enhancement (improving an existing functionality like performance)
  • [x] ✨ 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

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

OhB00 avatar Jul 05 '22 19:07 OhB00

Deploy Preview for nuxt3-docs canceled.

Name Link
Latest commit 3b1953282b2dd1c754f5d5a2dd7e92ea5bb5a2c3
Latest deploy log https://app.netlify.com/sites/nuxt3-docs/deploys/62c5b2713793c200081ec037

netlify[bot] avatar Jul 05 '22 19:07 netlify[bot]

3rd times a charm

OhB00 avatar Jul 06 '22 06:07 OhB00

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 nuxt/framework#7056
  3. Iterating over with some refactors.
  4. (main fix) Done via nuxt/framework#7055 -- probably needs iteration to handle edge cases like ones @danielroe mentioned in https://github.com/nuxt/framework/pull/7055#discussion_r958315070
  5. (same as 4)
  6. Moved to discussion nuxt/nuxt.js#14746
  7. Merged via nuxt/framework#7055
  8. (same as 3)

Thanks again!

pi0 avatar Aug 30 '22 11:08 pi0