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

feat(nuxt3, bridge): add vue:setup hook #2408

Merged
merged 7 commits into from
Dec 21, 2021
Merged

feat(nuxt3, bridge): add vue:setup hook #2408

merged 7 commits into from
Dec 21, 2021

Conversation

danielroe
Copy link
Member

@danielroe danielroe commented Dec 16, 2021

πŸ”— Linked issue

resolves nuxt/nuxt#12828, resolves nuxt/bridge#222

❓ 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

This PR adds a vue:setup hook that is called within the root setup() function of the app. This also provides a full replacement for onGlobalSetup in the legacy composition API module.

Note: it will not be awaited because we are 'outside' of the root <Suspense>.

πŸ“ Checklist

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

@danielroe danielroe added enhancement New feature or request bridge nuxt3 labels Dec 16, 2021
@danielroe danielroe requested a review from pi0 December 16, 2021 09:58
@danielroe danielroe self-assigned this Dec 16, 2021
@netlify
Copy link

netlify bot commented Dec 16, 2021

βœ”οΈ Deploy Preview for nuxt3-docs ready!

πŸ”¨ Explore the source changes: 3f03934

πŸ” Inspect the deploy log: https://app.netlify.com/sites/nuxt3-docs/deploys/61c1e6f02603a600089110d1

😎 Browse the preview: https://deploy-preview-2408--nuxt3-docs.netlify.app

@atinux
Copy link
Member

atinux commented Dec 16, 2021

I love it!


nuxtApp.nuxt2Context.app.setup = function (...args) {
const result = _originalSetup instanceof Function ? _originalSetup(...args) : {}
nuxtApp.callHook('vue:setup')
Copy link
Member

Choose a reason for hiding this comment

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

Shouldn't we avoid on setup? (if capi's setup does not supports, at least need to handle promise's catch or warn user about async if promise returned)


export default {
setup () {
useNuxtApp().callHook('vue:setup')
Copy link
Member

@pi0 pi0 Dec 17, 2021

Choose a reason for hiding this comment

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

We should await on this call (or chain promise)

Suggested change
useNuxtApp().callHook('vue:setup')
return useNuxtApp().callHook('vue:setup')

Copy link
Member Author

@danielroe danielroe Dec 17, 2021

Choose a reason for hiding this comment

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

If we await this call we will need another root component with suspense above this one. I think it's reasonable to require vue:setup hook to have only syncronous callbacks (but happy to implement that or another solution if we need this to be async).

Copy link
Member

Choose a reason for hiding this comment

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

Your reasoning makes sense but in this case, we need to handle two cases:

  • In server-side, strictly disallow returning promise as it can potentially resolve after rendering lifecycle
  • Handle rejected promises

Maybe as a simple solution simply detect and warn about async usage?

Copy link
Member Author

Choose a reason for hiding this comment

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

Sounds good to me πŸ‘ Will implement.

Copy link
Member Author

Choose a reason for hiding this comment

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

pushed an implementation but will open a PR to add support for sync calling of hooks in hookable (unjs/hookable#34)

@Jamiewarb
Copy link

Super keen for this 🀩

@pi0 pi0 merged commit e674d0f into main Dec 21, 2021
@pi0 pi0 deleted the feat/nuxt3-vue-setup-hook branch December 21, 2021 14:44
@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.

vue:setup hook add support for app.vue or global setup in nuxt bridge
4 participants