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: duplicate meta tags #2164

Merged
merged 8 commits into from
Mar 23, 2020
Merged
Show file tree
Hide file tree
Changes from 4 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 0 additions & 1 deletion packages/@vuepress/core/lib/client/index.ssr.html
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@
<meta charset="utf-8">
<meta name="viewport" content="width=device-width,initial-scale=1">
<title>{{ title }}</title>
<meta name="description" content="{{ description }}">
<meta name="generator" content="VuePress {{ version }}">
{{{ userHeadTags }}}
{{{ pageMeta }}}
Expand Down
97 changes: 75 additions & 22 deletions packages/@vuepress/core/lib/client/root-mixins/updateMeta.js
Original file line number Diff line number Diff line change
@@ -1,38 +1,53 @@
import unionBy from 'lodash/unionBy'

export default {
// created will be called on both client and ssr
created () {
if (this.$ssrContext) {
const siteMetaTags = this.$site.headTags
.filter(item => item[0] === 'meta')
.map(item => item[1])
haoranpb marked this conversation as resolved.
Show resolved Hide resolved

const meta = this.getMergedMetaTags(siteMetaTags)
haoranpb marked this conversation as resolved.
Show resolved Hide resolved

this.$ssrContext.title = this.$title
this.$ssrContext.lang = this.$lang
this.$ssrContext.description = this.$page.description || this.$description
this.$ssrContext.pageMeta = renderPageMeta(meta)
haoranpb marked this conversation as resolved.
Show resolved Hide resolved
}
},

// Other life cycles will only be called at client
mounted () {
// init currentMetaTags from DOM
this.currentMetaTags = [...document.querySelectorAll('meta')]

// indirectly init siteMetaTags from DOM
this.siteMetaTags = this.currentMetaTags.map(element => {
const siteMeta = {}
for (const attribute of element.attributes) {
siteMeta[attribute.name] = attribute.value
}
return siteMeta
})
haoranpb marked this conversation as resolved.
Show resolved Hide resolved

// update title / meta tags
this.currentMetaTags = new Set()
this.updateMeta()
},

methods: {
updateMeta () {
document.title = this.$title
document.documentElement.lang = this.$lang
const userMeta = this.$page.frontmatter.meta || []
const meta = userMeta.slice(0)
const useGlobalDescription = userMeta.filter(m => m.name === 'description').length === 0

// #665 Avoid duplicate description meta at runtime.
if (useGlobalDescription) {
meta.push({ name: 'description', content: this.$description })
}

// Including description meta coming from SSR.
const descriptionMetas = document.querySelectorAll('meta[name="description"]')
if (descriptionMetas.length) {
descriptionMetas.forEach(m => this.currentMetaTags.add(m))
}
const newMetaTags = this.getMergedMetaTags(this.siteMetaTags)
this.currentMetaTags = updateMetaTags(newMetaTags, this.currentMetaTags)
},

this.currentMetaTags = new Set(updateMetaTags(meta, this.currentMetaTags))
getMergedMetaTags (siteMeta) {
const pageMeta = (this.$page.frontmatter.meta || []).slice(0)
haoranpb marked this conversation as resolved.
Show resolved Hide resolved
// pageMetaTags have higher priority than siteMetaTags
// description needs special attention for it has too many entries
haoranpb marked this conversation as resolved.
Show resolved Hide resolved
return unionBy([{ name: 'description', content: this.$description }],
pageMeta, siteMeta, metaIdentifier)
}
},

Expand All @@ -47,14 +62,20 @@ export default {
}
}

function updateMetaTags (meta, current) {
if (current) {
[...current].forEach(c => {
/**
* Replace currentMetaTags with newMetaTags
* @param {Array<Object>} newMetaTags
* @param {Array<HTMLElement>} currentMetaTags
* @returns {Array<HTMLElement>}
*/
function updateMetaTags (newMetaTags, currentMetaTags) {
if (currentMetaTags) {
[...currentMetaTags].forEach(c => {
document.head.removeChild(c)
})
}
if (meta) {
return meta.map(m => {
if (newMetaTags) {
return newMetaTags.map(m => {
const tag = document.createElement('meta')
Object.keys(m).forEach(key => {
tag.setAttribute(key, m[key])
Expand All @@ -64,3 +85,35 @@ function updateMetaTags (meta, current) {
})
}
}

/**
* Try to identify a meta tag by name, property or itemprop
*
* Return a complete string if none provided
* @param {Object} tag from frontmatter or siteMetaTags
* @returns {String}
*/
function metaIdentifier (tag) {
for (const item of ['name', 'property', 'itemprop']) {
if (tag.hasOwnProperty(item)) return tag[item] + item
}
return JSON.stringify(tag)
}

/**
* Render meta tags
*
* @param {Array} meta
* @returns {Array<string>}
*/

function renderPageMeta (meta) {
if (!meta) return ''
return meta.map(m => {
let res = `<meta`
Object.keys(m).forEach(key => {
res += ` ${key}="${m[key]}"`
Copy link
Contributor

Choose a reason for hiding this comment

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

Shouldn't this be escaped? For example, this config.js:

module.exports = {
  description: 'Image cache & resize service',
}

renders as (within version 1.4.1):

    <meta name="description" content="Image cache & resize service">

which is probably not correct. fwiw, version 1.4.0 didn't had this problem.

Suggestion:

Suggested change
res += ` ${key}="${m[key]}"`
res += ` ${key}="${escape(m[key])}"`

and this import at the top of the file:

const escape = require('escape-html')

})
return res + `>`
}).join('\n ')
}
2 changes: 1 addition & 1 deletion packages/@vuepress/core/lib/node/App.js
Original file line number Diff line number Diff line change
Expand Up @@ -438,6 +438,7 @@ module.exports = class App {
title: this.siteConfig.title || '',
description: this.siteConfig.description || '',
base: this.base,
headTags: this.siteConfig.head || [],
pages: this.pages.map(page => page.toJson()),
themeConfig: this.siteConfig.themeConfig || {},
locales
Expand Down Expand Up @@ -499,4 +500,3 @@ module.exports = class App {
return this
}
}

28 changes: 4 additions & 24 deletions packages/@vuepress/core/lib/node/build/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -77,9 +77,11 @@ module.exports = class Build extends EventEmitter {
})

// pre-render head tags from user config
// filter out meta tags for they will be injected in updateMeta.js
this.userHeadTags = (this.context.siteConfig.head || [])
.filter(item => item[0] !== 'meta')
haoranpb marked this conversation as resolved.
Show resolved Hide resolved
.map(renderHeadTag)
.join('\n ')
.join('\n ')
haoranpb marked this conversation as resolved.
Show resolved Hide resolved

// if the user does not have a custom 404.md, generate the theme's default
if (!this.context.pages.some(p => p.path === '/404.html')) {
Expand Down Expand Up @@ -138,14 +140,10 @@ module.exports = class Build extends EventEmitter {
readline.cursorTo(process.stdout, 0)
process.stdout.write(`Rendering page: ${pagePath}`)

// #565 Avoid duplicate description meta at SSR.
const meta = (page.frontmatter && page.frontmatter.meta || []).filter(item => item.name !== 'description')
const pageMeta = renderPageMeta(meta)

const context = {
url: page.path,
userHeadTags: this.userHeadTags,
pageMeta,
pageMeta: null,
haoranpb marked this conversation as resolved.
Show resolved Hide resolved
title: 'VuePress',
lang: 'en',
description: '',
Expand Down Expand Up @@ -225,24 +223,6 @@ function renderAttrs (attrs = {}) {
}
}

/**
* Render meta tags
*
* @param {Array} meta
* @returns {Array<string>}
*/

function renderPageMeta (meta) {
if (!meta) return ''
return meta.map(m => {
let res = `<meta`
Object.keys(m).forEach(key => {
res += ` ${key}="${escape(m[key])}"`
})
return res + `>`
}).join('')
}

/**
* find and remove empty style chunk caused by
* https://github.com/webpack-contrib/mini-css-extract-plugin/issues/85
Expand Down