diff --git a/compat/server.browser.js b/compat/server.browser.js index 9e8858e822..51c7a5d332 100644 --- a/compat/server.browser.js +++ b/compat/server.browser.js @@ -1,4 +1,11 @@ +import { renderToString } from 'preact-render-to-string'; + export { renderToString, renderToString as renderToStaticMarkup } from 'preact-render-to-string'; + +export default { + renderToString, + renderToStaticMarkup: renderToString +}; diff --git a/compat/server.mjs b/compat/server.mjs index 9e8858e822..51c7a5d332 100644 --- a/compat/server.mjs +++ b/compat/server.mjs @@ -1,4 +1,11 @@ +import { renderToString } from 'preact-render-to-string'; + export { renderToString, renderToString as renderToStaticMarkup } from 'preact-render-to-string'; + +export default { + renderToString, + renderToStaticMarkup: renderToString +}; diff --git a/compat/src/index.d.ts b/compat/src/index.d.ts index 00f47d0def..28125420bc 100644 --- a/compat/src/index.d.ts +++ b/compat/src/index.d.ts @@ -17,6 +17,7 @@ declare namespace React { export import Inputs = _hooks.Inputs; export import PropRef = _hooks.PropRef; export import Reducer = _hooks.Reducer; + export import Dispatch = _hooks.Dispatch; export import Ref = _hooks.Ref; export import StateUpdater = _hooks.StateUpdater; export import useCallback = _hooks.useCallback; @@ -40,6 +41,7 @@ declare namespace React { ): T; // Preact Defaults + export import Context = preact.Context; export import ContextType = preact.ContextType; export import RefObject = preact.RefObject; export import Component = preact.Component; diff --git a/debug/src/debug.js b/debug/src/debug.js index 60baab0031..d76a60ca45 100644 --- 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() { @@ -32,9 +42,11 @@ export function initDebug() { let oldBeforeDiff = options._diff; let oldDiffed = options.diffed; let oldVnode = options.vnode; + let oldRender = options._render; let oldCatchError = options._catchError; let oldRoot = options._root; let oldHook = options._hook; + let oldCommit = options._commit; const warnedComponents = !isWeakMapSupported ? null : { @@ -43,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; @@ -115,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; @@ -145,41 +169,6 @@ export function initDebug() { ); } - if ( - (type === 'thead' || type === 'tfoot' || type === 'tbody') && - parentVNode.type !== 'table' - ) { - console.error( - 'Improper nesting of table. Your should have a 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 should have a parent.' + - serializeVNode(vnode) + - `\n\n${getOwnerStack(vnode)}` - ); - } else if (type === 'td' && parentVNode.type !== 'tr') { - console.error( - 'Improper nesting of table. Your parent.' + - serializeVNode(vnode) + - `\n\n${getOwnerStack(vnode)}` - ); - } else if (type === 'th' && parentVNode.type !== 'tr') { - console.error( - 'Improper nesting of table. Your .' + - serializeVNode(vnode) + - `\n\n${getOwnerStack(vnode)}` - ); - } - if ( vnode.ref !== undefined && typeof vnode.ref != 'function' && @@ -252,6 +241,13 @@ export function initDebug() { if (oldBeforeDiff) oldBeforeDiff(vnode); }; + options._render = vnode => { + if (oldRender) { + oldRender(vnode); + } + hooksAllowed = true; + }; + options._hook = (comp, index, type) => { if (!comp || !hooksAllowed) { throw new Error('Hook can only be invoked from render methods.'); @@ -379,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 should have a
should have a
should have a
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 should have a parent.' + + serializeVNode(vnode) + + `\n\n${getOwnerStack(vnode)}` + ); + } else if (type === 'td' && domParentName !== 'tr') { + console.error( + 'Improper nesting of table. Your parent.' + + serializeVNode(vnode) + + `\n\n${getOwnerStack(vnode)}` + ); + } else if (type === 'th' && domParentName !== 'tr') { + console.error( + 'Improper nesting of table. Your .' + + 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 index e72668be2a..cbcd01b063 100644 --- 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(
should have a
should have a
, scratch); expect(console.error).to.not.be.called; }); + + it('should include DOM parents outside of root node', () => { + const Table = () => ( + + + + ); + + const table = document.createElement('table'); + scratch.appendChild(table); + render(
Head
, table); + expect(console.error).to.not.be.called; + }); }); describe('PropTypes', () => { diff --git a/devtools/src/devtools.js b/devtools/src/devtools.js index 5f61152334..670d45b6df 100644 --- a/devtools/src/devtools.js +++ b/devtools/src/devtools.js @@ -2,7 +2,7 @@ import { options, Fragment, Component } from 'preact'; export function initDevTools() { if (typeof window != 'undefined' && window.__PREACT_DEVTOOLS__) { - window.__PREACT_DEVTOOLS__.attachPreact('10.14.1', options, { + window.__PREACT_DEVTOOLS__.attachPreact('10.16.0', options, { Fragment, Component }); diff --git a/hooks/src/index.d.ts b/hooks/src/index.d.ts index dfec612eaa..561f034943 100644 --- a/hooks/src/index.d.ts +++ b/hooks/src/index.d.ts @@ -15,6 +15,7 @@ export function useState(): [ ]; export type Reducer = (prevState: S, action: A) => S; +export type Dispatch = (action: A) => void; /** * An alternative to `useState`. * @@ -27,7 +28,7 @@ export type Reducer = (prevState: S, action: A) => S; export function useReducer( reducer: Reducer, initialState: S -): [S, (action: A) => void]; +): [S, Dispatch]; /** * An alternative to `useState`. @@ -43,7 +44,7 @@ export function useReducer( reducer: Reducer, initialArg: I, init: (arg: I) => S -): [S, (action: A) => void]; +): [S, Dispatch]; /** @deprecated Use the `Ref` type instead. */ type PropRef = MutableRef; diff --git a/hooks/src/index.js b/hooks/src/index.js index 56f1aa11d3..3b611b4933 100644 --- a/hooks/src/index.js +++ b/hooks/src/index.js @@ -53,6 +53,7 @@ options._render = vnode => { hooks._pendingEffects.forEach(invokeCleanup); hooks._pendingEffects.forEach(invokeEffect); hooks._pendingEffects = []; + currentIndex = 0; } } previousComponent = currentComponent; diff --git a/hooks/test/browser/combinations.test.js b/hooks/test/browser/combinations.test.js index 02db5b74f2..028110e8b9 100644 --- a/hooks/test/browser/combinations.test.js +++ b/hooks/test/browser/combinations.test.js @@ -398,4 +398,83 @@ describe('combinations', () => { }); expect(scratch.textContent).to.equal('2'); }); + + it.skip('parent and child refs should be set before all effects', () => { + const anchorId = 'anchor'; + const tooltipId = 'tooltip'; + const effectLog = []; + + let useRef2 = sinon.spy(init => { + const realRef = useRef(init); + const ref = useRef(init); + Object.defineProperty(ref, 'current', { + get: () => realRef.current, + set: value => { + realRef.current = value; + effectLog.push('set ref ' + value?.tagName); + } + }); + return ref; + }); + + function Tooltip({ anchorRef, children }) { + // For example, used to manually position the tooltip + const tooltipRef = useRef2(null); + + useLayoutEffect(() => { + expect(anchorRef.current?.id).to.equal(anchorId); + expect(tooltipRef.current?.id).to.equal(tooltipId); + effectLog.push('tooltip layout effect'); + }, [anchorRef, tooltipRef]); + useEffect(() => { + expect(anchorRef.current?.id).to.equal(anchorId); + expect(tooltipRef.current?.id).to.equal(tooltipId); + effectLog.push('tooltip effect'); + }, [anchorRef, tooltipRef]); + + return ( +
+
+ {children} +
+
+ ); + } + + function App() { + // For example, used to define what element to anchor the tooltip to + const anchorRef = useRef2(null); + + useLayoutEffect(() => { + expect(anchorRef.current?.id).to.equal(anchorId); + effectLog.push('anchor layout effect'); + }, [anchorRef]); + useEffect(() => { + expect(anchorRef.current?.id).to.equal(anchorId); + effectLog.push('anchor effect'); + }, [anchorRef]); + + return ( +
+

+ More info +

+ a tooltip +
+ ); + } + + act(() => { + render(, scratch); + }); + + expect(effectLog).to.deep.equal([ + 'set ref P', + 'set ref DIV', + 'tooltip layout effect', + 'anchor layout effect', + 'tooltip effect', + 'anchor effect' + ]); + }); }); diff --git a/hooks/test/browser/errorBoundary.test.js b/hooks/test/browser/errorBoundary.test.js index 9412ef421d..0e7de2f625 100644 --- a/hooks/test/browser/errorBoundary.test.js +++ b/hooks/test/browser/errorBoundary.test.js @@ -1,6 +1,6 @@ import { createElement, render } from 'preact'; import { setupScratch, teardown } from '../../../test/_util/helpers'; -import { useErrorBoundary } from 'preact/hooks'; +import { useErrorBoundary, useLayoutEffect } from 'preact/hooks'; import { setupRerender } from 'preact/test-utils'; /** @jsx createElement */ @@ -107,4 +107,49 @@ describe('errorBoundary', () => { expect(spy2).to.be.calledWith(error); expect(scratch.innerHTML).to.equal('

Error

'); }); + + it('does not invoke old effects when a cleanup callback throws an error and is handled', () => { + let throwErr = false; + let thrower = sinon.spy(() => { + if (throwErr) { + throw new Error('test'); + } + }); + let badEffect = sinon.spy(() => thrower); + let goodEffect = sinon.spy(); + + function EffectThrowsError() { + useLayoutEffect(badEffect); + return Test; + } + + function Child({ children }) { + useLayoutEffect(goodEffect); + return children; + } + + function App() { + const [err] = useErrorBoundary(); + return err ? ( +

Error

+ ) : ( + + + + ); + } + + render(, scratch); + expect(scratch.innerHTML).to.equal('Test'); + expect(badEffect).to.be.calledOnce; + expect(goodEffect).to.be.calledOnce; + + throwErr = true; + render(, scratch); + rerender(); + expect(scratch.innerHTML).to.equal('

Error

'); + expect(thrower).to.be.calledOnce; + expect(badEffect).to.be.calledOnce; + expect(goodEffect).to.be.calledOnce; + }); }); diff --git a/hooks/test/browser/useEffect.test.js b/hooks/test/browser/useEffect.test.js index 2075e5f2a1..ce4bb73ab3 100644 --- a/hooks/test/browser/useEffect.test.js +++ b/hooks/test/browser/useEffect.test.js @@ -1,4 +1,4 @@ -import { act } from 'preact/test-utils'; +import { act, teardown as teardownAct } from 'preact/test-utils'; import { createElement, render, Fragment, Component } from 'preact'; import { useEffect, useState, useRef } from 'preact/hooks'; import { setupScratch, teardown } from '../../../test/_util/helpers'; @@ -369,6 +369,51 @@ describe('useEffect', () => { ); }); + it('hooks should be called in right order', async () => { + teardownAct(); + + let increment; + + const Counter = () => { + const [count, setCount] = useState(0); + useState('binggo!!'); + const renderRoot = useRef(); + useEffect(() => { + const div = renderRoot.current; + render(, div); + }, [count]); + + increment = () => { + setCount(x => x + 1); + return Promise.resolve().then(() => setCount(x => x + 1)); + }; + + return ( +
+
Count: {count}
+
+
+ ); + }; + + const Dummy = () => { + useState(); + return
dummy
; + }; + + render(, scratch); + + expect(scratch.innerHTML).to.equal( + '
Count: 0
' + ); + /** Using the act function will affect the timing of the useEffect */ + await increment(); + + expect(scratch.innerHTML).to.equal( + '
Count: 2
dummy
' + ); + }); + it('handles errors correctly', () => { class ErrorBoundary extends Component { constructor(props) { diff --git a/hooks/test/browser/useLayoutEffect.test.js b/hooks/test/browser/useLayoutEffect.test.js index 5a5e75eb7b..f20fa1198a 100644 --- a/hooks/test/browser/useLayoutEffect.test.js +++ b/hooks/test/browser/useLayoutEffect.test.js @@ -476,4 +476,41 @@ describe('useLayoutEffect', () => { expect(calls.length).to.equal(1); expect(calls).to.deep.equal(['doing effecthi']); }); + + it('should run layout affects after all refs are invoked', () => { + const calls = []; + const verifyRef = name => el => { + calls.push(name); + expect(document.body.contains(el), name).to.equal(true); + }; + + const App = () => { + const ref = useRef(); + useLayoutEffect(() => { + expect(ref.current).to.equalNode(scratch.querySelector('p')); + + calls.push('doing effect'); + return () => { + calls.push('cleaning up'); + }; + }); + + return ( +
+

+ Hi +

+
+ ); + }; + + act(() => { + render(, scratch); + }); + expect(calls).to.deep.equal([ + 'callback ref inner', + 'callback ref outer', + 'doing effect' + ]); + }); }); diff --git a/package-lock.json b/package-lock.json index 82fea89825..7876cfe36f 100644 --- a/package-lock.json +++ b/package-lock.json @@ -1,12 +1,12 @@ { "name": "preact", - "version": "10.14.1", + "version": "10.16.0", "lockfileVersion": 2, "requires": true, "packages": { "": { "name": "preact", - "version": "10.14.1", + "version": "10.16.0", "license": "MIT", "devDependencies": { "@actions/github": "^5.0.0", @@ -59,7 +59,7 @@ "sade": "^1.7.4", "sinon": "^9.2.3", "sinon-chai": "^3.5.0", - "typescript": "4.4.2", + "typescript": "^4.9.5", "undici": "^4.12.0" }, "funding": { @@ -2563,21 +2563,6 @@ "node": ">=10" } }, - "node_modules/@wdio/config/node_modules/typescript": { - "version": "4.9.5", - "resolved": "https://registry.npmjs.org/typescript/-/typescript-4.9.5.tgz", - "integrity": "sha512-1FXk9E2Hm+QzZQ7z+McJiHL4NW1F2EzMu9Nq9i3zAaGqibafqYwCVU6WyWAuyQRRzOlxou8xZSyXLEN8oKj24g==", - "dev": true, - "optional": true, - "peer": true, - "bin": { - "tsc": "bin/tsc", - "tsserver": "bin/tsserver" - }, - "engines": { - "node": ">=4.2.0" - } - }, "node_modules/@wdio/logger": { "version": "7.26.0", "resolved": "https://registry.npmjs.org/@wdio/logger/-/logger-7.26.0.tgz", @@ -2746,21 +2731,6 @@ } } }, - "node_modules/@wdio/utils/node_modules/typescript": { - "version": "4.9.5", - "resolved": "https://registry.npmjs.org/typescript/-/typescript-4.9.5.tgz", - "integrity": "sha512-1FXk9E2Hm+QzZQ7z+McJiHL4NW1F2EzMu9Nq9i3zAaGqibafqYwCVU6WyWAuyQRRzOlxou8xZSyXLEN8oKj24g==", - "dev": true, - "optional": true, - "peer": true, - "bin": { - "tsc": "bin/tsc", - "tsserver": "bin/tsserver" - }, - "engines": { - "node": ">=4.2.0" - } - }, "node_modules/accepts": { "version": "1.3.8", "resolved": "https://registry.npmjs.org/accepts/-/accepts-1.3.8.tgz", @@ -5367,21 +5337,6 @@ } } }, - "node_modules/devtools/node_modules/typescript": { - "version": "4.9.5", - "resolved": "https://registry.npmjs.org/typescript/-/typescript-4.9.5.tgz", - "integrity": "sha512-1FXk9E2Hm+QzZQ7z+McJiHL4NW1F2EzMu9Nq9i3zAaGqibafqYwCVU6WyWAuyQRRzOlxou8xZSyXLEN8oKj24g==", - "dev": true, - "optional": true, - "peer": true, - "bin": { - "tsc": "bin/tsc", - "tsserver": "bin/tsserver" - }, - "engines": { - "node": ">=4.2.0" - } - }, "node_modules/devtools/node_modules/ua-parser-js": { "version": "1.0.35", "resolved": "https://registry.npmjs.org/ua-parser-js/-/ua-parser-js-1.0.35.tgz", @@ -15279,9 +15234,9 @@ } }, "node_modules/typescript": { - "version": "4.4.2", - "resolved": "https://registry.npmjs.org/typescript/-/typescript-4.4.2.tgz", - "integrity": "sha512-gzP+t5W4hdy4c+68bfcv0t400HVJMMd2+H9B7gae1nQlBzCqvrXX+6GL/b3GAgyTH966pzrZ70/fRjwAtZksSQ==", + "version": "4.9.5", + "resolved": "https://registry.npmjs.org/typescript/-/typescript-4.9.5.tgz", + "integrity": "sha512-1FXk9E2Hm+QzZQ7z+McJiHL4NW1F2EzMu9Nq9i3zAaGqibafqYwCVU6WyWAuyQRRzOlxou8xZSyXLEN8oKj24g==", "dev": true, "bin": { "tsc": "bin/tsc", @@ -15608,21 +15563,6 @@ } } }, - "node_modules/webdriver/node_modules/typescript": { - "version": "4.9.5", - "resolved": "https://registry.npmjs.org/typescript/-/typescript-4.9.5.tgz", - "integrity": "sha512-1FXk9E2Hm+QzZQ7z+McJiHL4NW1F2EzMu9Nq9i3zAaGqibafqYwCVU6WyWAuyQRRzOlxou8xZSyXLEN8oKj24g==", - "dev": true, - "optional": true, - "peer": true, - "bin": { - "tsc": "bin/tsc", - "tsserver": "bin/tsserver" - }, - "engines": { - "node": ">=4.2.0" - } - }, "node_modules/webdriverio": { "version": "7.30.2", "resolved": "https://registry.npmjs.org/webdriverio/-/webdriverio-7.30.2.tgz", @@ -15739,21 +15679,6 @@ "url": "https://github.com/sponsors/sindresorhus" } }, - "node_modules/webdriverio/node_modules/typescript": { - "version": "4.9.5", - "resolved": "https://registry.npmjs.org/typescript/-/typescript-4.9.5.tgz", - "integrity": "sha512-1FXk9E2Hm+QzZQ7z+McJiHL4NW1F2EzMu9Nq9i3zAaGqibafqYwCVU6WyWAuyQRRzOlxou8xZSyXLEN8oKj24g==", - "dev": true, - "optional": true, - "peer": true, - "bin": { - "tsc": "bin/tsc", - "tsserver": "bin/tsserver" - }, - "engines": { - "node": ">=4.2.0" - } - }, "node_modules/webidl-conversions": { "version": "3.0.1", "resolved": "https://registry.npmjs.org/webidl-conversions/-/webidl-conversions-3.0.1.tgz", @@ -18010,14 +17935,6 @@ "requires": { "brace-expansion": "^2.0.1" } - }, - "typescript": { - "version": "4.9.5", - "resolved": "https://registry.npmjs.org/typescript/-/typescript-4.9.5.tgz", - "integrity": "sha512-1FXk9E2Hm+QzZQ7z+McJiHL4NW1F2EzMu9Nq9i3zAaGqibafqYwCVU6WyWAuyQRRzOlxou8xZSyXLEN8oKj24g==", - "dev": true, - "optional": true, - "peer": true } } }, @@ -18140,14 +18057,6 @@ "@types/node": "^18.0.0", "got": "^11.8.1" } - }, - "typescript": { - "version": "4.9.5", - "resolved": "https://registry.npmjs.org/typescript/-/typescript-4.9.5.tgz", - "integrity": "sha512-1FXk9E2Hm+QzZQ7z+McJiHL4NW1F2EzMu9Nq9i3zAaGqibafqYwCVU6WyWAuyQRRzOlxou8xZSyXLEN8oKj24g==", - "dev": true, - "optional": true, - "peer": true } } }, @@ -20183,14 +20092,6 @@ "got": "^11.8.1" } }, - "typescript": { - "version": "4.9.5", - "resolved": "https://registry.npmjs.org/typescript/-/typescript-4.9.5.tgz", - "integrity": "sha512-1FXk9E2Hm+QzZQ7z+McJiHL4NW1F2EzMu9Nq9i3zAaGqibafqYwCVU6WyWAuyQRRzOlxou8xZSyXLEN8oKj24g==", - "dev": true, - "optional": true, - "peer": true - }, "ua-parser-js": { "version": "1.0.35", "resolved": "https://registry.npmjs.org/ua-parser-js/-/ua-parser-js-1.0.35.tgz", @@ -27597,9 +27498,9 @@ } }, "typescript": { - "version": "4.4.2", - "resolved": "https://registry.npmjs.org/typescript/-/typescript-4.4.2.tgz", - "integrity": "sha512-gzP+t5W4hdy4c+68bfcv0t400HVJMMd2+H9B7gae1nQlBzCqvrXX+6GL/b3GAgyTH966pzrZ70/fRjwAtZksSQ==", + "version": "4.9.5", + "resolved": "https://registry.npmjs.org/typescript/-/typescript-4.9.5.tgz", + "integrity": "sha512-1FXk9E2Hm+QzZQ7z+McJiHL4NW1F2EzMu9Nq9i3zAaGqibafqYwCVU6WyWAuyQRRzOlxou8xZSyXLEN8oKj24g==", "dev": true }, "ua-parser-js": { @@ -27833,14 +27734,6 @@ "@types/node": "^18.0.0", "got": "^11.8.1" } - }, - "typescript": { - "version": "4.9.5", - "resolved": "https://registry.npmjs.org/typescript/-/typescript-4.9.5.tgz", - "integrity": "sha512-1FXk9E2Hm+QzZQ7z+McJiHL4NW1F2EzMu9Nq9i3zAaGqibafqYwCVU6WyWAuyQRRzOlxou8xZSyXLEN8oKj24g==", - "dev": true, - "optional": true, - "peer": true } } }, @@ -27927,14 +27820,6 @@ "resolved": "https://registry.npmjs.org/type-fest/-/type-fest-0.20.2.tgz", "integrity": "sha512-Ne+eE4r0/iWnpAxD852z3A+N0Bt5RN//NjJwRd2VFHEmrywxf5vsZlh4R6lixl6B+wz/8d+maTSAkN1FIkI3LQ==", "dev": true - }, - "typescript": { - "version": "4.9.5", - "resolved": "https://registry.npmjs.org/typescript/-/typescript-4.9.5.tgz", - "integrity": "sha512-1FXk9E2Hm+QzZQ7z+McJiHL4NW1F2EzMu9Nq9i3zAaGqibafqYwCVU6WyWAuyQRRzOlxou8xZSyXLEN8oKj24g==", - "dev": true, - "optional": true, - "peer": true } } }, diff --git a/package.json b/package.json index ad697cb303..346e6d4c1b 100644 --- a/package.json +++ b/package.json @@ -1,7 +1,7 @@ { "name": "preact", "amdName": "preact", - "version": "10.14.1", + "version": "10.16.0", "private": false, "description": "Fast 3kb React-compatible Virtual DOM library.", "main": "dist/preact.js", @@ -305,7 +305,7 @@ "sade": "^1.7.4", "sinon": "^9.2.3", "sinon-chai": "^3.5.0", - "typescript": "4.4.2", + "typescript": "^4.9.5", "undici": "^4.12.0" }, "overrides": { diff --git a/src/component.js b/src/component.js index 13a69114e7..2270391120 100644 --- a/src/component.js +++ b/src/component.js @@ -125,7 +125,8 @@ function renderComponent(component) { parentDom = component._parentDom; if (parentDom) { - let commitQueue = []; + let commitQueue = [], + refQueue = []; const oldVNode = assign({}, vnode); oldVNode._original = vnode._original + 1; @@ -138,9 +139,11 @@ function renderComponent(component) { vnode._hydrating != null ? [oldDom] : null, commitQueue, oldDom == null ? getDomSibling(vnode) : oldDom, - vnode._hydrating + vnode._hydrating, + refQueue ); - commitRoot(commitQueue, vnode); + + commitRoot(commitQueue, vnode, refQueue); if (vnode._dom != oldDom) { updateParentDomPointers(vnode); diff --git a/src/diff/children.js b/src/diff/children.js index 4c388ed6f3..90afbaf65e 100644 --- a/src/diff/children.js +++ b/src/diff/children.js @@ -1,7 +1,6 @@ import { diff, unmount, applyRef } from './index'; import { createVNode, Fragment } from '../create-element'; import { EMPTY_OBJ, EMPTY_ARR } from '../constants'; -import { getDomSibling } from '../component'; import { isArray } from '../util'; /** @@ -23,6 +22,7 @@ import { isArray } from '../util'; * render (except when hydrating). Can be a sibling DOM element when diffing * Fragments that have siblings. In most cases, it starts out as `oldChildren[0]._dom`. * @param {boolean} isHydrating Whether or not we are in hydration + * @param {Array} refQueue an array of elements needed to invoke refs */ export function diffChildren( parentDom, @@ -34,18 +34,27 @@ export function diffChildren( excessDomChildren, commitQueue, oldDom, - isHydrating + isHydrating, + refQueue ) { - let i, j, oldVNode, childVNode, newDom, firstChildDom, refs; + let i, + j, + oldVNode, + childVNode, + newDom, + firstChildDom, + skew = 0; // This is a compression of oldParentVNode!=null && oldParentVNode != EMPTY_OBJ && oldParentVNode._children || EMPTY_ARR // as EMPTY_OBJ._children should be `undefined`. let oldChildren = (oldParentVNode && oldParentVNode._children) || EMPTY_ARR; - let oldChildrenLength = oldChildren.length; + let oldChildrenLength = oldChildren.length, + remainingOldChildren = oldChildrenLength, + newChildrenLength = renderResult.length; newParentVNode._children = []; - for (i = 0; i < renderResult.length; i++) { + for (i = 0; i < newChildrenLength; i++) { childVNode = renderResult[i]; if ( @@ -104,40 +113,22 @@ export function diffChildren( childVNode._parent = newParentVNode; childVNode._depth = newParentVNode._depth + 1; - // Check if we find a corresponding element in oldChildren. - // If found, delete the array item by setting to `undefined`. - // We use `undefined`, as `null` is reserved for empty placeholders - // (holes). - oldVNode = oldChildren[i]; + let skewedIndex = i + skew; + const matchingIndex = findMatchingIndex( + childVNode, + oldChildren, + skewedIndex, + remainingOldChildren + ); - if ( - oldVNode === null || - (oldVNode && - childVNode.key == oldVNode.key && - childVNode.type === oldVNode.type) - ) { - oldChildren[i] = undefined; + if (matchingIndex === -1) { + oldVNode = EMPTY_OBJ; } else { - // Either oldVNode === undefined or oldChildrenLength > 0, - // so after this loop oldVNode == null or oldVNode is a valid value. - for (j = 0; j < oldChildrenLength; j++) { - oldVNode = oldChildren[j]; - // If childVNode is unkeyed, we only match similarly unkeyed nodes, otherwise we match by key. - // We always match by type (in either case). - if ( - oldVNode && - childVNode.key == oldVNode.key && - childVNode.type === oldVNode.type - ) { - oldChildren[j] = undefined; - break; - } - oldVNode = null; - } + oldVNode = oldChildren[matchingIndex] || EMPTY_OBJ; + oldChildren[matchingIndex] = undefined; + remainingOldChildren--; } - oldVNode = oldVNode || EMPTY_OBJ; - // Morph the old element into the new one, but don't append it to the dom yet diff( parentDom, @@ -148,15 +139,17 @@ export function diffChildren( excessDomChildren, commitQueue, oldDom, - isHydrating + isHydrating, + refQueue ); newDom = childVNode._dom; if ((j = childVNode.ref) && oldVNode.ref != j) { - if (!refs) refs = []; - if (oldVNode.ref) refs.push(oldVNode.ref, null, childVNode); - refs.push(j, childVNode._component || newDom, childVNode); + if (oldVNode.ref) { + applyRef(oldVNode.ref, null, childVNode); + } + refQueue.push(j, childVNode._component || newDom, childVNode); } if (newDom != null) { @@ -164,24 +157,60 @@ export function diffChildren( firstChildDom = newDom; } + let isMounting = oldVNode === EMPTY_OBJ || oldVNode._original === null; + let hasMatchingIndex = !isMounting && matchingIndex === skewedIndex; + if (isMounting) { + if (matchingIndex == -1) { + skew--; + } + } else if (matchingIndex !== skewedIndex) { + if (matchingIndex === skewedIndex + 1) { + skew++; + hasMatchingIndex = true; + } else if (matchingIndex > skewedIndex) { + if (remainingOldChildren > newChildrenLength - skewedIndex) { + skew += matchingIndex - skewedIndex; + hasMatchingIndex = true; + } else { + // ### Change from keyed: I think this was missing from the algo... + skew--; + } + } else if (matchingIndex < skewedIndex) { + if (matchingIndex == skewedIndex - 1) { + skew = matchingIndex - skewedIndex; + } else { + skew = 0; + } + } else { + skew = 0; + } + } + + skewedIndex = i + skew; + hasMatchingIndex = + hasMatchingIndex || (matchingIndex == i && !isMounting); + if ( typeof childVNode.type == 'function' && - childVNode._children === oldVNode._children + (matchingIndex !== skewedIndex || + oldVNode._children === childVNode._children) ) { - childVNode._nextDom = oldDom = reorderChildren( - childVNode, - oldDom, - parentDom - ); + oldDom = reorderChildren(childVNode, oldDom, parentDom); + } else if (typeof childVNode.type != 'function' && !hasMatchingIndex) { + oldDom = placeChild(parentDom, newDom, oldDom); + } else if (childVNode._nextDom !== undefined) { + // Only Fragments or components that return Fragment like VNodes will + // have a non-undefined _nextDom. Continue the diff from the sibling + // of last DOM child of this child VNode + oldDom = childVNode._nextDom; + + // Eagerly cleanup _nextDom. We don't need to persist the value because + // it is only used by `diffChildren` to determine where to resume the diff after + // diffing Components and Fragments. Once we store it the nextDOM local var, we + // can clean up the property + childVNode._nextDom = undefined; } else { - oldDom = placeChild( - parentDom, - childVNode, - oldVNode, - oldChildren, - newDom, - oldDom - ); + oldDom = newDom.nextSibling; } if (typeof newParentVNode.type == 'function') { @@ -194,14 +223,6 @@ export function diffChildren( // node's nextSibling. newParentVNode._nextDom = oldDom; } - } else if ( - oldDom && - oldVNode._dom == oldDom && - oldDom.parentNode != parentDom - ) { - // The above condition is to handle null placeholders. See test in placeholder.test.js: - // `efficiently replace null placeholders in parent rerenders` - oldDom = getDomSibling(oldVNode); } } @@ -218,24 +239,19 @@ export function diffChildren( // If the newParentVNode.__nextDom points to a dom node that is about to // be unmounted, then get the next sibling of that vnode and set // _nextDom to it - newParentVNode._nextDom = getLastDom(oldParentVNode).nextSibling; + + newParentVNode._nextDom = oldChildren[i]._dom.nextSibling; } unmount(oldChildren[i], oldChildren[i]); } } - - // Set refs only after unmount - if (refs) { - for (i = 0; i < refs.length; i++) { - applyRef(refs[i], refs[++i], refs[++i]); - } - } } function reorderChildren(childVNode, oldDom, parentDom) { // Note: VNodes in nested suspended trees may be missing _children. let c = childVNode._children; + let tmp = 0; for (; c && tmp < c.length; tmp++) { let vnode = c[tmp]; @@ -249,7 +265,7 @@ function reorderChildren(childVNode, oldDom, parentDom) { if (typeof vnode.type == 'function') { oldDom = reorderChildren(vnode, oldDom, parentDom); } else { - oldDom = placeChild(parentDom, vnode, vnode, c, vnode._dom, oldDom); + oldDom = placeChild(parentDom, vnode._dom, oldDom); } } } @@ -276,81 +292,59 @@ export function toChildArray(children, out) { return out; } -function placeChild( - parentDom, - childVNode, - oldVNode, - oldChildren, - newDom, - oldDom -) { - let nextDom; - if (childVNode._nextDom !== undefined) { - // Only Fragments or components that return Fragment like VNodes will - // have a non-undefined _nextDom. Continue the diff from the sibling - // of last DOM child of this child VNode - nextDom = childVNode._nextDom; - - // Eagerly cleanup _nextDom. We don't need to persist the value because - // it is only used by `diffChildren` to determine where to resume the diff after - // diffing Components and Fragments. Once we store it the nextDOM local var, we - // can clean up the property - childVNode._nextDom = undefined; - } else if ( - oldVNode == null || - newDom != oldDom || - newDom.parentNode == null - ) { - outer: if (oldDom == null || oldDom.parentNode !== parentDom) { - parentDom.appendChild(newDom); - nextDom = null; - } else { - // `j (oldVNode != null ? 1 : 0)) { + while (x >= 0 || y < oldChildren.length) { + if (x >= 0) { + oldVNode = oldChildren[x]; + if (oldVNode && key == oldVNode.key && type === oldVNode.type) { + return x; + } + x--; + } - if (vnode._children) { - for (let i = vnode._children.length - 1; i >= 0; i--) { - let child = vnode._children[i]; - if (child) { - let lastDom = getLastDom(child); - if (lastDom) { - return lastDom; + if (y < oldChildren.length) { + oldVNode = oldChildren[y]; + if (oldVNode && key == oldVNode.key && type === oldVNode.type) { + return y; } + y++; } } } - return null; + return -1; } diff --git a/src/diff/index.js b/src/diff/index.js index 477663cd9e..2537a1b35b 100644 --- a/src/diff/index.js +++ b/src/diff/index.js @@ -20,7 +20,8 @@ import options from '../options'; * element any new dom elements should be placed around. Likely `null` on first * render (except when hydrating). Can be a sibling DOM element when diffing * Fragments that have siblings. In most cases, it starts out as `oldChildren[0]._dom`. - * @param {boolean} [isHydrating] Whether or not we are in hydration + * @param {boolean} isHydrating Whether or not we are in hydration + * @param {Array} refQueue an array of elements needed to invoke refs */ export function diff( parentDom, @@ -31,7 +32,8 @@ export function diff( excessDomChildren, commitQueue, oldDom, - isHydrating + isHydrating, + refQueue ) { let tmp, newType = newVNode.type; @@ -134,14 +136,14 @@ export function diff( } if ( - (!c._force && - c.shouldComponentUpdate != null && + !c._force && + ((c.shouldComponentUpdate != null && c.shouldComponentUpdate( newProps, c._nextState, componentContext ) === false) || - newVNode._original === oldVNode._original + newVNode._original === oldVNode._original) ) { // More info about this here: https://gist.github.com/JoviDeCroock/bec5f2ce93544d2e6070ef8e0036e4e8 if (newVNode._original !== oldVNode._original) { @@ -154,8 +156,6 @@ export function diff( c._dirty = false; } - // In cases of bailing due to strict-equality we have to reset force as well - c._force = false; newVNode._dom = oldVNode._dom; newVNode._children = oldVNode._children; newVNode._children.forEach(vnode => { @@ -188,6 +188,7 @@ export function diff( c.context = componentContext; c.props = newProps; c._parentDom = parentDom; + c._force = false; let renderHook = options._render, count = 0; @@ -240,7 +241,8 @@ export function diff( excessDomChildren, commitQueue, oldDom, - isHydrating + isHydrating, + refQueue ); c.base = newVNode._dom; @@ -255,8 +257,6 @@ export function diff( if (clearProcessingException) { c._pendingError = c._processingException = null; } - - c._force = false; } else if ( excessDomChildren == null && newVNode._original === oldVNode._original @@ -272,7 +272,8 @@ export function diff( isSvg, excessDomChildren, commitQueue, - isHydrating + isHydrating, + refQueue ); } @@ -296,7 +297,11 @@ export function diff( * which have callbacks to invoke in commitRoot * @param {import('../internal').VNode} root */ -export function commitRoot(commitQueue, root) { +export function commitRoot(commitQueue, root, refQueue) { + for (let i = 0; i < refQueue.length; i++) { + applyRef(refQueue[i], refQueue[++i], refQueue[++i]); + } + if (options._commit) options._commit(root, commitQueue); commitQueue.some(c => { @@ -326,6 +331,7 @@ export function commitRoot(commitQueue, root) { * @param {Array} commitQueue List of components * which have callbacks to invoke in commitRoot * @param {boolean} isHydrating Whether or not we are in hydration + * @param {Array} refQueue an array of elements needed to invoke refs * @returns {import('../internal').PreactElement} */ function diffElementNodes( @@ -336,7 +342,8 @@ function diffElementNodes( isSvg, excessDomChildren, commitQueue, - isHydrating + isHydrating, + refQueue ) { let oldProps = oldVNode.props; let newProps = newVNode.props; @@ -448,7 +455,8 @@ function diffElementNodes( excessDomChildren ? excessDomChildren[0] : oldVNode._children && getDomSibling(oldVNode, 0), - isHydrating + isHydrating, + refQueue ); // Remove children that are not part of any vnode. diff --git a/src/index.d.ts b/src/index.d.ts index eb192aa15c..a2a881d2bb 100644 --- a/src/index.d.ts +++ b/src/index.d.ts @@ -89,7 +89,7 @@ export type ComponentProps< export interface FunctionComponent

{ (props: RenderableProps

, context?: any): VNode | null; displayName?: string; - defaultProps?: Partial

; + defaultProps?: Partial

| undefined; } export interface FunctionalComponent

extends FunctionComponent

{} @@ -194,7 +194,11 @@ export function createElement( ClassAttributes) | null, ...children: ComponentChildren[] -): VNode; +): VNode< + | (JSXInternal.DOMAttributes & + ClassAttributes) + | null +>; export function createElement< P extends JSXInternal.HTMLAttributes, T extends HTMLElement @@ -202,7 +206,7 @@ export function createElement< type: keyof JSXInternal.IntrinsicElements, props: (ClassAttributes & P) | null, ...children: ComponentChildren[] -): VNode; +): VNode<(ClassAttributes & P) | null>; export function createElement< P extends JSXInternal.SVGAttributes, T extends HTMLElement @@ -210,7 +214,7 @@ export function createElement< type: keyof JSXInternal.IntrinsicElements, props: (ClassAttributes & P) | null, ...children: ComponentChildren[] -): VNode; +): VNode<(ClassAttributes & P) | null>; export function createElement( type: string, props: @@ -219,12 +223,14 @@ export function createElement( JSXInternal.SVGAttributes) | null, ...children: ComponentChildren[] -): VNode; +): VNode< + ClassAttributes & JSXInternal.HTMLAttributes & JSXInternal.SVGAttributes +>; export function createElement

( type: ComponentType

, props: (Attributes & P) | null, ...children: ComponentChildren[] -): VNode; +): VNode

; export namespace createElement { export import JSX = JSXInternal; } @@ -236,7 +242,11 @@ export function h( ClassAttributes) | null, ...children: ComponentChildren[] -): VNode; +): VNode< + | (JSXInternal.DOMAttributes & + ClassAttributes) + | null +>; export function h< P extends JSXInternal.HTMLAttributes, T extends HTMLElement @@ -244,7 +254,7 @@ export function h< type: keyof JSXInternal.IntrinsicElements, props: (ClassAttributes & P) | null, ...children: ComponentChildren[] -): VNode; +): VNode<(ClassAttributes & P) | null>; export function h< P extends JSXInternal.SVGAttributes, T extends HTMLElement @@ -252,7 +262,7 @@ export function h< type: keyof JSXInternal.IntrinsicElements, props: (ClassAttributes & P) | null, ...children: ComponentChildren[] -): VNode; +): VNode<(ClassAttributes & P) | null>; export function h( type: string, props: @@ -261,12 +271,17 @@ export function h( JSXInternal.SVGAttributes) | null, ...children: ComponentChildren[] -): VNode; +): VNode< + | (ClassAttributes & + JSXInternal.HTMLAttributes & + JSXInternal.SVGAttributes) + | null +>; export function h

( type: ComponentType

, props: (Attributes & P) | null, ...children: ComponentChildren[] -): VNode; +): VNode<(Attributes & P) | null>; export namespace h { export import JSX = JSXInternal; } diff --git a/src/jsx.d.ts b/src/jsx.d.ts index 46ab9b6ac4..26b91cd81a 100644 --- a/src/jsx.d.ts +++ b/src/jsx.d.ts @@ -1073,6 +1073,8 @@ export namespace JSXInternal { Target, WheelEvent >; + export type TargetedPictureInPictureEvent = + TargetedEvent; export interface EventHandler { (this: void, event: E): void; @@ -1116,6 +1118,8 @@ export namespace JSXInternal { export type WheelEventHandler = EventHandler< TargetedWheelEvent >; + export type PictureInPictureEventHandler = + EventHandler>; export interface DOMAttributes extends PreactDOMAttributes { @@ -1144,6 +1148,10 @@ export namespace JSXInternal { // Details Events onToggle?: GenericEventHandler | undefined; + // Dialog Events + onClose?: GenericEventHandler | undefined; + onCancel?: GenericEventHandler | undefined; + // Focus Events onFocus?: FocusEventHandler | undefined; onFocusCapture?: FocusEventHandler | undefined; @@ -1317,8 +1325,22 @@ export namespace JSXInternal { onAnimationIterationCapture?: AnimationEventHandler | undefined; // Transition Events + onTransitionCancel?: TransitionEventHandler; + onTransitionCancelCapture?: TransitionEventHandler; onTransitionEnd?: TransitionEventHandler; onTransitionEndCapture?: TransitionEventHandler; + onTransitionRun?: TransitionEventHandler; + onTransitionRunCapture?: TransitionEventHandler; + onTransitionStart?: TransitionEventHandler; + onTransitionStartCapture?: TransitionEventHandler; + + // PictureInPicture Events + onEnterPictureInPicture?: PictureInPictureEventHandler; + onEnterPictureInPictureCapture?: PictureInPictureEventHandler; + onLeavePictureInPicture?: PictureInPictureEventHandler; + onLeavePictureInPictureCapture?: PictureInPictureEventHandler; + onResize?: PictureInPictureEventHandler; + onResizeCapture?: PictureInPictureEventHandler; } // All the WAI-ARIA 1.1 attributes from https://www.w3.org/TR/wai-aria-1.1/ @@ -1334,6 +1356,16 @@ export namespace JSXInternal { 'aria-autocomplete'?: Signalish< 'none' | 'inline' | 'list' | 'both' | undefined >; + /** + * Defines a string value that labels the current element, which is intended to be converted into Braille. + * @see aria-label. + */ + 'aria-braillelabel'?: Signalish; + /** + * Defines a human-readable, author-localized abbreviated description for the role of an element, which is intended to be converted into Braille. + * @see aria-roledescription. + */ + 'aria-brailleroledescription'?: Signalish; /** Indicates an element is being modified and that assistive technologies MAY want to wait until the modifications are complete before exposing them to the user. */ 'aria-busy'?: Signalish; /** @@ -1353,6 +1385,11 @@ export namespace JSXInternal { * @see aria-colspan. */ 'aria-colindex'?: Signalish; + /** + * Defines a human readable text alternative of aria-colindex. + * @see aria-rowindextext. + */ + 'aria-colindextext'?: Signalish; /** * Defines the number of columns spanned by a cell or gridcell within a table, grid, or treegrid. * @see aria-colindex @@ -1373,6 +1410,11 @@ export namespace JSXInternal { * @see aria-labelledby */ 'aria-describedby'?: Signalish; + /** + * Defines a string value that describes or annotates the current element. + * @see related aria-describedby. + */ + 'aria-description'?: Signalish; /** * Identifies the element that provides a detailed, extended description for the object. * @see aria-describedby. @@ -1506,6 +1548,11 @@ export namespace JSXInternal { * @see aria-rowspan. */ 'aria-rowindex'?: Signalish; + /** + * Defines a human readable text alternative of aria-rowindex. + * @see aria-colindextext. + */ + 'aria-rowindextext'?: Signalish; /** * Defines the number of rows spanned by a cell or gridcell within a table, grid, or treegrid. * @see aria-rowindex diff --git a/src/render.js b/src/render.js index 3f92be680e..02901b041d 100644 --- a/src/render.js +++ b/src/render.js @@ -33,7 +33,8 @@ export function render(vnode, parentDom, replaceNode) { createElement(Fragment, null, [vnode]); // List of effects that need to be called after diffing. - let commitQueue = []; + let commitQueue = [], + refQueue = []; diff( parentDom, // Determine the new vnode tree and store it on the DOM element on @@ -55,11 +56,12 @@ export function render(vnode, parentDom, replaceNode) { : oldVNode ? oldVNode._dom : parentDom.firstChild, - isHydrating + isHydrating, + refQueue ); // Flush all queued effects - commitRoot(commitQueue, vnode); + commitRoot(commitQueue, vnode, refQueue); } /** diff --git a/test-utils/src/index.js b/test-utils/src/index.js index 4bf7a21f77..f8df33393d 100644 --- a/test-utils/src/index.js +++ b/test-utils/src/index.js @@ -32,11 +32,19 @@ export function act(cb) { // been set up. // // If an exception occurs, the outermost `act` will handle cleanup. - const result = cb(); - if (isThenable(result)) { - return result.then(() => { - --actDepth; - }); + try { + const result = cb(); + if (isThenable(result)) { + return result.then(() => { + --actDepth; + }, (e) => { + --actDepth; + throw e; + }) + } + } catch(e) { + --actDepth; + throw e; } --actDepth; return Promise.resolve(); diff --git a/test-utils/test/shared/act.test.js b/test-utils/test/shared/act.test.js index d1f6ef248f..af9823b906 100644 --- a/test-utils/test/shared/act.test.js +++ b/test-utils/test/shared/act.test.js @@ -385,6 +385,12 @@ describe('act', () => { } catch (e) {} }; + const tryNestedRenderBroken = () => { + act(() => { + tryRenderBroken(); + }); + } + describe('synchronously', () => { it('should rethrow the exception', () => { expect(renderBroken).to.throw('BrokenWidget is broken'); @@ -396,6 +402,12 @@ describe('act', () => { expect(scratch.textContent).to.equal('1'); }); + it('should not affect state updates in future renders when nested `act` throws an exception', () => { + tryNestedRenderBroken(); + renderWorking(); + expect(scratch.textContent).to.equal('1'); + }); + it('should not affect effects in future renders', () => { tryRenderBroken(); renderWorking(); diff --git a/test/_util/logCall.js b/test/_util/logCall.js index a9ce2e877b..d6d928e7ed 100644 --- a/test/_util/logCall.js +++ b/test/_util/logCall.js @@ -29,11 +29,26 @@ export function logCall(obj, method) { c += serialize(args[i]); } - // Normalize removeChild -> remove to keep output clean and readable - const operation = - method != 'removeChild' - ? `${serialize(this)}.${method}(${c})` - : `${serialize(c)}.remove()`; + let operation; + switch (method) { + case 'removeChild': { + operation = `${serialize(c)}.remove()`; + break; + } + case 'insertBefore': { + if (args[1] === null && args.length === 2) { + operation = `${serialize(this)}.appendChild(${serialize(args[0])})`; + } else { + operation = `${serialize(this)}.${method}(${c})`; + } + break; + } + default: { + operation = `${serialize(this)}.${method}(${c})`; + break; + } + } + log.push(operation); return old.apply(this, args); }; diff --git a/test/browser/components.test.js b/test/browser/components.test.js index 1391d52603..3d36e3a29f 100644 --- a/test/browser/components.test.js +++ b/test/browser/components.test.js @@ -2660,5 +2660,58 @@ describe('Components', () => { expect(scratch.innerHTML).to.equal('

bar
'); }); + + it('should skip shouldComponentUpdate when called during render', () => { + let isSCUCalled = false; + class App extends Component { + shouldComponentUpdate() { + isSCUCalled = true; + return false; + } + render() { + const isUpdated = this.isUpdated; + if (!isUpdated) { + this.isUpdated = true; + this.forceUpdate(); + } + return
Updated: {isUpdated ? 'yes' : 'no'}
; + } + } + render(, scratch); + rerender(); + expect(isSCUCalled).to.be.false; + expect(scratch.innerHTML).to.equal('
Updated: yes
'); + }); + + it('should break through strict equality optimization', () => { + let isSCUCalled = false; + + class Child extends Component { + componentDidMount() { + this.props.parent.forceUpdate(); + this.forceUpdate(); + this.isUpdated = true; + } + shouldComponentUpdate() { + isSCUCalled = true; + return false; + } + render() { + return
Updated: {this.isUpdated ? 'yes' : 'no'}
; + } + } + + class App extends Component { + children = (); + render() { + return this.children; + } + } + + render(, scratch); + rerender(); + expect(isSCUCalled).to.be.false; + expect(scratch.innerHTML).to.equal('
Updated: yes
'); + }); }); }); diff --git a/test/browser/fragments.test.js b/test/browser/fragments.test.js index e6df094d73..55842b952d 100644 --- a/test/browser/fragments.test.js +++ b/test/browser/fragments.test.js @@ -687,7 +687,8 @@ describe('Fragment', () => { expect(scratch.innerHTML).to.equal(htmlForFalse); expectDomLogToBe( [ - '
fooHellobeep.appendChild(
Hello)', + '
barHellobeep.insertBefore(
bar,
beep)', + '
Hellobarbeep.appendChild(
Hello)', '
barbeepHello.appendChild(
bar)' ], 'rendering true to false' @@ -1277,9 +1278,7 @@ describe('Fragment', () => { ); expectDomLogToBe([ '
    012345.insertBefore(
  1. 4,
  2. 0)', - '
      401235.insertBefore(
    1. 5,
    2. 0)', - // TODO: Hmmm why does this extra append happen? - '
        453012.appendChild(
      1. 3)' + '
          401235.insertBefore(
        1. 5,
        2. 0)' ]); clearLog(); @@ -1478,7 +1477,8 @@ describe('Fragment', () => { ); expectDomLogToBe( [ - '
          fooHellobeepboop.insertBefore(
          Hello,
          boop)', + '
          barHellobeepboop.insertBefore(
          bar,
          beep)', + '
          Hellobarbeepboop.insertBefore(
          Hello,
          boop)', '
          barbeepHelloboop.insertBefore(
          bar,
          boop)', '
          boop.remove()' ], @@ -1564,7 +1564,8 @@ describe('Fragment', () => { ); expectDomLogToBe( [ - '
          fooHellobeepbeepbeep.appendChild(
          Hello)', + '
          barHellobeepbeepbeep.insertBefore(
          bar,
          beep)', + '
          Hellobarbeepbeepbeep.appendChild(
          Hello)', '
          barbeepbeepbeepHello.appendChild(
          bar)' ], 'rendering from true to false' @@ -1580,9 +1581,9 @@ describe('Fragment', () => { ); expectDomLogToBe( [ - '
          beepbeepbeepHellofoo.insertBefore(
          foo,
          beep)', - '
          foobeepbeepbeepHello.insertBefore(
          Hello,
          beep)', - '
          fooHelloboopboopboop.appendChild(
          boop)' + '
          beepbeepbeepHellofoo.appendChild(
          Hello)', + '
          beepbeepbeepfooHello.insertBefore(
          foo,
          beep)', + '
          foobeepbeepbeepHello.insertBefore(
          Hello,
          beep)' ], 'rendering from false to true' ); @@ -2608,8 +2609,10 @@ describe('Fragment', () => { rerender(); expect(scratch.innerHTML).to.equal(bottom); expectDomLogToBe([ + '
          top panelNavigationContent.insertBefore(
          Navigation,
          top panel)', + '
          Navigationtop panelContent.insertBefore(
          Content,
          top panel)', '
          .appendChild(#text)', - '
          top panelNavigationContent.appendChild(
          bottom panel)', + '
          NavigationContenttop panel.insertBefore(
          bottom panel,
          top panel)', '
          top panel.remove()' ]); @@ -2703,6 +2706,96 @@ describe('Fragment', () => { expectDomLogToBe(['
          2.remove()', '
          3.remove()']); }); + it('should properly unmount Fragment children around an unmounting null placeholder #2987', () => { + const arrayOf = (n, fill = 0) => new Array(n).fill(fill); + + class App extends Component { + constructor(props) { + super(props); + + this.state = { + renderFirstElement: true, + renderLastElement: true, + childrenProps: [ + { + arrayData: arrayOf(10).map((_, index) => ({ + key: index + 5, + text: index + 5 + })) + }, + { + arrayData: arrayOf(10).map((_, index) => ({ + key: index + 15, + text: index + 15 + })) + } + ] + }; + } + + componentDidMount() { + // eslint-disable-next-line react/no-did-mount-set-state + this.setState({ + renderFirstElement: false, + renderLastElement: true, + childrenProps: [ + { + arrayData: arrayOf(15).map((_, index) => ({ + key: index, + text: index + })) + }, + { + arrayData: arrayOf(5).map((_, index) => ({ + key: index + 15, + text: index + 15 + })) + } + ] + }); + } + + render() { + const { renderFirstElement, renderLastElement, childrenProps } = + this.state; + return ( +
          + {renderFirstElement &&
          This is the first div
          } + {childrenProps.map(({ arrayData }) => { + return arrayData.map(({ key, text }) => ( +
          {text}
          + )); + })} + {renderLastElement &&
          This is the last div
          } +
          + ); + } + } + + render(, scratch); + expect(scratch.innerHTML).to.equal( + div([ + div('This is the first div'), + arrayOf(20) + .map((_, i) => div(i + 5)) // 5 - 24 + .join(''), + div('This is the last div') + ]) + ); + + // Flush CDM setState call + rerender(); + expect(scratch.innerHTML).to.equal( + div([ + // "This is the first div" is unmounted using a null placeholder pattern + arrayOf(20) + .map((_, i) => div(i)) // 0 - 19 (0 - 4 are inserted, 20 - 24 are unmounted) + .join(''), + div('This is the last div') + ]) + ); + }); + it('should efficiently place new children and unmount nested Fragment children', () => { //
          4 is added and Fragment sibling unmounts. Does
          4 get correct _nextDom pointer? function App({ condition }) { @@ -2952,9 +3045,6 @@ describe('Fragment', () => { rerender(); expect(scratch.innerHTML).to.equal([div('A'), div(1), div(2)].join('')); - expectDomLogToBe([ - '
          B.remove()', - '
          1A2.insertBefore(
          1,
          2)' - ]); + expectDomLogToBe(['
          B.remove()', '
          2A1.appendChild(
          2)']); }); }); diff --git a/test/browser/keys.test.js b/test/browser/keys.test.js index bb99932cf1..118efbcf82 100644 --- a/test/browser/keys.test.js +++ b/test/browser/keys.test.js @@ -1,7 +1,8 @@ -import { createElement, Component, render } from 'preact'; +import { createElement, Component, render, createRef } from 'preact'; import { setupScratch, teardown } from '../_util/helpers'; import { logCall, clearLog, getLog } from '../_util/logCall'; import { div } from '../_util/dom'; +import { setupRerender } from 'preact/test-utils'; /** @jsx createElement */ @@ -9,6 +10,8 @@ describe('keys', () => { /** @type {HTMLDivElement} */ let scratch; + let rerender; + /** @type {string[]} */ let ops; @@ -70,6 +73,7 @@ describe('keys', () => { }); beforeEach(() => { + rerender = setupRerender(); scratch = setupScratch(); ops = []; }); @@ -319,11 +323,7 @@ describe('keys', () => { render(, scratch); expect(scratch.textContent).to.equal('abcd', 'move to beginning'); expect(getLog()).to.deep.equal( - [ - '
            bcda.appendChild(
          1. b)', - '
              cdab.appendChild(
            1. c)', - '
                dabc.appendChild(
              1. d)' - ], + ['
                  bcda.insertBefore(
                1. a,
                2. b)'], 'move to beginning' ); }); @@ -341,15 +341,15 @@ describe('keys', () => { render(, scratch); expect(scratch.textContent).to.equal(values.join('')); expect(getLog()).to.deep.equal([ - '
                    abcdefghij.appendChild(
                  1. i)', - '
                      abcdefghji.appendChild(
                    1. h)', - '
                        abcdefgjih.appendChild(
                      1. g)', - '
                          abcdefjihg.appendChild(
                        1. f)', - '
                            abcdejihgf.appendChild(
                          1. e)', - '
                              abcdjihgfe.appendChild(
                            1. d)', - '
                                abcjihgfed.appendChild(
                              1. c)', - '
                                  abjihgfedc.appendChild(
                                1. b)', - '
                                    ajihgfedcb.appendChild(
                                  1. a)' + '
                                      abcdefghij.insertBefore(
                                    1. j,
                                    2. a)', + '
                                        jabcdefghi.insertBefore(
                                      1. i,
                                      2. a)', + '
                                          jiabcdefgh.insertBefore(
                                        1. h,
                                        2. a)', + '
                                            jihabcdefg.insertBefore(
                                          1. g,
                                          2. a)', + '
                                              jihgabcdef.insertBefore(
                                            1. f,
                                            2. a)', + '
                                                jihgfabcde.insertBefore(
                                              1. e,
                                              2. a)', + '
                                                  jihgfeabcd.insertBefore(
                                                1. d,
                                                2. a)', + '
                                                    jihgfedabc.insertBefore(
                                                  1. c,
                                                  2. a)', + '
                                                      jihgfedcab.appendChild(
                                                    1. a)' ]); }); @@ -558,6 +558,130 @@ describe('keys', () => { expect(Stateful2Ref).to.equal(Stateful2MovedRef); }); + it('should effectively iterate on large lists', done => { + const newItems = () => + Array(100) + .fill(0) + .map((item, i) => i); + + let set, + mutatedNodes = []; + + class App extends Component { + constructor(props) { + super(props); + this.state = { items: newItems() }; + set = this.set = this.set.bind(this); + this.ref = createRef(); + } + + componentDidMount() { + const observer = new MutationObserver(listener); + observer.observe(this.ref.current, { childList: true }); + + function listener(mutations) { + for (const { addedNodes } of mutations) { + for (const node of addedNodes) { + mutatedNodes.push(node); + } + } + } + } + + set() { + const currentItems = this.state.items; + const items = newItems().filter(id => { + const isVisible = currentItems.includes(id); + return id >= 20 && id <= 80 ? !isVisible : isVisible; + }); + this.setState({ items }); + } + + render() { + return ( +
                                                      + {this.state.items.map(i => ( +
                                                      {i}
                                                      + ))} +
                                                      + ); + } + } + + render(, scratch); + + set(); + rerender(); + + setTimeout(() => { + expect(mutatedNodes.length).to.equal(0); + done(); + }); + }); + + it('should effectively iterate on large component lists', done => { + const newItems = () => + Array(100) + .fill(0) + .map((item, i) => i); + + let set, + mutatedNodes = []; + + const Row = ({ i }) =>

                                                      {i}

                                                      ; + + class App extends Component { + constructor(props) { + super(props); + this.state = { items: newItems() }; + set = this.set = this.set.bind(this); + this.ref = createRef(); + } + + componentDidMount() { + const observer = new MutationObserver(listener); + observer.observe(this.ref.current, { childList: true }); + + function listener(mutations) { + for (const { addedNodes } of mutations) { + for (const node of addedNodes) { + mutatedNodes.push(node); + } + } + } + } + + set() { + const currentItems = this.state.items; + const items = newItems().filter(id => { + const isVisible = currentItems.includes(id); + return id >= 20 && id <= 80 ? !isVisible : isVisible; + }); + this.setState({ items }); + } + + render() { + return ( +
                                                      + {this.state.items.map(i => ( + + ))} +
                                                      + ); + } + } + + render(, scratch); + + set(); + rerender(); + + setTimeout(() => { + expect(mutatedNodes.length).to.equal(0); + done(); + }); + }); + it('should not preserve state when switching between keyed and unkeyed components as children', () => { // React & Preact v8 behavior: https://codesandbox.io/s/8l3p6lz9kj diff --git a/test/browser/refs.test.js b/test/browser/refs.test.js index a5962ea500..7e4224af57 100644 --- a/test/browser/refs.test.js +++ b/test/browser/refs.test.js @@ -1,5 +1,5 @@ import { setupRerender } from 'preact/test-utils'; -import { createElement, render, Component, createRef } from 'preact'; +import { createElement, render, Component, createRef, Fragment } from 'preact'; import { setupScratch, teardown } from '../_util/helpers'; /** @jsx createElement */ @@ -103,8 +103,8 @@ describe('refs', () => { 'called with H1', 'called with DIV', 'called with null', - 'called with H1', 'called with null', + 'called with H1', 'called with DIV' ]); }); @@ -509,25 +509,23 @@ describe('refs', () => { expect(ref.current).to.equal(scratch.firstChild.firstChild); }); - // TODO - it.skip('should properly call null for memoized components keyed', () => { - const calls = []; + it('should properly call null for memoized components keyed', () => { + let calls = []; const element =
                                                      calls.push(x)}>hey
                                                      ; function App(props) { return
                                                      {element}
                                                      ; } render(, scratch); + expect(calls).to.deep.equal([scratch.firstChild.firstChild]); + calls = []; + render(, scratch); + expect(calls).to.deep.equal([null, scratch.firstChild.firstChild]); + calls = []; + render(, scratch); - expect(calls.length).to.equal(5); - expect(calls).to.deep.equal([ - scratch.firstChild.firstChild, - null, - scratch.firstChild.firstChild, - null, - scratch.firstChild.firstChild - ]); + expect(calls).to.deep.equal([null, scratch.firstChild.firstChild]); }); it('should properly call null for memoized components unkeyed', () => { @@ -543,4 +541,178 @@ describe('refs', () => { expect(calls.length).to.equal(1); expect(calls[0]).to.equal(scratch.firstChild.firstChild); }); + + // Test for #4049 + it('should first clean-up refs and after apply them', () => { + let calls = []; + let set; + class App extends Component { + constructor(props) { + super(props); + this.state = { + phase: 1 + }; + set = () => this.setState({ phase: 2 }); + } + + render(props, { phase }) { + return ( + + {phase === 1 ? ( +
                                                      +
                                                      + r + ? calls.push('adding ref to two') + : calls.push('removing ref from two') + } + > + Element two +
                                                      +
                                                      + r + ? calls.push('adding ref to three') + : calls.push('removing ref from three') + } + > + Element three +
                                                      +
                                                      + ) : phase === 2 ? ( +
                                                      +
                                                      + r + ? calls.push('adding ref to one') + : calls.push('removing ref from one') + } + > + Element one +
                                                      +
                                                      +
                                                      + r + ? calls.push('adding ref to two') + : calls.push('removing ref from two') + } + > + Element two +
                                                      +
                                                      + r + ? calls.push('adding ref to three') + : calls.push('removing ref from three') + } + > + Element three +
                                                      +
                                                      +
                                                      + ) : null} +
                                                      + ); + } + } + + render(, scratch); + + expect(calls).to.deep.equal(['adding ref to two', 'adding ref to three']); + calls = []; + + set(); + rerender(); + expect(calls).to.deep.equal([ + 'removing ref from two', + 'adding ref to one', + 'adding ref to two', + 'adding ref to three' + ]); + }); + + it('should bind refs before componentDidMount', () => { + /** @type {import('preact').RefObject[]} */ + const refs = []; + + class Parent extends Component { + componentDidMount() { + // Child refs should be set + expect(refs.length).to.equal(2); + expect(refs[0].current.tagName).to.equal('SPAN'); + expect(refs[1].current.tagName).to.equal('SPAN'); + } + + render(props) { + return props.children; + } + } + + class Child extends Component { + constructor(props) { + super(props); + + this.ref = createRef(); + } + + componentDidMount() { + // SPAN refs should be set + expect(this.ref.current.tagName).to.equal('SPAN'); + expect(document.body.contains(this.ref.current)).to.equal(true); + refs.push(this.ref); + } + + render() { + return Hello; + } + } + + render( + + + + , + scratch + ); + + expect(scratch.innerHTML).to.equal('HelloHello'); + expect(refs[0].current).to.equalNode(scratch.firstChild); + expect(refs[1].current).to.equalNode(scratch.lastChild); + }); + + it('should call refs after element is added to document on initial mount', () => { + const verifyRef = name => el => + expect(document.body.contains(el), name).to.equal(true); + + function App() { + return ( +
                                                      +

                                                      Hello

                                                      +
                                                      + ); + } + + render(, scratch); + }); + + it('should call refs after element is added to document on update', () => { + const verifyRef = name => el => + expect(document.body.contains(el), name).to.equal(true); + + function App({ show = false }) { + return ( +
                                                      + {show && ( +

                                                      + Hello +

                                                      + )} +
                                                      + ); + } + + render(, scratch); + render(, scratch); + }); }); diff --git a/test/browser/render.test.js b/test/browser/render.test.js index dc04ca3011..308f768185 100644 --- a/test/browser/render.test.js +++ b/test/browser/render.test.js @@ -1061,6 +1061,51 @@ describe('render()', () => { expect(scratch.textContent).to.equal('01'); }); + it('should not remove iframe', () => { + let setState; + const Iframe = () => { + return
          ' + ); + clearLog(); + setState({ value: false }); + rerender(); + + expect(scratch.innerHTML).to.equal( + '
          ' + ); + expect(getLog()).to.deep.equal([ + '
          Test2.remove()', + '
          Test3.remove()' + ]); + }); + it('should not cause infinite loop with referentially equal props', () => { let i = 0; let prevDiff = options._diff;