diff --git a/benches/src/util.js b/benches/src/util.js index 1a596114ef..fe2dbef9c5 100644 --- a/benches/src/util.js +++ b/benches/src/util.js @@ -19,6 +19,12 @@ export function afterFrameAsync() { return promise; } +const majorTask = () => + new Promise(resolve => { + window.addEventListener('message', resolve, { once: true }); + window.postMessage('major task delay', '*'); + }); + let count = 0; const channel = new MessageChannel(); const callbacks = new Map(); @@ -64,10 +70,20 @@ export async function mutateAndLayout(mutation, times = 1) { await forceLayout(); } -export function measureMemory() { +export async function measureMemory() { if ('gc' in window && 'memory' in performance) { // Report results in MBs + performance.mark('gc-start'); window.gc(); + performance.measure('gc', 'gc-start'); + + // window.gc synchronously triggers one Major GC. However that MajorGC + // asynchronously triggers additional MajorGCs until the + // usedJSHeapSizeBefore and usedJSHeapSizeAfter are the same. Here, we'll + // wait a moment for some (hopefully all) additional GCs to finish before + // measuring the memory. + await majorTask(); + performance.mark('measure-memory'); window.usedJSHeapSize = performance.memory.usedJSHeapSize / 1e6; } else { window.usedJSHeapSize = 0; diff --git a/compat/mangle.json b/compat/mangle.json index 8352f38e63..506a6a41c2 100644 --- a/compat/mangle.json +++ b/compat/mangle.json @@ -18,4 +18,4 @@ } } } -} +} \ No newline at end of file diff --git a/compat/src/render.js b/compat/src/render.js index 13cc7fc9c0..724ef0cdd7 100644 --- a/compat/src/render.js +++ b/compat/src/render.js @@ -11,7 +11,7 @@ import { IS_NON_DIMENSIONAL } from './util'; export const REACT_ELEMENT_TYPE = Symbol.for('react.element'); const CAMEL_PROPS = - /^(?:accent|alignment|arabic|baseline|cap|clip(?!PathU)|color|dominant|fill|flood|font|glyph(?!R)|horiz|image|letter|lighting|marker(?!H|W|U)|overline|paint|pointer|shape|stop|strikethrough|stroke|text(?!L)|transform|underline|unicode|units|v|vector|vert|word|writing|x(?!C))[A-Z]/; + /^(?:accent|alignment|arabic|baseline|cap|clip(?!PathU)|color|dominant|fill|flood|font|glyph(?!R)|horiz|image(!S)|letter|lighting|marker(?!H|W|U)|overline|paint|pointer|shape|stop|strikethrough|stroke|text(?!L)|transform|underline|unicode|units|v|vector|vert|word|writing|x(?!C))[A-Z]/; const IS_DOM = typeof document !== 'undefined'; const ON_ANI = /^on(Ani|Tra|Tou|BeforeInp|Compo)/; const CAMEL_REPLACE = /[A-Z0-9]/g; diff --git a/compat/test/browser/cloneElement.test.js b/compat/test/browser/cloneElement.test.js index f3e4f78d97..1b00bb594a 100644 --- a/compat/test/browser/cloneElement.test.js +++ b/compat/test/browser/cloneElement.test.js @@ -1,5 +1,10 @@ import { createElement as preactH } from 'preact'; -import React, { createElement, render, cloneElement } from 'preact/compat'; +import React, { + createElement, + render, + cloneElement, + Component +} from 'preact/compat'; import { setupScratch, teardown } from '../../../test/_util/helpers'; describe('compat cloneElement', () => { @@ -93,4 +98,17 @@ describe('compat cloneElement', () => { render(clone, scratch); expect(scratch.textContent).to.equal('foo'); }); + + it('should prevent undefined properties from overriding default props', () => { + class Example extends Component { + render(props) { + return
thing
; + } + } + Example.defaultProps = { color: 'blue' }; + + const element = ; + const clone = cloneElement(element, { color: undefined }); + expect(clone.props.color).to.equal('blue'); + }); }); diff --git a/compat/test/browser/render.test.js b/compat/test/browser/render.test.js index 46d535d00b..3c0fa51208 100644 --- a/compat/test/browser/render.test.js +++ b/compat/test/browser/render.test.js @@ -495,6 +495,21 @@ describe('compat render', () => { ); }); + it('shouldnot transform imageSrcSet', () => { + render( + , + scratch + ); + expect(scratch.innerHTML).to.equal( + '' + ); + }); + it("should support react-relay's usage of __SECRET_INTERNALS_DO_NOT_USE_OR_YOU_WILL_BE_FIRED", () => { const Ctx = createContext('foo'); diff --git a/debug/mangle.json b/debug/mangle.json index 8352f38e63..506a6a41c2 100644 --- a/debug/mangle.json +++ b/debug/mangle.json @@ -18,4 +18,4 @@ } } } -} +} \ No newline at end of file diff --git a/debug/package.json b/debug/package.json index 633c9bf3dc..771905f1b2 100644 --- a/debug/package.json +++ b/debug/package.json @@ -17,6 +17,7 @@ }, "exports": { ".": { + "types": "./src/index.d.ts", "browser": "./dist/debug.mjs", "umd": "./dist/debug.umd.js", "import": "./dist/debug.mjs", diff --git a/debug/src/debug.js b/debug/src/debug.js index 942e92ca89..50f273e3ab 100644 --- a/debug/src/debug.js +++ b/debug/src/debug.js @@ -41,7 +41,7 @@ export function initDebug() { useEffect: new WeakMap(), useLayoutEffect: new WeakMap(), lazyPropTypes: new WeakMap() - }; + }; const deprecations = []; options._catchError = (error, vnode, oldVNode) => { @@ -341,15 +341,15 @@ export function initDebug() { if (oldVnode) oldVnode(vnode); }; - options.diffed = vnode => { + options.diffed = internal => { hooksAllowed = false; - if (oldDiffed) oldDiffed(vnode); + if (oldDiffed) oldDiffed(internal); - if (vnode._children != null) { + if (internal._children != null) { const keys = []; - for (let i = 0; i < vnode._children.length; i++) { - const child = vnode._children[i]; + for (let i = 0; i < internal._children.length; i++) { + const child = internal._children[i]; if (!child || child.key == null) continue; const key = child.key; @@ -358,8 +358,8 @@ export function initDebug() { 'Following component has two or more children with the ' + `same key attribute: "${key}". This may cause glitches and misbehavior ` + 'in rendering process. Component: \n\n' + - serializeVNode(vnode) + - `\n\n${getOwnerStack(vnode)}` + serializeVNode(internal) + + `\n\n${getOwnerStack(internal)}` ); // Break early to not spam the console @@ -369,6 +369,27 @@ export function initDebug() { keys.push(key); } } + + if (internal.data && internal.data.__hooks != null) { + // Validate that none of the hooks in this component contain arguments that are NaN. + // This is a common mistake that can be hard to debug, so we want to catch it early. + const hooks = internal.data.__hooks._list; + if (hooks) { + for (let i = 0; i < hooks.length; i += 1) { + const hook = hooks[i]; + if (hook._args) { + for (const arg of hook._args) { + if (Number.isNaN(arg)) { + const componentName = getDisplayName(internal); + throw new Error( + `Invalid argument passed to hook. Hooks should not be called with NaN in the dependency array. Hook index ${i} in component ${componentName} was called with NaN.` + ); + } + } + } + } + } + } }; } diff --git a/debug/test/browser/validateHookArgs.test.js b/debug/test/browser/validateHookArgs.test.js new file mode 100644 index 0000000000..19f7cb7fd3 --- /dev/null +++ b/debug/test/browser/validateHookArgs.test.js @@ -0,0 +1,74 @@ +import { createElement, render, createRef } from 'preact'; +import { + useState, + useEffect, + useLayoutEffect, + useCallback, + useMemo, + useImperativeHandle +} from 'preact/hooks'; +import { setupRerender } from 'preact/test-utils'; +import { setupScratch, teardown } from '../../../test/_util/helpers'; +import 'preact/debug'; + +/** @jsx createElement */ + +describe('Hook argument validation', () => { + /** + * @param {string} name + * @param {(arg: number) => void} hook + */ + function validateHook(name, hook) { + const TestComponent = ({ initialValue }) => { + const [value, setValue] = useState(initialValue); + hook(value); + + return ( + + ); + }; + + it(`should error if ${name} is mounted with NaN as an argument`, async () => { + expect(() => + render(, scratch) + ).to.throw(/Hooks should not be called with NaN in the dependency array/); + }); + + it(`should error if ${name} is updated with NaN as an argument`, async () => { + render(, scratch); + + expect(() => { + scratch.querySelector('button').click(); + rerender(); + }).to.throw( + /Hooks should not be called with NaN in the dependency array/ + ); + }); + } + + /** @type {HTMLElement} */ + let scratch; + /** @type {() => void} */ + let rerender; + + beforeEach(() => { + scratch = setupScratch(); + rerender = setupRerender(); + }); + + afterEach(() => { + teardown(scratch); + }); + + validateHook('useEffect', arg => useEffect(() => {}, [arg])); + validateHook('useLayoutEffect', arg => useLayoutEffect(() => {}, [arg])); + validateHook('useCallback', arg => useCallback(() => {}, [arg])); + validateHook('useMemo', arg => useMemo(() => {}, [arg])); + + const ref = createRef(); + validateHook('useImperativeHandle', arg => { + useImperativeHandle(ref, () => undefined, [arg]); + }); +}); diff --git a/devtools/mangle.json b/devtools/mangle.json index 8352f38e63..506a6a41c2 100644 --- a/devtools/mangle.json +++ b/devtools/mangle.json @@ -18,4 +18,4 @@ } } } -} +} \ No newline at end of file diff --git a/hooks/mangle.json b/hooks/mangle.json index 8352f38e63..506a6a41c2 100644 --- a/hooks/mangle.json +++ b/hooks/mangle.json @@ -18,4 +18,4 @@ } } } -} +} \ No newline at end of file diff --git a/hooks/test/browser/useImperativeHandle.test.js b/hooks/test/browser/useImperativeHandle.test.js index 24b9e13775..ac71065696 100644 --- a/hooks/test/browser/useImperativeHandle.test.js +++ b/hooks/test/browser/useImperativeHandle.test.js @@ -180,7 +180,7 @@ describe('useImperativeHandle', () => { expect(() => render(, scratch)).to.not.throw(); }); - it('should reset ref to null when the component get unmounted', () => { + it('should reset ref object to null when the component get unmounted', () => { let ref, createHandleSpy = sinon.spy(() => ({ test: () => 'test' })); @@ -198,4 +198,25 @@ describe('useImperativeHandle', () => { expect(createHandleSpy).to.have.been.calledOnce; expect(ref.current).to.equal(null); }); + + it('should reset ref callback to null when the component get unmounted', () => { + const ref = sinon.spy(); + const handle = { test: () => 'test' }; + const createHandleSpy = sinon.spy(() => handle); + + function Comp() { + useImperativeHandle(ref, createHandleSpy, [1]); + return

Test

; + } + + render(, scratch); + expect(createHandleSpy).to.have.been.calledOnce; + expect(ref).to.have.been.calledWith(handle); + + ref.resetHistory(); + + render(
, scratch); + expect(createHandleSpy).to.have.been.calledOnce; + expect(ref).to.have.been.calledWith(null); + }); }); diff --git a/jsx-runtime/mangle.json b/jsx-runtime/mangle.json index 8352f38e63..506a6a41c2 100644 --- a/jsx-runtime/mangle.json +++ b/jsx-runtime/mangle.json @@ -18,4 +18,4 @@ } } } -} +} \ No newline at end of file diff --git a/package.json b/package.json index 461142e2ac..158500c80f 100644 --- a/package.json +++ b/package.json @@ -95,6 +95,16 @@ "import": "./compat/server.mjs", "require": "./compat/server.js" }, + "./compat/jsx-runtime": { + "types": "./jsx-runtime/src/index.d.ts", + "import": "./compat/jsx-runtime.mjs", + "require": "./compat/jsx-runtime.js" + }, + "./compat/jsx-dev-runtime": { + "types": "./jsx-runtime/src/index.d.ts", + "import": "./compat/jsx-dev-runtime.mjs", + "require": "./compat/jsx-dev-runtime.js" + }, "./package.json": "./package.json", "./compat/package.json": "./compat/package.json", "./debug/package.json": "./debug/package.json", diff --git a/src/create-element.js b/src/create-element.js index ce1f9a3171..b6be79c424 100644 --- a/src/create-element.js +++ b/src/create-element.js @@ -1,5 +1,6 @@ import { UNDEFINED } from './constants'; import options from './options'; +import { isArray } from './util'; let vnodeId = 0; @@ -65,7 +66,7 @@ export function normalizeToVNode(childVNode) { } if (type == 'object' || type == 'function') { - if (Array.isArray(childVNode)) { + if (isArray(childVNode)) { return createElement(Fragment, null, childVNode); } diff --git a/src/diff/children.js b/src/diff/children.js index 4eabaf8b7e..fb402cdca2 100644 --- a/src/diff/children.js +++ b/src/diff/children.js @@ -12,6 +12,7 @@ import { mount } from './mount'; import { patch } from './patch'; import { unmount } from './unmount'; import { createInternal, getDomSibling } from '../tree'; +import { isArray } from '../util'; /** * Update an internal with new children. @@ -274,7 +275,7 @@ export function insertComponentDom(internal, nextSibling, parentDom) { export function toChildArray(children, out) { out = out || []; if (children == null || typeof children == 'boolean') { - } else if (Array.isArray(children)) { + } else if (isArray(children)) { for (children of children) { toChildArray(children, out); } diff --git a/src/diff/mount.js b/src/diff/mount.js index 81162f518b..632cb5b7bb 100644 --- a/src/diff/mount.js +++ b/src/diff/mount.js @@ -19,6 +19,7 @@ import { createInternal, getParentContext } from '../tree'; import options from '../options'; import { ENABLE_CLASSES } from '../component'; import { commitQueue } from './commit'; +import { isArray } from '../util'; /** * Diff two virtual nodes and apply proper changes to the DOM * @param {import('../internal').Internal} internal The Internal node to mount @@ -201,7 +202,7 @@ function mountElement(internal, dom, refs) { } else if (newChildren != null) { mountChildren( internal, - Array.isArray(newChildren) ? newChildren : [newChildren], + isArray(newChildren) ? newChildren : [newChildren], dom, isNew ? null : dom.firstChild, refs @@ -409,7 +410,7 @@ function mountComponent(internal, startDom) { if (renderResult.type === Fragment && renderResult.key == null) { renderResult = renderResult.props.children; } - if (!Array.isArray(renderResult)) { + if (!isArray(renderResult)) { renderResult = [renderResult]; } } else { diff --git a/src/diff/patch.js b/src/diff/patch.js index aa1c9cbc4d..e6c4101aee 100644 --- a/src/diff/patch.js +++ b/src/diff/patch.js @@ -23,6 +23,7 @@ import { mountChildren } from './mount'; import { Fragment } from '../create-element'; import { ENABLE_CLASSES } from '../component'; import { commitQueue } from './commit'; +import { isArray } from '../util'; /** * Diff two virtual nodes and apply proper changes to the DOM @@ -182,7 +183,7 @@ function patchElement(internal, vnode) { if (oldHtml) dom.innerHTML = ''; patchChildren( internal, - newChildren && Array.isArray(newChildren) ? newChildren : [newChildren], + newChildren && isArray(newChildren) ? newChildren : [newChildren], dom ); } @@ -311,7 +312,7 @@ function patchComponent(internal, newVNode) { if (renderResult.type === Fragment && renderResult.key == null) { renderResult = renderResult.props.children; } - if (!Array.isArray(renderResult)) { + if (!isArray(renderResult)) { renderResult = [renderResult]; } } else { diff --git a/src/diff/props.js b/src/diff/props.js index bbf0ed62b9..a756e71afc 100644 --- a/src/diff/props.js +++ b/src/diff/props.js @@ -78,6 +78,8 @@ export function setProperty(dom, name, value, oldValue, isSvg) { // cast to `0` instead name !== 'tabIndex' && name !== 'download' && + name !== 'rowSpan' && + name !== 'colSpan' && name in dom ) { try { diff --git a/src/index.d.ts b/src/index.d.ts index 4de256f9cb..8de5c40a4f 100644 --- a/src/index.d.ts +++ b/src/index.d.ts @@ -4,6 +4,22 @@ import { JSXInternal } from './jsx'; export import JSX = JSXInternal; +// Public interface of the internal backing-node +interface Internal

{ + type: string | ComponentType

; + /** The props object for Elements/Components, and the string contents for Text */ + props: (P & { children: ComponentChildren }) | string | number; + key: any; + ref: Ref | null; + + /** Bitfield containing information about the Internal or its component. */ + flags: number; + /** Polymorphic property to store extensions like hooks on */ + data: object | Element | Text; + /** The function that triggers in-place re-renders for an internal */ + rerender: (internal: Internal) => void; +} + // // Preact Virtual DOM // ----------------------------------- @@ -311,9 +327,9 @@ export interface Options { /** Attach a hook that is invoked whenever a VNode is created. */ vnode?(vnode: VNode): void; /** Attach a hook that is invoked immediately before a vnode is unmounted. */ - unmount?(vnode: VNode): void; + unmount?(vnode: Internal): void; /** Attach a hook that is invoked after a vnode has rendered. */ - diffed?(vnode: VNode): void; + diffed?(vnode: Internal): void; event?(e: Event): any; requestAnimationFrame?(callback: () => void): void; debounceRendering?(cb: () => void): void; diff --git a/src/jsx.d.ts b/src/jsx.d.ts index afa715b302..712c327fa0 100644 --- a/src/jsx.d.ts +++ b/src/jsx.d.ts @@ -1713,7 +1713,12 @@ export namespace JSXInternal { | string | undefined | SignalLike; - hidden?: boolean | undefined | SignalLike; + hidden?: + | boolean + | 'hidden' + | 'until-found' + | undefined + | SignalLike; high?: number | undefined | SignalLike; href?: string | undefined | SignalLike; hrefLang?: string | undefined | SignalLike; @@ -1722,7 +1727,8 @@ export namespace JSXInternal { httpEquiv?: string | undefined | SignalLike; icon?: string | undefined | SignalLike; id?: string | undefined | SignalLike; - indeterminate?: boolean | undefined | SignalLike; + indeterminate?: boolean | undefined | SignalLike; + inert?: boolean | undefined | SignalLike; inputMode?: string | undefined | SignalLike; integrity?: string | undefined | SignalLike; is?: string | undefined | SignalLike; diff --git a/src/util.js b/src/util.js new file mode 100644 index 0000000000..3d67de5c37 --- /dev/null +++ b/src/util.js @@ -0,0 +1 @@ +export const isArray = Array.isArray; diff --git a/test/browser/render.test.js b/test/browser/render.test.js index 84fa64e9aa..6fcb0dc355 100644 --- a/test/browser/render.test.js +++ b/test/browser/render.test.js @@ -1244,4 +1244,46 @@ describe('render()', () => { expect(items[0]).to.have.property('parentNode').that.should.exist; expect(items[1]).to.have.property('parentNode').that.should.exist; }); + + // Test for #3969 + it('should clear rowspan and colspan', () => { + let update; + class App extends Component { + constructor(props) { + super(props); + this.state = { active: true }; + update = this.setState.bind(this); + } + + render() { + return ( +

+ {this.state.active ? ( + + + + +
+ Foo +
+ ) : ( + + + + +
Foo
+ )} +
+ ); + } + } + + render(, scratch); + + update({ active: false }); + rerender(); + + expect(scratch.querySelector('td[rowspan]')).to.equal(null); + expect(scratch.querySelector('td[colspan]')).to.equal(null); + }); });