Skip to content

Commit 96e8488

Browse files
JoviDeCroockJoviDeCroock
committed
Remove automatic px suffix (#4665)
* Remove automatic px suffix * Remove from jsx-runtime
1 parent 60d761b commit 96e8488

File tree

7 files changed

+236
-27
lines changed

7 files changed

+236
-27
lines changed

‎compat/src/render.js‎

Lines changed: 12 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,7 @@ import {
2424
useSyncExternalStore,
2525
useTransition
2626
} from './index';
27-
import { assign } from './util';
27+
import { assign, IS_NON_DIMENSIONAL } from './util';
2828

2929
export const REACT_ELEMENT_TYPE = Symbol.for('react.element');
3030

@@ -117,7 +117,17 @@ function handleDomVNode(vnode) {
117117
}
118118

119119
let lowerCased = i.toLowerCase();
120-
if (i === 'defaultValue' && 'value' in props && props.value == null) {
120+
if (i === 'style' && typeof value === 'object') {
121+
for (let key in value) {
122+
if (typeof value[key] === 'number' && !IS_NON_DIMENSIONAL.test(key)) {
123+
value[key] += 'px';
124+
}
125+
}
126+
} else if (
127+
i === 'defaultValue' &&
128+
'value' in props &&
129+
props.value == null
130+
) {
121131
// `defaultValue` is treated as a fallback `value` when a value prop is present but null/undefined.
122132
// `defaultValue` for Elements with no value prop is the same as the DOM defaultValue property.
123133
i = 'value';

‎compat/src/util.js‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,3 +11,6 @@ export function shallowDiffers(a, b) {
1111
for (let i in b) if (i !== '__source' && a[i] !== b[i]) return true;
1212
return false;
1313
}
14+
15+
export const IS_NON_DIMENSIONAL =
16+
/acit|ex(?:s|g|n|p|$)|rph|grid|ows|mnc|ntw|ine[ch]|zoo|^ord|itera/i;

‎compat/test/browser/render.test.js‎

Lines changed: 205 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -597,4 +597,209 @@ describe('compat render', () => {
597597

598598
expect(scratch.textContent).to.equal('foo');
599599
});
600+
601+
it('should append "px" to unitless inline css values', () => {
602+
// These are all CSS Properties that support a single <length> value
603+
// that must have a unit. If we encounter a number we append "px" to it.
604+
// The list is taken from: https://developer.mozilla.org/en-US/docs/Web/CSS/Reference
605+
const unitless = {
606+
'border-block': 2,
607+
'border-block-end-width': 3,
608+
'border-block-start-width': 4,
609+
'border-block-width': 5,
610+
'border-bottom-left-radius': 6,
611+
'border-bottom-right-radius': 7,
612+
'border-bottom-width': 8,
613+
'border-end-end-radius': 9,
614+
'border-end-start-radius': 10,
615+
'border-image-outset': 11,
616+
'border-image-width': 12,
617+
'border-inline': 2,
618+
'border-inline-end': 3,
619+
'border-inline-end-width': 4,
620+
'border-inline-start': 1,
621+
'border-inline-start-width': 123,
622+
'border-inline-width': 123,
623+
'border-left': 123,
624+
'border-left-width': 123,
625+
'border-radius': 123,
626+
'border-right': 123,
627+
'border-right-width': 123,
628+
'border-spacing': 123,
629+
'border-start-end-radius': 123,
630+
'border-start-start-radius': 123,
631+
'border-top': 123,
632+
'border-top-left-radius': 123,
633+
'border-top-right-radius': 123,
634+
'border-top-width': 123,
635+
'border-width': 123,
636+
bottom: 123,
637+
'column-gap': 123,
638+
'column-rule-width': 23,
639+
'column-width': 23,
640+
'flex-basis': 23,
641+
'font-size': 123,
642+
'grid-gap': 23,
643+
'grid-auto-columns': 123,
644+
'grid-auto-rows': 123,
645+
'grid-template-columns': 23,
646+
'grid-template-rows': 23,
647+
height: 123,
648+
'inline-size': 23,
649+
inset: 23,
650+
'inset-block-end': 12,
651+
'inset-block-start': 12,
652+
'inset-inline-end': 213,
653+
'inset-inline-start': 213,
654+
left: 213,
655+
'letter-spacing': 213,
656+
margin: 213,
657+
'margin-block': 213,
658+
'margin-block-end': 213,
659+
'margin-block-start': 213,
660+
'margin-bottom': 213,
661+
'margin-inline': 213,
662+
'margin-inline-end': 213,
663+
'margin-inline-start': 213,
664+
'margin-left': 213,
665+
'margin-right': 213,
666+
'margin-top': 213,
667+
'mask-position': 23,
668+
'mask-size': 23,
669+
'max-block-size': 23,
670+
'max-height': 23,
671+
'max-inline-size': 23,
672+
'max-width': 23,
673+
'min-block-size': 23,
674+
'min-height': 23,
675+
'min-inline-size': 23,
676+
'min-width': 23,
677+
'object-position': 23,
678+
'outline-offset': 23,
679+
'outline-width': 123,
680+
padding: 123,
681+
'padding-block': 123,
682+
'padding-block-end': 123,
683+
'padding-block-start': 123,
684+
'padding-bottom': 123,
685+
'padding-inline': 123,
686+
'padding-inline-end': 123,
687+
'padding-inline-start': 123,
688+
'padding-left': 123,
689+
'padding-right': 123,
690+
'padding-top': 123,
691+
perspective: 123,
692+
right: 123,
693+
'scroll-margin': 123,
694+
'scroll-margin-block': 123,
695+
'scroll-margin-block-start': 123,
696+
'scroll-margin-bottom': 123,
697+
'scroll-margin-inline': 123,
698+
'scroll-margin-inline-end': 123,
699+
'scroll-margin-inline-start': 123,
700+
'scroll-margin-inline-left': 123,
701+
'scroll-margin-inline-right': 123,
702+
'scroll-margin-inline-top': 123,
703+
'scroll-padding': 123,
704+
'scroll-padding-block': 123,
705+
'scroll-padding-block-end': 123,
706+
'scroll-padding-block-start': 123,
707+
'scroll-padding-bottom': 123,
708+
'scroll-padding-inline': 123,
709+
'scroll-padding-inline-end': 123,
710+
'scroll-padding-inline-start': 123,
711+
'scroll-padding-left': 123,
712+
'scroll-padding-right': 123,
713+
'scroll-padding-top': 123,
714+
'shape-margin': 123,
715+
'text-decoration-thickness': 123,
716+
'text-indent': 123,
717+
'text-underline-offset': 123,
718+
top: 123,
719+
'transform-origin': 123,
720+
translate: 123,
721+
width: 123,
722+
'word-spacing': 123
723+
};
724+
725+
// These are all CSS properties that have valid numeric values.
726+
// Our appending logic must not be applied here
727+
const untouched = {
728+
'-webkit-line-clamp': 2,
729+
'animation-iteration-count': 3,
730+
'column-count': 2,
731+
// TODO: unsupported atm
732+
// columns: 2,
733+
flex: 1,
734+
'flex-grow': 1,
735+
'flex-shrink': 1,
736+
'font-size-adjust': 123,
737+
'font-weight': 12,
738+
'grid-column': 2,
739+
'grid-column-end': 2,
740+
'grid-column-start': 2,
741+
'grid-row': 2,
742+
'grid-row-end': 2,
743+
'grid-row-start': 2,
744+
// TODO: unsupported atm
745+
//'line-height': 2,
746+
'mask-border-outset': 2,
747+
'mask-border-slice': 2,
748+
'mask-border-width': 2,
749+
'max-zoom': 2,
750+
'min-zoom': 2,
751+
opacity: 123,
752+
order: 123,
753+
orphans: 2,
754+
'grid-row-gap': 23,
755+
scale: 23,
756+
// TODO: unsupported atm
757+
//'tab-size': 23,
758+
widows: 123,
759+
'z-index': 123,
760+
zoom: 123
761+
};
762+
763+
render(
764+
<div
765+
style={{
766+
...unitless,
767+
...untouched
768+
}}
769+
/>,
770+
scratch
771+
);
772+
773+
let style = scratch.firstChild.style;
774+
775+
// Check properties that MUST not be changed
776+
for (const key in unitless) {
777+
// Check if css property is supported
778+
if (
779+
window.CSS &&
780+
typeof window.CSS.supports === 'function' &&
781+
window.CSS.supports(key, unitless[key])
782+
) {
783+
expect(
784+
String(style[key]).endsWith('px'),
785+
`Should append px "${key}: ${unitless[key]}" === "${key}: ${style[key]}"`
786+
).to.equal(true);
787+
}
788+
}
789+
790+
// Check properties that MUST not be changed
791+
for (const key in untouched) {
792+
// Check if css property is supported
793+
if (
794+
window.CSS &&
795+
typeof window.CSS.supports === 'function' &&
796+
window.CSS.supports(key, untouched[key])
797+
) {
798+
expect(
799+
!String(style[key]).endsWith('px'),
800+
`Should be left as is: "${key}: ${untouched[key]}" === "${key}: ${style[key]}"`
801+
).to.equal(true);
802+
}
803+
}
804+
});
600805
});

‎jsx-runtime/src/index.js‎

Lines changed: 9 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,5 @@
11
import { options, Fragment } from 'preact';
22
import { encodeEntities } from './utils';
3-
import { IS_NON_DIMENSIONAL } from '../../src/constants';
43

54
let vnodeId = 0;
65

@@ -19,9 +18,9 @@ const isArray = Array.isArray;
1918

2019
/**
2120
* JSX.Element factory used by Babel's {runtime:"automatic"} JSX transform
22-
* @param {VNode['type']} type
23-
* @param {VNode['props']} props
24-
* @param {VNode['key']} [key]
21+
* @param {import('../../src/internal').VNode['type']} type
22+
* @param {import('preact').VNode['props']} props
23+
* @param {import('preact').VNode['key']} [key]
2524
* @param {unknown} [isStaticChildren]
2625
* @param {unknown} [__source]
2726
* @param {unknown} [__self]
@@ -46,7 +45,7 @@ function createVNode(type, props, key, isStaticChildren, __source, __self) {
4645
}
4746
}
4847

49-
/** @type {VNode & { __source: any; __self: any }} */
48+
/** @type {import('../../src/internal').VNode & { __source: any; __self: any }} */
5049
const vnode = {
5150
type,
5251
props: normalizedProps,
@@ -73,12 +72,13 @@ function createVNode(type, props, key, isStaticChildren, __source, __self) {
7372
* Create a template vnode. This function is not expected to be
7473
* used directly, but rather through a precompile JSX transform
7574
* @param {string[]} templates
76-
* @param {Array<string | null | VNode>} exprs
77-
* @returns {VNode}
75+
* @param {Array<string | null | import('preact').VNode>} exprs
76+
* @returns {import('preact').VNode}
7877
*/
7978
function jsxTemplate(templates, ...exprs) {
8079
const vnode = createVNode(Fragment, { tpl: templates, exprs });
8180
// Bypass render to string top level Fragment optimization
81+
// @ts-ignore
8282
vnode.key = vnode._vnode;
8383
return vnode;
8484
}
@@ -112,16 +112,7 @@ function jsxAttr(name, value) {
112112
: JS_TO_CSS[prop] ||
113113
(JS_TO_CSS[prop] = prop.replace(CSS_REGEX, '-$&').toLowerCase());
114114

115-
let suffix = ';';
116-
if (
117-
typeof val === 'number' &&
118-
// Exclude custom-attributes
119-
!name.startsWith('--') &&
120-
!IS_NON_DIMENSIONAL.test(name)
121-
) {
122-
suffix = 'px;';
123-
}
124-
str = str + name + ':' + val + suffix;
115+
str = str + name + ':' + val + ';';
125116
}
126117
}
127118
return name + '="' + str + '"';
@@ -144,7 +135,7 @@ function jsxAttr(name, value) {
144135
* is not expected to be used directly, but rather through a
145136
* precompile JSX transform
146137
* @param {*} value
147-
* @returns {string | null | VNode | Array<string | null | VNode>}
138+
* @returns {string | null | import('preact').VNode | Array<string | null | import('preact').VNode>}
148139
*/
149140
function jsxEscape(value) {
150141
if (

‎jsx-runtime/test/browser/jsx-runtime.test.js‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -133,7 +133,9 @@ describe('precompiled JSX', () => {
133133
});
134134

135135
it('should serialize style object', () => {
136-
expect(jsxAttr('style', { padding: 3 })).to.equal('style="padding:3px;"');
136+
expect(jsxAttr('style', { padding: '3px' })).to.equal(
137+
'style="padding:3px;"'
138+
);
137139
});
138140
});
139141

‎src/diff/props.js‎

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,15 +1,13 @@
1-
import { IS_NON_DIMENSIONAL, NULL, SVG_NAMESPACE } from '../constants';
1+
import { NULL, SVG_NAMESPACE } from '../constants';
22
import options from '../options';
33

44
function setStyle(style, key, value) {
55
if (key[0] == '-') {
66
style.setProperty(key, value == NULL ? '' : value);
77
} else if (value == NULL) {
88
style[key] = '';
9-
} else if (typeof value != 'number' || IS_NON_DIMENSIONAL.test(key)) {
10-
style[key] = value;
119
} else {
12-
style[key] = value + 'px';
10+
style[key] = value;
1311
}
1412
}
1513

‎test/browser/style.test.js‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -55,8 +55,8 @@ describe('style attribute', () => {
5555
backgroundPosition: '10px 10px',
5656
'background-size': 'cover',
5757
gridRowStart: 1,
58-
padding: 5,
59-
top: 100,
58+
padding: '5px',
59+
top: '100px',
6060
left: '100%'
6161
};
6262

0 commit comments

Comments
 (0)