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

Move IS_NON_DIMENSIONAL to compat #2629

Merged
merged 4 commits into from
Jul 16, 2020
Merged
Show file tree
Hide file tree
Changes from all 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
10 changes: 10 additions & 0 deletions compat/src/render.js
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import {
Component
} from 'preact';
import { applyEventNormalization } from './events';
import { IS_NON_DIMENSIONAL } from './util';

const CAMEL_PROPS = /^(?:accent|alignment|arabic|baseline|cap|clip(?!PathU)|color|fill|flood|font|glyph(?!R)|horiz|marker(?!H|W|U)|overline|paint|stop|strikethrough|stroke|text(?!L)|underline|unicode|units|v|vector|vert|word|writing|x(?!C))[A-Z]/;

Expand Down Expand Up @@ -141,6 +142,15 @@ options.vnode = vnode => {
] = props[i];
}
}

let style = props.style;
if (typeof style === 'object') {
for (i in style) {
if (typeof style[i] === 'number' && !IS_NON_DIMENSIONAL.test(i)) {
style[i] += 'px';
}
}
}
}

// Events
Expand Down
2 changes: 2 additions & 0 deletions compat/src/util.js
Original file line number Diff line number Diff line change
Expand Up @@ -21,3 +21,5 @@ export function shallowDiffers(a, b) {
for (let i in b) if (i !== '__source' && a[i] !== b[i]) return true;
return false;
}

export const IS_NON_DIMENSIONAL = /^(-|f[lo].*[^se]$|g.{5,}[^ps]$|z|o[pr]|(W.{5})?[lL]i.*(t|mp)$|an|(bo|s).{4}Im|sca|m.{6}[ds]|ta|c.*[st]$|wido|ini)/;
202 changes: 202 additions & 0 deletions compat/test/browser/render.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -304,4 +304,206 @@ describe('compat render', () => {
expect(mountSpy).to.be.calledOnce;
expect(updateSpy).to.not.be.calledOnce;
});

it('should append "px" to unitless inline css values', () => {
// These are all CSS Properties that support a single <length> value
// that must have a unit. If we encounter a number we append "px" to it.
// The list is taken from: https://developer.mozilla.org/en-US/docs/Web/CSS/Reference
const unitless = {
'border-block': 2,
'border-block-end-width': 3,
'border-block-start-width': 4,
'border-block-width': 5,
'border-bottom-left-radius': 6,
'border-bottom-right-radius': 7,
'border-bottom-width': 8,
'border-end-end-radius': 9,
'border-end-start-radius': 10,
'border-image-outset': 11,
'border-image-width': 12,
'border-inline': 2,
'border-inline-end': 3,
'border-inline-end-width': 4,
'border-inline-start': 1,
'border-inline-start-width': 123,
'border-inline-width': 123,
'border-left': 123,
'border-left-width': 123,
'border-radius': 123,
'border-right': 123,
'border-right-width': 123,
'border-spacing': 123,
'border-start-end-radius': 123,
'border-start-start-radius': 123,
'border-top': 123,
'border-top-left-radius': 123,
'border-top-right-radius': 123,
'border-top-width': 123,
'border-width': 123,
bottom: 123,
'column-gap': 123,
'column-rule-width': 23,
'column-width': 23,
'flex-basis': 23,
'font-size': 123,
'grid-gap': 23,
'grid-auto-columns': 123,
'grid-auto-rows': 123,
'grid-template-columns': 23,
'grid-template-rows': 23,
height: 123,
'inline-size': 23,
inset: 23,
'inset-block-end': 12,
'inset-block-start': 12,
'inset-inline-end': 213,
'inset-inline-start': 213,
left: 213,
'letter-spacing': 213,
margin: 213,
'margin-block': 213,
'margin-block-end': 213,
'margin-block-start': 213,
'margin-bottom': 213,
'margin-inline': 213,
'margin-inline-end': 213,
'margin-inline-start': 213,
'margin-left': 213,
'margin-right': 213,
'margin-top': 213,
'mask-position': 23,
'mask-size': 23,
'max-block-size': 23,
'max-height': 23,
'max-inline-size': 23,
'max-width': 23,
'min-block-size': 23,
'min-height': 23,
'min-inline-size': 23,
'min-width': 23,
'object-position': 23,
'outline-offset': 23,
'outline-width': 123,
padding: 123,
'padding-block': 123,
'padding-block-end': 123,
'padding-block-start': 123,
'padding-bottom': 123,
'padding-inline': 123,
'padding-inline-end': 123,
'padding-inline-start': 123,
'padding-left': 123,
'padding-right': 123,
'padding-top': 123,
perspective: 123,
right: 123,
'scroll-margin': 123,
'scroll-margin-block': 123,
'scroll-margin-block-start': 123,
'scroll-margin-bottom': 123,
'scroll-margin-inline': 123,
'scroll-margin-inline-end': 123,
'scroll-margin-inline-start': 123,
'scroll-margin-inline-left': 123,
'scroll-margin-inline-right': 123,
'scroll-margin-inline-top': 123,
'scroll-padding': 123,
'scroll-padding-block': 123,
'scroll-padding-block-end': 123,
'scroll-padding-block-start': 123,
'scroll-padding-bottom': 123,
'scroll-padding-inline': 123,
'scroll-padding-inline-end': 123,
'scroll-padding-inline-start': 123,
'scroll-padding-left': 123,
'scroll-padding-right': 123,
'scroll-padding-top': 123,
'shape-margin': 123,
'text-decoration-thickness': 123,
'text-indent': 123,
'text-underline-offset': 123,
top: 123,
'transform-origin': 123,
translate: 123,
width: 123,
'word-spacing': 123
};

// These are all CSS properties that have valid numeric values.
// Our appending logic must not be applied here
const untouched = {
'-webkit-line-clamp': 2,
'animation-iteration-count': 3,
'column-count': 2,
columns: 2,
flex: 1,
'flex-grow': 1,
'flex-shrink': 1,
'font-size-adjust': 123,
'font-weight': 12,
'grid-column': 2,
'grid-column-end': 2,
'grid-column-start': 2,
'grid-row': 2,
'grid-row-end': 2,
'grid-row-start': 2,
'line-height': 2,
'mask-border-outset': 2,
'mask-border-slice': 2,
'mask-border-width': 2,
'max-zoom': 2,
'min-zoom': 2,
opacity: 123,
order: 123,
orphans: 2,
'grid-row-gap': 23,
scale: 23,
'tab-size': 23,
widows: 123,
'z-index': 123,
zoom: 123
};

render(
<div
style={{
...unitless,
...untouched
}}
/>,
scratch
);

let style = scratch.firstChild.style;

// Check properties that MUST not be changed
for (const key in unitless) {
// Check if css property is supported
if (
window.CSS &&
typeof window.CSS.supports === 'function' &&
window.CSS.supports(key, unitless[key])
) {
expect(
String(style[key]).endsWith('px'),
`Should append px "${key}: ${unitless[key]}" === "${key}: ${style[key]}"`
).to.equal(true);
}
}

// Check properties that MUST not be changed
for (const key in untouched) {
// Check if css property is supported
if (
window.CSS &&
typeof window.CSS.supports === 'function' &&
window.CSS.supports(key, untouched[key])
) {
expect(
!String(style[key]).endsWith('px'),
`Should be left as is: "${key}: ${untouched[key]}" === "${key}: ${style[key]}"`
).to.equal(true);
}
}
});
});
20 changes: 19 additions & 1 deletion debug/src/debug.js
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ import {
getCurrentVNode,
getDisplayName
} from './component-stack';
import { IS_NON_DIMENSIONAL } from '../../compat/src/util';

const isWeakMapSupported = typeof WeakMap == 'function';

Expand Down Expand Up @@ -158,11 +159,12 @@ export function initDebug() {
);
}

let isCompatNode = '$$typeof' in vnode;
if (
vnode.ref !== undefined &&
typeof vnode.ref != 'function' &&
typeof vnode.ref != 'object' &&
!('$$typeof' in vnode) // allow string refs when preact-compat is installed
!isCompatNode // allow string refs when preact-compat is installed
) {
throw new Error(
`Component's "ref" property should be a function, or an object created ` +
Expand All @@ -186,6 +188,22 @@ export function initDebug() {
serializeVNode(vnode) +
`\n\n${getOwnerStack(vnode)}`
);
} else if (
!isCompatNode &&
key === 'style' &&
vnode.props[key] !== null &&
typeof vnode.props[key] === 'object'
) {
const style = vnode.props[key];
for (let i in style) {
if (typeof style[i] === 'number' && !IS_NON_DIMENSIONAL.test(i)) {
console.warn(
`Inline CSS value is missing a unit like "rem" or "px: "${i}: ${style[i]}"\n` +
serializeVNode(vnode) +
`\n\n${getOwnerStack(vnode)}`
);
}
}
}
}
}
Expand Down
6 changes: 6 additions & 0 deletions debug/test/browser/debug.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -271,6 +271,12 @@ describe('debug', () => {
expect(console.error).to.not.be.called;
});

it('should warn on invalid unitless inline CSS value', () => {
render(<div style={{ padding: 5 }} />, scratch);
expect(console.warn).to.be.calledOnce;
expect(console.warn.args[0][0]).to.match(/CSS value is missing a unit/);
});

describe('duplicate keys', () => {
const List = props => <ul>{props.children}</ul>;
const ListItem = props => <li>{props.children}</li>;
Expand Down
1 change: 0 additions & 1 deletion src/constants.js
Original file line number Diff line number Diff line change
@@ -1,3 +1,2 @@
export const EMPTY_OBJ = {};
export const EMPTY_ARR = [];
export const IS_NON_DIMENSIONAL = /^(-|f[lo].*[^se]$|g.{5,}[^ps]$|z|o[pr]|(W.{5})?[lL]i.*(t|mp)$|an|(bo|s).{4}Im|sca|m.{6}[ds]|ta|c.*[st]$|wido|ini)/;
6 changes: 0 additions & 6 deletions src/diff/props.js
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@
import { IS_NON_DIMENSIONAL } from '../constants';
import options from '../options';

/**
Expand Down Expand Up @@ -36,11 +35,6 @@ export function diffProps(dom, newProps, oldProps, isSvg, hydrate) {
function setStyle(style, key, value) {
if (key[0] === '-') {
style.setProperty(key, value);
} else if (
typeof value == 'number' &&
IS_NON_DIMENSIONAL.test(key) === false
) {
style[key] = value + 'px';
} else if (value == null) {
style[key] = '';
} else {
Expand Down
4 changes: 2 additions & 2 deletions test/browser/style.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -54,8 +54,8 @@ describe('style attribute', () => {
backgroundPosition: '10px 10px',
'background-size': 'cover',
gridRowStart: 1,
padding: 5,
top: 100,
padding: '5px',
top: '100px',
left: '100%'
};

Expand Down