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

feat(nuxt, schema): official @vueuse/head v1 support #8975

Merged
merged 21 commits into from Nov 15, 2022
Merged

feat(nuxt, schema): official @vueuse/head v1 support #8975

merged 21 commits into from Nov 15, 2022

Conversation

harlan-zw
Copy link
Collaborator

@harlan-zw harlan-zw commented Nov 14, 2022

πŸ”— Linked issue

Remake of PR #8908 (comment)

❓ 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

New Nuxt v3 projects are already using v1 @vueuse/head outside of this PR. There are no open issues for the v1 release currently.

This PR aims to:

πŸ“ Checklist

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

Basic documentation, will need continued work over the coming weeks. I've kept the two new composable undocumented for the initial release, shouldn't be an issue for now?

@codesandbox
Copy link

codesandbox bot commented Nov 14, 2022

CodeSandbox logoCodeSandbox logoΒ  Open in CodeSandbox Web Editor | VS Code | VS Code Insiders

@netlify
Copy link

netlify bot commented Nov 14, 2022

❌ Deploy Preview for nuxt3-docs failed.

Name Link
πŸ”¨ Latest commit 18aab25
πŸ” Latest deploy log https://app.netlify.com/sites/nuxt3-docs/deploys/6373a8067d418b0009be40ee

@@ -33,6 +33,7 @@ describe.skipIf(isWindows)('minimal nuxt application', () => {
"_nuxt/error-404.js",
"_nuxt/error-500.js",
"_nuxt/error-component.js",
"_nuxt/index.js",
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

No idea why this is being generated

@harlan-zw harlan-zw marked this pull request as ready for review November 14, 2022 12:26
@harlan-zw
Copy link
Collaborator Author

Ready for review, I'll write a couple of tests soon

@pi0
Copy link
Member

pi0 commented Nov 14, 2022

Thanks so much for working on this @harlan-zw πŸ’― I will have a local look soon but overall unhead migration seems already in good direction.

Since we are so close to latest RC and v3 release, i think it is good idea to consider including this change and release fixes in upstream if any. Only, i would appreciate if we can minimize new feature additions to reduce chances of breaking changes for end-users.

@harlan-zw
Copy link
Collaborator Author

harlan-zw commented Nov 14, 2022

Thanks so much for working on this @harlan-zw 100 I will have a local look soon but overall unhead migration seems already in good direction.

Since we are so close to latest RC and v3 release, i think it is good idea to consider including this change and release fixes in upstream if any. Only, i would appreciate if we can minimize new feature additions to reduce chances of breaking changes for end-users.

Good idea, will remove anything non-essential. I can release a module to opt-in to these new features

@pi0 pi0 mentioned this pull request Nov 15, 2022
@pi0
Copy link
Member

pi0 commented Nov 15, 2022

Overall looks good to me! Let's try changes on Nuxt edge.

@pi0 pi0 changed the title feat(head): official @vueuse/head v1 support feat(nuxt, schema): official @vueuse/head v1 support Nov 15, 2022
@nathanchase
Copy link
Contributor

It doesn't seem that useHead updates on computed properties.

Example:

const pageTitle = computed(() =>
  route.params.query
    ? `πŸ” Search results for "${route.params.query}"`
    : 'πŸ” Search',
);

useHead({
  title: pageTitle.value,
});

Even if pageTitle correctly includes the variable, the page title never updates.

@tylerforesthauser
Copy link

I think you have to use just the computed ref. So use pageTitle not pageTitle.value.

@nathanchase
Copy link
Contributor

I think you have to use just the computed ref. So use pageTitle not pageTitle.value.

Ah, of course. Thank you.

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.

None yet

5 participants