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 <Head> fragment detection in Vue 3 adapter #1509

Merged
merged 2 commits into from
May 19, 2023
Merged

Fix <Head> fragment detection in Vue 3 adapter #1509

merged 2 commits into from
May 19, 2023

Conversation

craigrileyuk
Copy link
Contributor

In development builds, a v-for array is received by the renderNodes function as type "Symbol(Fragment)" which is correctly detected. However, in production builds, this same array is received as "Symbol()". This results in the v-for array of Head elements being discarded.

In development builds, a `v-for` array is received by the `renderNodes` function as type "Symbol(Fragment)" which is correctly detected. However, in production builds, this same array is received as "Symbol()". This results in the `v-for` array of Head elements being discarded.
@reinink
Copy link
Member

reinink commented May 19, 2023

@craigrileyuk Thanks for catching and fixing this! 🙏

@reinink reinink changed the title Change Vue3 Head fragment detection method Fix <Head> fragment detection in Vue 3 adapter May 19, 2023
@reinink
Copy link
Member

reinink commented May 19, 2023

@SergkeiM
Copy link

SergkeiM commented May 22, 2023

@reinink @craigrileyuk after update I get this error:

Uncaught (in promise) TypeError: Cannot read properties of undefined (reading 'toString')
    at Proxy.renderTag (head.ts:65:21)
if (node.type.toString() === 'Symbol(Text)') {

@SergkeiM
Copy link

After digging into the issue, it works fine prior vue <= v3.2.47 and breaks in v3.3.0

@craigrileyuk craigrileyuk deleted the vue3-head-render-nodes-fix branch May 22, 2023 23:50
@craigrileyuk
Copy link
Contributor Author

craigrileyuk commented May 22, 2023

Yeah. Vue seems to have made a major change to its vNode type property.

Currently the Head component can’t handle comment tags or other Vue components (e.g. if you tried to use <FacebookMeta /> inside Head.

If no one gets to it before the weekend, I’ll give it a proper refactor.

Until then, a workaround is to avoid anything that can cause a comment (e.g v-if)

@SergkeiM
Copy link

Hi @craigrileyuk I think this is related: vuejs/core#4733, will check it more.

@cjholowatyj
Copy link

cjholowatyj commented May 26, 2023

I'm running [email protected] & [email protected] not sure if I'm having the same issue as what has been reported as fixed...
Screenshot 2023-05-23 at 10 47 46 PM
Screenshot 2023-05-23 at 10 55 11 PM

Or perhaps I'm having a separate issue entirely... When I add <Head> to my Vue components, the node in renderTag(node) is null...

@SergkeiM
Copy link

I guess this #1570 PR by @craigrileyuk fixes this issues

@olieady
Copy link

olieady commented Jun 1, 2023

Having the same issue when trying to get my Breadcrumbs schema rendering in the head. All working before Vue 3.3.

<template>
    <Breadcrumbs
        :items="breadcrumbs"
        class="my-1 px-2"
        link-mode="inertia"
        v-if="breadcrumbs.length > 1"
    />
    <Head>
        {{ breadcrumbsSchema }}
    </Head>
</template>

<script setup>
import { Breadcrumbs } from "../";

import { usePage, Head } from "@inertiajs/vue3";
import { computed } from "vue";

const breadcrumbs = computed(() => usePage().props.breadcrumbs);

const breadcrumbsSchema = computed(() => {
    return usePage().props.breadcrumbsSchema;
});
</script>

@reinink
Copy link
Member

reinink commented Jun 9, 2023

This fixed has been released as part of v1.0.8 👍

@olieady
Copy link

olieady commented Jun 12, 2023

Hey, this doesn't fix the issue when trying to add JSON schema to <head>.

Component which adds JSON schema.

<template>
    <Head>
        {{ breadcrumbsSchema }}
    </Head>
</template>

<script setup>
import { usePage, Head } from '@inertiajs/vue3'
import { computed } from 'vue'

const breadcrumbsSchema = computed(() => usePage().props.breadcrumbsSchema)
</script>

JSON schema passed through via HandlesInertiaRequests middleware.

<script type="application/ld+json">{"@context":"https:\/\/schema.org","@type":"BreadcrumbList","itemListElement":[{"@type":"ListItem","position":1,"item":{"@id":"https:\/\/my-site.com\/","name":"Home","image":null}}]}</script>

This works up with the most recent inertia version. However, Vue needs to be pinned below 3.3.

Perhaps there's a better way to add JSON Schema to the head?

@craigrileyuk
Copy link
Contributor Author

Perhaps there's a better way to add JSON Schema to the head?

Vue doesn't like the use of <script> tags or <style> tags inside the template so this probably shouldn't work.

A far better option is to VueUse's useScriptTag and use it inside your layout component.

@olieady
Copy link

olieady commented Jun 13, 2023

Hey, I did think of this. However this wouldn’t be server side rendered. And since the purpose of using JSON schema here is to help search engines by supplying meta data about breadcrumbs, I feel it should probably be server side rendered.

Also I think VUE only has a problem with script and style tags directly in your template for parsing reasons right? It should be fine to render it like this.

@SergkeiM
Copy link

Agree with @olieady JSON schema is a must in SSR, otherwise the whole point of SSR is usless, as SSR is for SEO in most cases.

I guess the head component should be re-written, becuase lateley applying 1 fix creates 2 bugs.

@reinink FYI.

@iambryansanders
Copy link

Did either of you figure this out? @olieady @craigrileyuk @SergkeiM

Need to have ld+json rendered via SSR.

@olieady
Copy link

olieady commented Apr 24, 2024

This issue was fixed in a later version. Here's what my Breadcrumbs component looks like:

<template>
    <div>
        <Breadcrumbs
            :items="breadcrumbs"
            class="my-1 px-2"
            link-mode="inertia"
            v-if="breadcrumbs.length > 1"
        />
        <Head>
            {{ breadcrumbsSchema }}
        </Head>
    </div>
</template>

<script setup>
import { Breadcrumbs } from '@babestationui/vue'

import { usePage, Head } from '@inertiajs/vue3'
import { computed } from 'vue'

const breadcrumbs = computed(() => usePage().props.breadcrumbs)

const breadcrumbsSchema = computed(() => {
    return usePage().props.breadcrumbsSchema
})
</script>

@SergkeiM
Copy link

Did either of you figure this out? @olieady @craigrileyuk @SergkeiM

Need to have ld+json rendered via SSR.

To be honest I switched to unhead

Discussion

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants