Remove automatic px suffix

preactjs · Feb 14, 2025 · f1505c3 · f1505c3
1 parent 5094a51
commit f1505c3
Show file tree

Hide file tree

Showing 8 changed files with 236 additions and 17 deletions.
diff --git a/compat/src/render.js b/compat/src/render.js
@@ -24,7 +24,7 @@ import {
 	useSyncExternalStore,
 	useTransition
 } from './index';
-import { assign } from './util';
+import { assign, IS_NON_DIMENSIONAL } from './util';
 
 export const REACT_ELEMENT_TYPE = Symbol.for('react.element');
 
@@ -117,7 +117,17 @@ function handleDomVNode(vnode) {
 		}
 
 		let lowerCased = i.toLowerCase();
-		if (i === 'defaultValue' && 'value' in props && props.value == null) {
+		if (i === 'style' && typeof value === 'object') {
+			for (let key in value) {
+				if (typeof value[key] === 'number' && !IS_NON_DIMENSIONAL.test(key)) {
+					value[key] += 'px';
+				}
+			}
+		} else if (
+			i === 'defaultValue' &&
+			'value' in props &&
+			props.value == null
+		) {
 			// `defaultValue` is treated as a fallback `value` when a value prop is present but null/undefined.
 			// `defaultValue` for Elements with no value prop is the same as the DOM defaultValue property.
 			i = 'value';

diff --git a/compat/src/util.js b/compat/src/util.js
@@ -11,3 +11,6 @@ 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 =
+	/acit|ex(?:s|g|n|p|$)|rph|grid|ows|mnc|ntw|ine[ch]|zoo|^ord|itera/i;
diff --git a/compat/test/browser/render.test.js b/compat/test/browser/render.test.js
@@ -597,4 +597,209 @@ describe('compat render', () => {
 
 		expect(scratch.textContent).to.equal('foo');
 	});
+
+	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,
+			// TODO: unsupported atm
+			// 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,
+			// TODO: unsupported atm
+			//'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,
+			// TODO: unsupported atm
+			//'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);
+			}
+		}
+	});
 });
diff --git a/jsx-runtime/package.json b/jsx-runtime/package.json
@@ -3,6 +3,7 @@
   "amdName": "jsxRuntime",
   "version": "1.0.0",
   "private": true,
+  "sideEffects": false,
   "description": "Preact JSX runtime",
   "main": "dist/jsxRuntime.js",
   "module": "dist/jsxRuntime.module.js",

diff --git a/jsx-runtime/src/index.js b/jsx-runtime/src/index.js
@@ -1,6 +1,5 @@
 import { options, Fragment } from 'preact';
-import { encodeEntities } from './utils';
-import { IS_NON_DIMENSIONAL } from '../../src/constants';
+import { encodeEntities, IS_NON_DIMENSIONAL } from './utils';
 
 let vnodeId = 0;
 
@@ -19,9 +18,9 @@ const isArray = Array.isArray;
 
 /**
  * JSX.Element factory used by Babel's {runtime:"automatic"} JSX transform
- * @param {VNode['type']} type
- * @param {VNode['props']} props
- * @param {VNode['key']} [key]
+ * @param {import('../../src/internal').VNode['type']} type
+ * @param {import('preact').VNode['props']} props
+ * @param {import('preact').VNode['key']} [key]
  * @param {unknown} [isStaticChildren]
  * @param {unknown} [__source]
  * @param {unknown} [__self]
@@ -46,7 +45,7 @@ function createVNode(type, props, key, isStaticChildren, __source, __self) {
 		}
 	}
 
-	/** @type {VNode & { __source: any; __self: any }} */
+	/** @type {import('../../src/internal').VNode & { __source: any; __self: any }} */
 	const vnode = {
 		type,
 		props: normalizedProps,
@@ -73,12 +72,13 @@ function createVNode(type, props, key, isStaticChildren, __source, __self) {
  * Create a template vnode. This function is not expected to be
  * used directly, but rather through a precompile JSX transform
  * @param {string[]} templates
- * @param  {Array<string | null | VNode>} exprs
- * @returns {VNode}
+ * @param  {Array<string | null | import('preact').VNode>} exprs
+ * @returns {import('preact').VNode}
  */
 function jsxTemplate(templates, ...exprs) {
 	const vnode = createVNode(Fragment, { tpl: templates, exprs });
 	// Bypass render to string top level Fragment optimization
+	// @ts-ignore
 	vnode.key = vnode._vnode;
 	return vnode;
 }
@@ -144,7 +144,7 @@ function jsxAttr(name, value) {
  * is not expected to be used directly, but rather through a
  * precompile JSX transform
  * @param {*} value
- * @returns {string | null | VNode | Array<string | null | VNode>}
+ * @returns {string | null | import('preact').VNode | Array<string | null | import('preact').VNode>}
  */
 function jsxEscape(value) {
 	if (

diff --git a/jsx-runtime/src/utils.js b/jsx-runtime/src/utils.js
@@ -1,4 +1,6 @@
 const ENCODED_ENTITIES = /["&<]/;
+export const IS_NON_DIMENSIONAL =
+	/acit|ex(?:s|g|n|p|$)|rph|grid|ows|mnc|ntw|ine[ch]|zoo|^ord|itera/i;
 
 /** @param {string} str */
 export function encodeEntities(str) {

diff --git a/src/diff/props.js b/src/diff/props.js
@@ -1,15 +1,13 @@
-import { IS_NON_DIMENSIONAL, SVG_NAMESPACE } from '../constants';
+import { SVG_NAMESPACE } from '../constants';
 import options from '../options';
 
 function setStyle(style, key, value) {
 	if (key[0] == '-') {
 		style.setProperty(key, value == null ? '' : value);
 	} else if (value == null) {
 		style[key] = '';
-	} else if (typeof value != 'number' || IS_NON_DIMENSIONAL.test(key)) {
-		style[key] = value;
 	} else {
-		style[key] = value + 'px';
+		style[key] = value;
 	}
 }
 

diff --git a/test/browser/style.test.js b/test/browser/style.test.js
@@ -55,8 +55,8 @@ describe('style attribute', () => {
 			backgroundPosition: '10px 10px',
 			'background-size': 'cover',
 			gridRowStart: 1,
-			padding: 5,
-			top: 100,
+			padding: '5px',
+			top: '100px',
 			left: '100%'
 		};