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

feat(nuxt): allow useAppConfig with specific key#6776

Closed
pi0 wants to merge 4 commits intomainfrom
feat/app-config-scope
Closed

feat(nuxt): allow useAppConfig with specific key#6776
pi0 wants to merge 4 commits intomainfrom
feat/app-config-scope

Conversation

@pi0
Copy link
Copy Markdown
Member

@pi0 pi0 commented Aug 19, 2022

🔗 Linked issue

❓ 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

Context: #6333

useAppConfig returns a reactive object but it is super easy to loose reactivity when restructuring it's interface like const theme = useAppConfig().theme is not reactive anymore to accept HMR.

This PR adds a simple addition using computed for one top-level key.

Todo:

  • Type support
  • Reactivity without .value

📝 Checklist

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

@netlify
Copy link
Copy Markdown

netlify Bot commented Aug 19, 2022

Deploy Preview for nuxt3-docs ready!

Name Link
🔨 Latest commit 6498540
🔍 Latest deploy log https://app.netlify.com/sites/nuxt3-docs/deploys/62ff5c801971da00088235a9
😎 Deploy Preview https://deploy-preview-6776--nuxt3-docs.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site settings.

@pi0
Copy link
Copy Markdown
Member Author

pi0 commented Aug 19, 2022

/cc @antfu Do you know any better way to resolve it than computed?

@atinux
Copy link
Copy Markdown
Member

atinux commented Aug 19, 2022

One tricky way here is that when using a key, we will need to use .value right?

@antfu
Copy link
Copy Markdown
Member

antfu commented Aug 19, 2022

Do you mean toRef(nuxtApp._appConfig, key)?

@pi0
Copy link
Copy Markdown
Member Author

pi0 commented Aug 19, 2022

One tricky way here is that when using a key, we will need to use .value right?

Indeed both computed and toRef make access harder with .value...

Comment thread docs/content/2.guide/2.features/10.app-config.md Outdated
@pi0
Copy link
Copy Markdown
Member Author

pi0 commented Aug 19, 2022

Chaining reactive(toRef( seems to work pefectly fine and without .value (but catch is, users should obviously do this for object keys)

@pi0
Copy link
Copy Markdown
Member Author

pi0 commented Aug 19, 2022

TODO: Type support

nuxtApp._appConfig = reactive(__appConfig) as AppConfig
}
if (key) {
return reactive(toRef(nuxtApp._appConfig, key))
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.

I suspect this would work. Wrapping a ref with reactive() will return as-is.

I am not very sure about this feature. Kinda babysitting TBH. It's basically the limitation of reactive objects, same as you can't restructure Components' props - stating useAppConfig returns a reactive object should be enough.

I feel the best solution to improve this is the Reactivity Transform and the later vuejs/core#5856

@pi0 pi0 marked this pull request as draft August 19, 2022 20:33
@pi0
Copy link
Copy Markdown
Member Author

pi0 commented Aug 22, 2022

With fix in #6788, HMR issue should be solved. Let's revise this idea later.

@pi0 pi0 closed this Aug 22, 2022
@pi0 pi0 deleted the feat/app-config-scope branch August 22, 2022 09:15
@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.

4 participants