Merge pull request #4043 from preactjs/table-validate

preactjs · Jun 8, 2023 · 841ef82 · 841ef82
2 parents ac1f145 + 2833c8e
commit 841ef82
Show file tree

Hide file tree

Showing 2 changed files with 94 additions and 41 deletions.
diff --git a/debug/src/debug.js b/debug/src/debug.js
@@ -15,12 +15,22 @@ import { assign, isNaN } from './util';
 
 const isWeakMapSupported = typeof WeakMap == 'function';
 
-function getClosestDomNodeParent(parent) {
-	if (!parent) return {};
+/**
+ * @param {import('./internal').VNode} parent
+ * @returns {string}
+ */
+function getClosestDomNodeParentName(parent) {
+	if (!parent) return '';
 	if (typeof parent.type == 'function') {
-		return getClosestDomNodeParent(parent._parent);
+		if (parent._parent === null) {
+			if (parent._dom !== null && parent._dom.parentNode !== null) {
+				return parent._dom.parentNode.localName;
+			}
+			return '';
+		}
+		return getClosestDomNodeParentName(parent._parent);
 	}
-	return parent;
+	return /** @type {string} */ (parent.type);
 }
 
 export function initDebug() {
@@ -36,6 +46,7 @@ export function initDebug() {
 	let oldCatchError = options._catchError;
 	let oldRoot = options._root;
 	let oldHook = options._hook;
+	let oldCommit = options._commit;
 	const warnedComponents = !isWeakMapSupported
 		? null
 		: {
@@ -44,6 +55,8 @@ export function initDebug() {
 				lazyPropTypes: new WeakMap()
 		  };
 	const deprecations = [];
+	/** @type {import("./internal.d.ts").VNode[]} */
+	let checkVNodeDom = [];
 
 	options._catchError = (error, vnode, oldVNode, errorInfo) => {
 		let component = vnode && vnode._component;
@@ -116,8 +129,18 @@ export function initDebug() {
 	};
 
 	options._diff = vnode => {
-		let { type, _parent: parent } = vnode;
-		let parentVNode = getClosestDomNodeParent(parent);
+		let { type } = vnode;
+		if (
+			typeof type === 'string' &&
+			(type === 'thead' ||
+				type === 'tfoot' ||
+				type === 'tbody' ||
+				type === 'tr' ||
+				type === 'td' ||
+				type === 'th')
+		) {
+			checkVNodeDom.push(vnode);
+		}
 
 		hooksAllowed = true;
 
@@ -146,41 +169,6 @@ export function initDebug() {
 			);
 		}
 
-		if (
-			(type === 'thead' || type === 'tfoot' || type === 'tbody') &&
-			parentVNode.type !== 'table'
-		) {
-			console.error(
-				'Improper nesting of table. Your <thead/tbody/tfoot> should have a <table> parent.' +
-					serializeVNode(vnode) +
-					`\n\n${getOwnerStack(vnode)}`
-			);
-		} else if (
-			type === 'tr' &&
-			parentVNode.type !== 'thead' &&
-			parentVNode.type !== 'tfoot' &&
-			parentVNode.type !== 'tbody' &&
-			parentVNode.type !== 'table'
-		) {
-			console.error(
-				'Improper nesting of table. Your <tr> should have a <thead/tbody/tfoot/table> parent.' +
-					serializeVNode(vnode) +
-					`\n\n${getOwnerStack(vnode)}`
-			);
-		} else if (type === 'td' && parentVNode.type !== 'tr') {
-			console.error(
-				'Improper nesting of table. Your <td> should have a <tr> parent.' +
-					serializeVNode(vnode) +
-					`\n\n${getOwnerStack(vnode)}`
-			);
-		} else if (type === 'th' && parentVNode.type !== 'tr') {
-			console.error(
-				'Improper nesting of table. Your <th> should have a <tr>.' +
-					serializeVNode(vnode) +
-					`\n\n${getOwnerStack(vnode)}`
-			);
-		}
-
 		if (
 			vnode.ref !== undefined &&
 			typeof vnode.ref != 'function' &&
@@ -387,6 +375,57 @@ export function initDebug() {
 			}
 		}
 	};
+
+	options._commit = (root, queue) => {
+		for (let i = 0; i < checkVNodeDom.length; i++) {
+			const vnode = checkVNodeDom[i];
+
+			// Check if HTML nesting is valid. We need to do it in `options.diffed`
+			// so that we can optionally traverse outside the vdom root in case
+			// it's an island embedded in an existing (and valid) HTML tree.
+			const { type, _parent: parent } = vnode;
+
+			let domParentName = getClosestDomNodeParentName(parent);
+
+			if (
+				(type === 'thead' || type === 'tfoot' || type === 'tbody') &&
+				domParentName !== 'table'
+			) {
+				console.error(
+					'Improper nesting of table. Your <thead/tbody/tfoot> should have a <table> parent.' +
+						serializeVNode(vnode) +
+						`\n\n${getOwnerStack(vnode)}`
+				);
+			} else if (
+				type === 'tr' &&
+				domParentName !== 'thead' &&
+				domParentName !== 'tfoot' &&
+				domParentName !== 'tbody' &&
+				domParentName !== 'table'
+			) {
+				console.error(
+					'Improper nesting of table. Your <tr> should have a <thead/tbody/tfoot/table> parent.' +
+						serializeVNode(vnode) +
+						`\n\n${getOwnerStack(vnode)}`
+				);
+			} else if (type === 'td' && domParentName !== 'tr') {
+				console.error(
+					'Improper nesting of table. Your <td> should have a <tr> parent.' +
+						serializeVNode(vnode) +
+						`\n\n${getOwnerStack(vnode)}`
+				);
+			} else if (type === 'th' && domParentName !== 'tr') {
+				console.error(
+					'Improper nesting of table. Your <th> should have a <tr>.' +
+						serializeVNode(vnode) +
+						`\n\n${getOwnerStack(vnode)}`
+				);
+			}
+		}
+		checkVNodeDom = [];
+
+		if (oldCommit) oldCommit(root, queue);
+	};
 }
 
 const setState = Component.prototype.setState;

diff --git a/debug/test/browser/debug.test.js b/debug/test/browser/debug.test.js
@@ -15,6 +15,7 @@ const h = createElement;
 /** @jsx createElement */
 
 describe('debug', () => {
+	/** @type {HTMLDivElement} */
 	let scratch;
 	let errors = [];
 	let warnings = [];
@@ -513,6 +514,19 @@ describe('debug', () => {
 			render(<Table />, scratch);
 			expect(console.error).to.not.be.called;
 		});
+
+		it('should include DOM parents outside of root node', () => {
+			const Table = () => (
+				<tr>
+					<td>Head</td>
+				</tr>
+			);
+
+			const table = document.createElement('table');
+			scratch.appendChild(table);
+			render(<Table />, table);
+			expect(console.error).to.not.be.called;
+		});
 	});
 
 	describe('PropTypes', () => {