-
-
Notifications
You must be signed in to change notification settings - Fork 6
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
(Re-)introduce fine-grained head settings #73
Comments
Hi @TimJohns I agree with everything you wrote. I think we removed So yes, please feel free to create one or several PRs (re-)introducting the following settings:
Looking forward to it 🙏 @brillout FYI feel free to push back as it would be nice to align |
Actually let me back-pedal a bit. So the reason why we removed
Argument 2. holds as well for So I'm changing my mind here: we'd rather not (re-)introduce the However, I'd love to see the |
Thanks for the conversation @TimJohns @AurelienLourot. Yes, that's the reason I only kept Although I'm realizing that setting a global default description that is being overriden by only some pages is a use case that currently doesn't have a good DX. The issue is that @4350pChris WDYT? 👀 |
I would argue that some crawlers are using client-side navigation, like @crawlee/puppeteer. |
Not to complicate the matter, but one additional consideration worth noting is that while In my case, I provided explicit In any case, I will see if I can create an example as a basis for reasoning about it, and go from there, and we certainly have my specific actual use case to inform some discussion as well. Also, thanks for the work on Vike and vike-vue; I am finding it quite useful and I appreciate it. |
Indeed but I think it is possible already today, inside your implementation of the |
That (generating canonicalUrl programmatically) seems like it would make a particularly relevant example; I will see what I can come up with there. |
Keeping this issue open as @brillout and I had a talk earlier today. We're thinking of (re-)introducing more settings like
Having more fine-grained settings would alleviate that. On top of that, we'd like some settings to be more "semantic", e.g.:
|
Just to give my two cents here - is this really something to implement ourselves? I've been using https://unhead.unjs.io for some time now and really like it, so I wouldn't like to ditch that for vike's implementation. So we should definitely keep it open for users to use their own library / implementation here. On top of that - how would you generally see incorporating that library and focus only on making it work smoothly together? |
I'll put it here because it is related, even if it is a new feature: |
@pdanpdan 👍 Makes sense and it's on the radar as well. |
Another possible use case - I'm currently using I believe either a fine-grained head settings approach similar to If there isn't already a better approach, I am happy to contribute an example that shows the current workaround I'm using, as well as update it with either support for this feature or the one described in #87, as appropriate. |
I'm actually working on this for This is how your use case can be implemented with the upcoming import { Head } from 'vike-react/Head'
function Image({ src, author }: { src: string; author: string }) {
return (
<>
<img src={src} />
<Head>
<script
type="application/ld+json"
dangerouslySetInnerHTML={{
__html: JSON.stringify({
'@context': 'https://schema.org/',
contentUrl: { src },
creator: {
'@type': 'Person',
name: author
}
})
}}
></script>
</Head>
</>
)
} I guess we can do the same for
Yes, contribution welcome! If you're up for it, you could contribute replicating the |
Vue has the v-html directive and Render Functions that have the capability of injecting html. Here's what I am currently doing (directly within +Head.vue for a subset of pages, without the Security Note: I have not thoroughly analyzed this code for the possibility of injection attacks using carefully crafted <template class="h-100">
<link rel="apple-touch-icon" sizes="120x120" href="/favicons/apple-touch-icon.png">
<link rel="icon" type="image/png" sizes="32x32" href="/favicons/favicon-32x32.png">
<link rel="icon" type="image/png" sizes="16x16" href="/favicons/favicon-16x16.png">
<title>{{title}}</title>
<meta v-if="description" name="description" :content="description">
<link v-if="canonicalUrl" rel="canonical" :href="canonicalUrl">
<link rel="manifest" href="/favicons/site.webmanifest">
<link rel="shortcut icon" type="image/x-icon" href="/favicons/favicon.ico">
<meta name="msapplication-TileColor" content="#da532c">
<meta name="msapplication-config" content="/favicons/browserconfig.xml">
<meta name="theme-color" content="#ffffff">
<meta name="viewport" content="width=device-width, initial-scale=1.0" />
<template v-for="photo in photos">
<ImageObjectJSONLD :contentUrl="photo.imgSrc" :copyrightNotice="photo.copyrightNotice"/>
</template>
<link rel="preconnect" href="https://identitytoolkit.googleapis.com">
<link rel="preconnect" href="https://maps.gstatic.com">
<link rel="preconnect" href="https://accounts.google.com">
<link rel="preconnect" href="https://maps.googleapis.com">
<link rel="preconnect" href="https://lh3.googleusercontent.com">
<link rel="preconnect" href="https://api.epicroadtripplanner.com">
</template>
<script lang="ts" setup>
import { computed, h } from "vue";
import { useData } from 'vike-vue/useData';
import { Data } from './helpers';
const { title, description, canonicalUrl, stopTemplate } = useData<Data>();
const photos = computed(() => (stopTemplate?.photos || []))
type ImageObject = {
'@context': 'https://schema.org/',
'@type': 'ImageObject',
contentUrl: string,
copyrightNotice?: string
}
const ImageObjectJSONLD = (props: {contentUrl: string, copyrightNotice?: string}) => {
const imageObject: ImageObject =
{
'@context': 'https://schema.org/',
'@type': 'ImageObject',
contentUrl: props.contentUrl
};
if (props.copyrightNotice) imageObject.copyrightNotice = props.copyrightNotice;
return h('script', { type: 'application/ld+json', innerHTML: JSON.stringify(imageObject, null, 2)});
}
</script> |
I did some experimentation earlier this week and really want to double-down on what @4350pChris said here about ensuring things like https://unhead.unjs.io integrate smoothly.
Chris, I'm not sure if you got that working cleanly, but I tried some fairly complex workarounds because https://unhead.unjs.io's state is reset between the rendering of of the page content and the head content, and ultimately took a step back to try again another day. This may just be learning curve on my part, but I figured I'd share some details in case it's relevant to this issue thread. Specifically, it seems like the natural place to install unhead is the onCreateApp hook, but that didn't work for me out-of-the-box because I'm in fact still working to understand better how all of this interacts, particularly in my hybrid configuration (vs. a pure SPA or SSR app), but I thought I would post that observation in case it's helpful or on anyone else's mind on this topic as well. I've currently gone back to a tightly-coupled custom +Head.vue that "knows" what components I might have used in my +Page.vue based on the data. That is, I currently have a little bit of duplicated code in +Head.vue and +Page.vue that looks through the data returned by |
Yes, having a unhead integration is very much on the radar. Can you publish a working example of using Vike and unhead without vike-vue (Vike boilerplate with manual Vue integration: |
EDIT 2: I removed the prior diff entirely because it demonstrated a common XSS vulnerability, and I don't want to risk developers copy-and-pasting that code or AI agents getting trained on it. The diff below is in comparison to the Vike boilerplate that @brillout mentioned above (the output of Cheers, hope that helps, let me know if there is more I can do here, and thanks as always for all the work here! -Tim diff --git a/package.json b/package.json
index af4227a..83afd2c 100644
--- a/package.json
+++ b/package.json
@@ -11,6 +11,8 @@
"@types/compression": "^1.7.5",
"@types/express": "^4.17.21",
"@types/node": "^20.10.4",
+ "@unhead/ssr": "1.9.16",
+ "@unhead/vue": "1.9.16",
"@vitejs/plugin-vue": "^5.0.0",
"@vue/compiler-sfc": "^3.3.10",
"@vue/server-renderer": "^3.3.10",
diff --git a/pages/star-wars/@id/+Page.vue b/pages/star-wars/@id/+Page.vue
index 37612fe..fa79495 100644
--- a/pages/star-wars/@id/+Page.vue
+++ b/pages/star-wars/@id/+Page.vue
@@ -10,5 +10,22 @@
<script lang="ts" setup>
import type { Data } from './+data'
import { useData } from '../../../renderer/useData'
+import { useServerSeoMeta } from '@unhead/vue';
+
const data = useData<Data>()
+
+/**
+ *
+ * Since we're not sure whether fetched data content has been sanitized, it is critically
+ * important to use useHeadSafe(), useServerHeadSafe(), userSeoMeta(), or useSeoServerSeoMeta() here.
+ * If you are passing hard-coded literal values, or values that are otherwise guaranteed to have
+ * been sanitized, you can use useHead().
+ *
+ * @see https://unhead.unjs.io/usage/composables/use-head#xss-safety
+ *
+ */
+useServerSeoMeta({
+ title: data.value.movie.title,
+ description: data.value.movie.title,
+})
</script>
diff --git a/renderer/+onRenderHtml.ts b/renderer/+onRenderHtml.ts
index 68ddd23..c18a719 100644
--- a/renderer/+onRenderHtml.ts
+++ b/renderer/+onRenderHtml.ts
@@ -5,9 +5,9 @@ import { renderToString as renderToString_ } from '@vue/server-renderer'
import type { App } from 'vue'
import { escapeInject, dangerouslySkipEscape } from 'vike/server'
import { createVueApp } from './createVueApp'
-import logoUrl from './logo.svg'
import type { OnRenderHtmlAsync } from 'vike/types'
-import { getPageTitle } from './getPageTitle'
+import { renderSSRHead } from '@unhead/ssr'
+import { getActiveHead } from 'unhead'
const onRenderHtml: OnRenderHtmlAsync = async (pageContext): ReturnType<OnRenderHtmlAsync> => {
// This onRenderHtml() hook only supports SSR, see https://vike.dev/render-modes for how to modify
@@ -15,24 +15,23 @@ const onRenderHtml: OnRenderHtmlAsync = async (pageContext): ReturnType<OnRender
if (!pageContext.Page) throw new Error('My render() hook expects pageContext.Page to be defined')
const app = createVueApp(pageContext)
-
+
+ const head = getActiveHead();
+ if (!head) throw new Error('No active unhead head. Ensure you have installed unhead by calling createHead and app.use(head).')
+
const appHtml = await renderToString(app)
- // https://vike.dev/head
- const title = getPageTitle(pageContext)
- const desc = pageContext.data?.description || pageContext.config.description || 'Demo of using Vike'
+ const { headTags, bodyTags, bodyTagsOpen, htmlAttrs, bodyAttrs } = await renderSSRHead(head)
const documentHtml = escapeInject`<!DOCTYPE html>
- <html lang="en">
+ <html ${dangerouslySkipEscape(htmlAttrs)}>
<head>
- <meta charset="UTF-8" />
- <link rel="icon" href="${logoUrl}" />
- <meta name="viewport" content="width=device-width, initial-scale=1.0" />
- <meta name="description" content="${desc}" />
- <title>${title}</title>
+ ${dangerouslySkipEscape(headTags)}
</head>
- <body>
+ <body ${dangerouslySkipEscape(bodyAttrs)}>
+ ${dangerouslySkipEscape(bodyTagsOpen)}
<div id="app">${dangerouslySkipEscape(appHtml)}</div>
+ ${dangerouslySkipEscape(bodyTags)}
</body>
</html>`
diff --git a/renderer/Layout.vue b/renderer/Layout.vue
index 1a51e37..4c483c6 100644
--- a/renderer/Layout.vue
+++ b/renderer/Layout.vue
@@ -15,6 +15,71 @@
<script lang="ts" setup>
import Link from './Link.vue'
import './css/index.css'
+
+import { useServerHead, useServerHeadSafe } from '@unhead/vue'
+import { getPageTitle } from './getPageTitle'
+import logoUrl from './logo.svg'
+import { usePageContext } from './usePageContext'
+import { unref } from 'vue'
+
+/**
+ * title and desc are now site-wide defaults, and are overwritten by any page-specific or
+ * layout-specific updates to useHead({}).
+ *
+ * @see pages/star-wars/@id/+Page.vue
+ *
+ * Note that the order we render in is important. Only the most recent
+ * <title> and <meta name="description"> will be used, so we intentionally
+ * do this head.push prior to calling renderToString().
+ *
+ * @see https://unhead.unjs.io/usage/guides/handling-duplicates#deduping-logic
+ *
+ */
+const pageContext = unref(usePageContext());
+
+const title = getPageTitle(pageContext)
+const desc = pageContext.data?.description || pageContext.config.description || 'Demo of using Vike';
+
+/**
+ * useServerHead is used here for the values that don't need to be santized.
+ *
+ * For readability, these values could probably be combined with useServerHeadSafe,
+ * but currently the 'charset' meta tag is not supported by useServerHeadSafe.
+ * That's probably a (very minor) type bug, @see https:/unjs/unhead/issues/372
+ */
+useServerHead({
+ htmlAttrs: {
+ lang: 'en'
+ },
+ link: [
+ {
+ href: logoUrl,
+ rel: "icon"
+ }
+ ],
+ meta: [
+ { charset: 'utf-8' },
+ { name: 'viewport', content: 'width=device-width, initial-scale=1.0' },
+ ],
+})
+
+/**
+ *
+ * Since we're not sure whether fetched data content has been sanitized, it is critically
+ * important to use useHeadSafe(), useServerHeadSafe(), userSeoMeta(), or useSeoServerSeoMeta() here.
+ * If you are passing hard-coded literal values, or values that are otherwise guaranteed to have
+ * been sanitized, you can use useHead() or useServerHead().
+ *
+ * @see https://unhead.unjs.io/usage/composables/use-head#xss-safety
+ *
+ */
+useServerHeadSafe({
+ meta: [
+ { name: 'description', content: desc }
+ ],
+ title,
+})
+
</script>
<style>
diff --git a/renderer/createVueApp.ts b/renderer/createVueApp.ts
index c8a73b1..fd620cf 100644
--- a/renderer/createVueApp.ts
+++ b/renderer/createVueApp.ts
@@ -6,6 +6,7 @@ import { setData } from './useData'
import Layout from './Layout.vue'
import type { PageContext } from 'vike/types'
import { objectAssign } from './utils'
+import { createHead } from '@unhead/vue'
function createVueApp(pageContext: PageContext) {
const pageContextRef = shallowRef(pageContext)
@@ -17,6 +18,10 @@ function createVueApp(pageContext: PageContext) {
setPageContext(app, pageContextRef)
setData(app, dataRef)
+ const head = createHead()
+ app.use(head)
+
+
// app.changePage() is called upon navigation, see +onRenderClient.ts
objectAssign(app, {
changePage: (pageContext: PageContext) => {
|
@TimJohns Thank you! Let me finish my current priorities and I'll then take a stab at it (sometime around next week). In the meantime, feel free to try to create |
Done. Closing, but let me know if you believe something's missing. We are looking forward to gather feedback about the newly introduced head tag management improvements. |
As for |
I'm deprioritizing this. I created #183 to track progress. Contribution welcome.
I'd be curious to know this. This would also increase the priority to work on #183 . |
Thanks - I've updated to the latest and everything is working great for me, and I'll try unhead again with vikeVue when I get back. I don't know that I'd recommend prioritizing a specific integration, but there may be a change to the way the create app hooks are called in vikeVue that would allow unhead to be used if someone does have a preference. As for why someone might have a preference to use unhead, one reason that was relevant to me is that it has a set of handy schema.org integrations. I only needed image and rolled my own component, so I am definitely not blocked by this. I suspect others may just prefer unhead for familiarity coming from other ecosystems and frameworks, but unhead was new to me, so that's hypothetical on my part. |
Indeed, that's neat. I'm thinking maybe we create a Vike extension
Yes it's an argument, although long-term it isn't as big of an argument if we manage to pull off a better DX than vue-unhead. |
@4350pChris Indeed, for Also, for simple use cases, I think (but I'm happy to be shown wrong) that our DX is simpler. For advanced use cases, let's see if we can manage a better DX than |
In addition to the title, favicon, and lang, I would find it handy to be able to specify a canonical URL in the page config.
This could be beneficial for other SSG sites optimized for search engines (see How to specify a canonical with rel="canonical" and other methods.
I currently implement this in my
HeadDefault.vue
as below, but it seems to closely match the pattern in the latest onRender{Client|Html} code, and may have broader appeal beyond my specific use case, so if you think it would be beneficial, please let me know and I'll submit a PR. Curious also why it appears that description was removed in the latest code, as I also configure description on page-by-page basis.e.g.
Replaces my specific implementation:
The text was updated successfully, but these errors were encountered: