From d8c18b18216ebb0f0d21cb3317811354d471b97d Mon Sep 17 00:00:00 2001 From: Jason Miller Date: Thu, 2 May 2019 11:19:13 -0400 Subject: [PATCH 1/8] Also pass oldVNode to options.diff() --- src/diff/index.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/diff/index.js b/src/diff/index.js index 4a0e148d0a2..c827362eb64 100644 --- a/src/diff/index.js +++ b/src/diff/index.js @@ -36,7 +36,7 @@ export function diff(parentDom, newVNode, oldVNode, context, isSvg, excessDomChi // is equal. if (newVNode._self!==newVNode) return null; - if (options.diff) options.diff(newVNode); + if (options.diff) options.diff(newVNode, oldVNode); let c, p, isNew = false, oldProps, oldState, snapshot, newType = newVNode.type, clearProcessingException; From 956892aa1a9513917524354e1d73c37f0d0d285b Mon Sep 17 00:00:00 2001 From: Jason Miller Date: Thu, 2 May 2019 11:33:29 -0400 Subject: [PATCH 2/8] Debug: detect and warn about component thrashing --- debug/src/debug.js | 31 +++++++++++++++++++++++++++++-- 1 file changed, 29 insertions(+), 2 deletions(-) diff --git a/debug/src/debug.js b/debug/src/debug.js index c09a0863fd6..13672b0d2d8 100644 --- a/debug/src/debug.js +++ b/debug/src/debug.js @@ -26,7 +26,7 @@ export function initDebug() { `); }; - options.diff = vnode => { + options.diff = (vnode, oldVnode) => { let { type, props } = vnode; let children = props && props.children; @@ -71,6 +71,24 @@ export function initDebug() { } } } + + // Detect component thrashing. + // This is a common problem where the diff sees a component reference + // on every render, since it can't infer semantic equivalence. + if ( + // it's a component vnode + typeof type=='function' && + // and the constructor reference got changed + type !== oldVnode.type && + // ... but they seem to be the same source + componentsAreEquivalent(type, oldVnode.type) + ) { + console.error( + `🚨It looks like ${getDisplayName(vnode)} is being recreated on every render, causing serious performance problems.\n` + + `This usually means the component is being defined within another component or render function.\n` + + `For more information, see http://preactjs.com/errors/1` + ); + } // Check prop-types if available if (typeof vnode.type==='function' && vnode.type.propTypes) { @@ -98,9 +116,18 @@ export function initDebug() { keys.push(key); } - if (oldBeforeDiff) oldBeforeDiff(vnode); + if (oldBeforeDiff) oldBeforeDiff(vnode, oldVnode); }; + // If the constructor source text and prototype properties match, + // two different component references are likely to be the same. + function componentsAreEquivalent(a, b) { + return ( + Function.prototype.toString.call(a) == Function.prototype.toString.call(b) && + Object.keys(a.prototype).join() == Object.keys(b.prototype).join() + }; + } + options.hook = (comp) => { if (!comp) { throw new Error('Hook can only be invoked from render methods.'); From ac8f4f4ea33ff384e88e6b5f6f8e34509405eeb1 Mon Sep 17 00:00:00 2001 From: Jason Miller Date: Thu, 2 May 2019 12:08:51 -0400 Subject: [PATCH 3/8] fix syntax error --- debug/src/debug.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/debug/src/debug.js b/debug/src/debug.js index 13672b0d2d8..d88ca3af2c8 100644 --- a/debug/src/debug.js +++ b/debug/src/debug.js @@ -125,7 +125,7 @@ export function initDebug() { return ( Function.prototype.toString.call(a) == Function.prototype.toString.call(b) && Object.keys(a.prototype).join() == Object.keys(b.prototype).join() - }; + ); } options.hook = (comp) => { From da9ed2afea594a371ea4aedf81840ff725ac49b1 Mon Sep 17 00:00:00 2001 From: Jason Miller Date: Thu, 2 May 2019 12:13:53 -0400 Subject: [PATCH 4/8] avoid comparing functions to strings --- debug/src/debug.js | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/debug/src/debug.js b/debug/src/debug.js index d88ca3af2c8..e4f49c3e359 100644 --- a/debug/src/debug.js +++ b/debug/src/debug.js @@ -77,7 +77,7 @@ export function initDebug() { // on every render, since it can't infer semantic equivalence. if ( // it's a component vnode - typeof type=='function' && + typeof type=='function' && typeof oldVnode.type==='function' && // and the constructor reference got changed type !== oldVnode.type && // ... but they seem to be the same source @@ -123,8 +123,8 @@ export function initDebug() { // two different component references are likely to be the same. function componentsAreEquivalent(a, b) { return ( - Function.prototype.toString.call(a) == Function.prototype.toString.call(b) && - Object.keys(a.prototype).join() == Object.keys(b.prototype).join() + Function.prototype.toString.call(a) === Function.prototype.toString.call(b) && + Object.keys(a.prototype).join() === Object.keys(b.prototype).join() ); } From 05cc8ffa5ada751deb2280330126038ffee799cf Mon Sep 17 00:00:00 2001 From: Jason Miller Date: Sun, 30 Jun 2019 00:22:15 -0400 Subject: [PATCH 5/8] Add component to string caching --- debug/src/debug.js | 18 ++++++++++++++---- 1 file changed, 14 insertions(+), 4 deletions(-) diff --git a/debug/src/debug.js b/debug/src/debug.js index 6f91169b64e..cd38c8e149b 100644 --- a/debug/src/debug.js +++ b/debug/src/debug.js @@ -9,6 +9,7 @@ export function initDebug() { let oldDiffed = options.diffed; let oldVnode = options.vnode; const warnedComponents = { useEffect: {}, useLayoutEffect: {} }; + const serializedConstructorMap = typeof WeakMap!=='undefined' && WeakMap(); options.root = (vnode, parentNode) => { if (!parentNode) { @@ -139,10 +140,19 @@ export function initDebug() { // If the constructor source text and prototype properties match, // two different component references are likely to be the same. function componentsAreEquivalent(a, b) { - return ( - Function.prototype.toString.call(a) === Function.prototype.toString.call(b) && - Object.keys(a.prototype).join() === Object.keys(b.prototype).join() - ); + return componentToString(a) === componentToString(b); + } + + function componentToString(c) { + let str; + if (serializedConstructorMap && (str = serializedConstructorMap.get(c)) { + return str; + } + str = Function.prototype.toString.call(c) + Object.keys(c.prototype).join(); + if (serializedConstructorMap) { + serializedConstructorMap.set(c, str); + } + return str; } options.hook = (comp) => { From 7c2ea730cd39c23ca51da7d41c638a806e98cbc7 Mon Sep 17 00:00:00 2001 From: Jason Miller Date: Sun, 30 Jun 2019 00:26:13 -0400 Subject: [PATCH 6/8] Update debug.js --- debug/src/debug.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/debug/src/debug.js b/debug/src/debug.js index cd38c8e149b..72f5e5b5cee 100644 --- a/debug/src/debug.js +++ b/debug/src/debug.js @@ -145,7 +145,7 @@ export function initDebug() { function componentToString(c) { let str; - if (serializedConstructorMap && (str = serializedConstructorMap.get(c)) { + if (serializedConstructorMap && (str = serializedConstructorMap.get(c))) { return str; } str = Function.prototype.toString.call(c) + Object.keys(c.prototype).join(); From 4dea84aeb7a174027df5fdac6f2302069395f4d1 Mon Sep 17 00:00:00 2001 From: Jason Miller Date: Mon, 1 Jul 2019 21:01:34 -0400 Subject: [PATCH 7/8] Update debug.js --- debug/src/debug.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/debug/src/debug.js b/debug/src/debug.js index 72f5e5b5cee..e35e408ca78 100644 --- a/debug/src/debug.js +++ b/debug/src/debug.js @@ -9,7 +9,7 @@ export function initDebug() { let oldDiffed = options.diffed; let oldVnode = options.vnode; const warnedComponents = { useEffect: {}, useLayoutEffect: {} }; - const serializedConstructorMap = typeof WeakMap!=='undefined' && WeakMap(); + const serializedConstructorMap = typeof WeakMap!=='undefined' && new WeakMap(); options.root = (vnode, parentNode) => { if (!parentNode) { From bf0d3add06110c4cacd6e55f3c89a11327ea3eaa Mon Sep 17 00:00:00 2001 From: Jason Miller Date: Tue, 6 Aug 2019 16:29:56 -0400 Subject: [PATCH 8/8] re-add toChildArray import --- debug/src/debug.js | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/debug/src/debug.js b/debug/src/debug.js index 3c1c72758a1..fc637542879 100644 --- a/debug/src/debug.js +++ b/debug/src/debug.js @@ -1,6 +1,6 @@ import { checkPropTypes } from './check-props'; import { getDisplayName } from './devtools/custom'; -import { options } from 'preact'; +import { options, toChildArray } from 'preact'; import { ELEMENT_NODE, DOCUMENT_NODE, DOCUMENT_FRAGMENT_NODE } from './constants'; function getClosestDomNodeParent(parent) { @@ -63,9 +63,9 @@ export function initDebug() { options._diff = (vnode, oldVnode) => { if (vnode == null) { - if (oldBeforeDiff) oldBeforeDiff(vnode, oldVnode); - return; - } + if (oldBeforeDiff) oldBeforeDiff(vnode, oldVnode); + return; + } let { type, props, _parent: parent } = vnode; let children = props && props.children;