Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(nuxt): await nuxt ready state before refreshNuxtData #21008

Merged
merged 1 commit into from May 22, 2023

Conversation

lihbr
Copy link
Member

@lihbr lihbr commented May 22, 2023

πŸ”— Linked issue

n/a, internal mention: https://discord.com/channels/473401852243869706/1065634249174159420/1108033633870155836

❓ Type of change

  • πŸ“– Documentation (updates to the documentation, readme or JSdoc annotations)
  • 🐞 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)
  • 🧹 Chore (updates to the build process or auxiliary tools and libraries)
  • ⚠️ Breaking change (fix or feature that would cause existing functionality to change)

πŸ“š Description

This PRs aims at improving the DX of Nuxt utils by enforcing relevant utils to await Nuxt ready state before performing their actions. Basically, it ensures and simplifies the following kind of workflows:

// Before
- refreshNuxtData()                    // Inconsistent
- onNuxtReady(() => refreshNuxtData()) // Consistent

// After
+ refreshNuxtData()                    // Consistent

Indeed, some utils can have unexpected behavior when launched prior and/or during Nuxt initialization process. For example, refreshNuxtData(), when called from a plugin during Nuxt initialization process cannot refresh all Nuxt data properly because Nuxt data lifecycles are still in the process of being resolved. Awaiting for Nuxt to be initialized is almost mandatory if we want this util to work consistently.

Considered utils

So far, really, this PR only applies to refreshNuxtData(), I tried to consider utils alike.

  • ❌ abortNavigation, can only be used inside route middlewares
  • ❌ clearError, can't think of a scenario where this would make sense
  • ❔ clearNuxtData, if there's no data to clear, there's no data to clear(?)
  • ❔ navigateTo, maybe it applies to this one(?) not sure

That being said, if you see other utils where waiting for onNuxtReady seems mandatory, please voice it! Happy to update the PR.

Doing nothing

I've been torn a bit on whether or not we should bake in the onNuxtReady call. Basically: should we consider it a DX feature (having both separated) or a DX bug (what this PR fixes)

I'm leaning towards a DX bug hence this PR, but happy to reconsider it and close the PR if we settle on a DX feature (forcing explicit onNuxtReady calls where applicable)

Let me know if anything~

πŸ“ Checklist

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

@danielroe danielroe merged commit a672cd7 into main May 22, 2023
19 checks passed
@danielroe danielroe changed the title fix(nuxt): await nuxt before calling dependant utils fix(nuxt): await nuxt ready state before refreshNuxtData May 22, 2023
@danielroe danielroe deleted the fix/await-nuxt-before-calling-dependant-utils branch May 22, 2023 23:09
This was referenced May 22, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants