Skip to content

Commit

Permalink
fix(ssr): properly handle invalid and numeric style properties
Browse files Browse the repository at this point in the history
Ignores values that are not string or numbers, and append px as default
unit to appropriate properties.

There will still be certain cases where the user simply provides an
invalid string value to a property which will be too costly to detect
(it's possible by using the `cssstyle` package, but very heavy). But
in such cases the user would already notice the style is not working
on the client, so it's not really worth it for the perf implications.

fix vuejs#9231
  • Loading branch information
yyx990803 authored and hefeng committed Jan 25, 2019
1 parent 5ce92dc commit 8bc1955
Show file tree
Hide file tree
Showing 4 changed files with 108 additions and 4 deletions.
25 changes: 22 additions & 3 deletions src/platforms/web/server/modules/style.js
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
/* @flow */

import { escape } from '../util'
import { escape, noUnitNumericStyleProps } from '../util'
import { hyphenate } from 'shared/util'
import { getStyle } from 'web/util/style'

Expand All @@ -11,15 +11,34 @@ export function genStyle (style: Object): string {
const hyphenatedKey = hyphenate(key)
if (Array.isArray(value)) {
for (let i = 0, len = value.length; i < len; i++) {
styleText += `${hyphenatedKey}:${value[i]};`
styleText += normalizeValue(hyphenatedKey, value[i])
}
} else {
styleText += `${hyphenatedKey}:${value};`
styleText += normalizeValue(hyphenatedKey, value)
}
}
return styleText
}

function normalizeValue(key: string, value: any): string {
if (typeof value === 'string') {
return `${key}:${value};`
} else if (typeof value === 'number') {
// Handle numeric values.
// Turns out all evergreen browsers plus IE11 already support setting plain
// numbers on the style object and will automatically convert it to px if
// applicable, so we should support that on the server too.
if (noUnitNumericStyleProps[key]) {
return `${key}:${value};`
} else {
return `${key}:${value}px;`
}
} else {
// invalid values
return ``
}
}

export default function renderStyle (vnode: VNodeWithData): ?string {
const styleText = genStyle(getStyle(vnode, false))
if (styleText !== '') {
Expand Down
45 changes: 45 additions & 0 deletions src/platforms/web/server/util.js
Original file line number Diff line number Diff line change
Expand Up @@ -54,3 +54,48 @@ export function escape (s: string) {
function escapeChar (a) {
return ESC[a] || a
}

export const noUnitNumericStyleProps = {
"animation-iteration-count": true,
"border-image-outset": true,
"border-image-slice": true,
"border-image-width": true,
"box-flex": true,
"box-flex-group": true,
"box-ordinal-group": true,
"column-count": true,
"columns": true,
"flex": true,
"flex-grow": true,
"flex-positive": true,
"flex-shrink": true,
"flex-negative": true,
"flex-order": true,
"grid-row": true,
"grid-row-end": true,
"grid-row-span": true,
"grid-row-start": true,
"grid-column": true,
"grid-column-end": true,
"grid-column-span": true,
"grid-column-start": true,
"font-weight": true,
"line-clamp": true,
"line-height": true,
"opacity": true,
"order": true,
"orphans": true,
"tab-size": true,
"widows": true,
"z-index": true,
"zoom": true,
// SVG
"fill-opacity": true,
"flood-opacity": true,
"stop-opacity": true,
"stroke-dasharray": true,
"stroke-dashoffset": true,
"stroke-miterlimit": true,
"stroke-opacity": true,
"stroke-width": true
}
1 change: 0 additions & 1 deletion src/platforms/web/util/style.js
Original file line number Diff line number Diff line change
Expand Up @@ -69,4 +69,3 @@ export function getStyle (vnode: VNodeWithData, checkChild: boolean): Object {
}
return res
}

41 changes: 41 additions & 0 deletions test/ssr/ssr-string.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -1499,6 +1499,47 @@ describe('SSR: renderToString', () => {
done()
})
})

it('invalid style value', done => {
renderVmWithOptions({
template: '<div :style="style"><p :style="style2"/></div>',
data: {
// all invalid, should not even have "style" attribute
style: {
opacity: {},
color: null
},
// mix of valid and invalid
style2: {
opacity: 0,
color: null
}
}
}, result => {
expect(result).toContain(
'<div data-server-rendered="true"><p style="opacity:0;"></p></div>'
)
done()
})
})

it('numeric style value', done => {
renderVmWithOptions({
template: '<div :style="style"></div>',
data: {
style: {
opacity: 0, // opacity should display as-is
top: 0, // top and margin should be converted to '0px'
marginTop: 10
}
}
}, result => {
expect(result).toContain(
'<div data-server-rendered="true" style="opacity:0;top:0px;margin-top:10px;"></div>'
)
done()
})
})
})

function renderVmWithOptions (options, cb) {
Expand Down

0 comments on commit 8bc1955

Please sign in to comment.