From 0c2d9eee77bafc39dec925dc29e32b47811d1cac Mon Sep 17 00:00:00 2001 From: jdecroock Date: Fri, 2 Sep 2022 12:22:08 +0200 Subject: [PATCH 1/7] backport #3615 --- compat/src/render.js | 4 ++-- compat/test/browser/svg.test.js | 11 ++++++++++- 2 files changed, 12 insertions(+), 3 deletions(-) diff --git a/compat/src/render.js b/compat/src/render.js index 4c7456ad1f..7d850cace8 100644 --- a/compat/src/render.js +++ b/compat/src/render.js @@ -10,7 +10,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|marker(?!H|W|U)|overline|paint|shape|stop|strikethrough|stroke|text(?!L)|underline|unicode|units|v|vector|vert|word|writing|x(?!C))[A-Z]/; +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]/; const IS_DOM = typeof document !== 'undefined'; // type="file|checkbox|radio". @@ -161,7 +161,7 @@ options.vnode = vnode => { } else if (/^on(Ani|Tra|Tou|BeforeInp|Compo)/.test(i)) { i = i.toLowerCase(); } else if (nonCustomElement && CAMEL_PROPS.test(i)) { - i = i.replace(/[A-Z0-9]/, '-$&').toLowerCase(); + i = i.replace(/[A-Z0-9]/g, '-$&').toLowerCase(); } else if (value === null) { value = undefined; } diff --git a/compat/test/browser/svg.test.js b/compat/test/browser/svg.test.js index 532ccb3e78..355f7877c2 100644 --- a/compat/test/browser/svg.test.js +++ b/compat/test/browser/svg.test.js @@ -73,13 +73,22 @@ describe('svg', () => { clipPath="value" clipRule="value" clipPathUnits="value" + colorInterpolationFilters="auto" + fontSizeAdjust="value" glyphOrientationHorizontal="value" + glyphOrientationVertical="value" shapeRendering="crispEdges" glyphRef="value" + horizAdvX="value" + horizOriginX="value" markerStart="value" markerHeight="value" markerUnits="value" markerWidth="value" + unitsPerEm="value" + vertAdvY="value" + vertOriginX="value" + vertOriginY="value" x1="value" xChannelSelector="value" />, @@ -88,7 +97,7 @@ describe('svg', () => { expect(serializeHtml(scratch)).to.eql( sortAttributes( - '' + '' ) ); }); From ae87b44e3326651f6cde079e52fa0b5327600c94 Mon Sep 17 00:00:00 2001 From: jdecroock Date: Fri, 2 Sep 2022 12:24:01 +0200 Subject: [PATCH 2/7] backport #3633 --- compat/src/index.js | 2 +- compat/test/browser/hooks.test.js | 30 ++++++++++++++++++++++++++++++ 2 files changed, 31 insertions(+), 1 deletion(-) diff --git a/compat/src/index.js b/compat/src/index.js index ba2ea905b3..8d2bd16140 100644 --- a/compat/src/index.js +++ b/compat/src/index.js @@ -145,7 +145,7 @@ export function useSyncExternalStore(subscribe, getSnapshot) { useEffect(() => { return subscribe(() => { - setState(getSnapshot()); + setState(() => getSnapshot()); }); }, [subscribe, getSnapshot]); diff --git a/compat/test/browser/hooks.test.js b/compat/test/browser/hooks.test.js index f501ae7d91..cdf102c992 100644 --- a/compat/test/browser/hooks.test.js +++ b/compat/test/browser/hooks.test.js @@ -127,5 +127,35 @@ describe('React-18-hooks', () => { expect(scratch.innerHTML).to.equal('

hello new world

'); }); + + it('should not call function values on subscription', () => { + let flush; + const subscribe = sinon.spy(cb => { + flush = cb; + return () => {}; + }); + + let i = 0; + const getSnapshot = sinon.spy(() => { + return () => 'value: ' + i++; + }); + + const App = () => { + const value = useSyncExternalStore(subscribe, getSnapshot); + return

{value()}

; + }; + + act(() => { + render(, scratch); + }); + expect(scratch.innerHTML).to.equal('

value: 0

'); + expect(subscribe).to.be.calledOnce; + expect(getSnapshot).to.be.calledOnce; + + flush(); + rerender(); + + expect(scratch.innerHTML).to.equal('

value: 1

'); + }); }); }); From 176bd3cb201245f70013345071bc106ba0100a99 Mon Sep 17 00:00:00 2001 From: jdecroock Date: Fri, 2 Sep 2022 12:25:18 +0200 Subject: [PATCH 3/7] backport #3643 --- hooks/src/index.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/hooks/src/index.js b/hooks/src/index.js index 100ae43c37..8149f92167 100644 --- a/hooks/src/index.js +++ b/hooks/src/index.js @@ -14,7 +14,7 @@ let previousInternal; /** @type {number} */ let currentHook = 0; -/** @type {Array} */ +/** @type {Array} */ let afterPaintEffects = []; let EMPTY = []; @@ -189,7 +189,6 @@ export function useReducer(reducer, initialState, init) { return currentValue !== hookState._value[0]; }; - } return hookState._nextValue || hookState._value; @@ -359,6 +358,7 @@ export function useErrorBoundary(cb) { function flushAfterPaintEffects() { let internal; while ((internal = afterPaintEffects.shift())) { + if (!internal.data.__hooks) continue; if (~internal.flags & MODE_UNMOUNTING) { try { internal.data.__hooks._pendingEffects.forEach(invokeCleanup); From 4ea4f935ac8b1ca11334a292e577ab92dac77735 Mon Sep 17 00:00:00 2001 From: jdecroock Date: Fri, 2 Sep 2022 12:26:59 +0200 Subject: [PATCH 4/7] backport #3655 --- compat/src/index.js | 9 +++++-- compat/test/browser/hooks.test.js | 43 ++++++++++++++++++++++++++----- 2 files changed, 44 insertions(+), 8 deletions(-) diff --git a/compat/src/index.js b/compat/src/index.js index 8d2bd16140..b3407574f3 100644 --- a/compat/src/index.js +++ b/compat/src/index.js @@ -140,8 +140,13 @@ export const useInsertionEffect = useLayoutEffect; export function useSyncExternalStore(subscribe, getSnapshot) { const [state, setState] = useState(getSnapshot); - // TODO: in suspense for data we could have a discrepancy here because Preact won't re-init the "useState" - // when this unsuspends which could lead to stale state as the subscription is torn down. + const value = getSnapshot(); + + useLayoutEffect(() => { + if (value !== state) { + setState(() => value); + } + }, [subscribe, value, getSnapshot]); useEffect(() => { return subscribe(() => { diff --git a/compat/test/browser/hooks.test.js b/compat/test/browser/hooks.test.js index cdf102c992..825cb07509 100644 --- a/compat/test/browser/hooks.test.js +++ b/compat/test/browser/hooks.test.js @@ -4,7 +4,9 @@ import React, { useInsertionEffect, useSyncExternalStore, useTransition, - render + render, + useState, + useCallback } from 'preact/compat'; import { setupRerender, act } from 'preact/test-utils'; import { setupScratch, teardown } from '../../../test/_util/helpers'; @@ -91,7 +93,7 @@ describe('React-18-hooks', () => { }); expect(scratch.innerHTML).to.equal('

hello world

'); expect(subscribe).to.be.calledOnce; - expect(getSnapshot).to.be.calledOnce; + expect(getSnapshot).to.be.calledTwice; }); it('subscribes and rerenders when called', () => { @@ -119,7 +121,7 @@ describe('React-18-hooks', () => { }); expect(scratch.innerHTML).to.equal('

hello world

'); expect(subscribe).to.be.calledOnce; - expect(getSnapshot).to.be.calledOnce; + expect(getSnapshot).to.be.calledTwice; called = true; flush(); @@ -135,9 +137,11 @@ describe('React-18-hooks', () => { return () => {}; }); + const func = () => 'value: ' + i++; + let i = 0; const getSnapshot = sinon.spy(() => { - return () => 'value: ' + i++; + return func; }); const App = () => { @@ -150,12 +154,39 @@ describe('React-18-hooks', () => { }); expect(scratch.innerHTML).to.equal('

value: 0

'); expect(subscribe).to.be.calledOnce; - expect(getSnapshot).to.be.calledOnce; + expect(getSnapshot).to.be.calledTwice; flush(); rerender(); - expect(scratch.innerHTML).to.equal('

value: 1

'); + expect(scratch.innerHTML).to.equal('

value: 0

'); + }); + + it('works with useCallback', () => { + let toggle; + const App = () => { + const [state, setState] = useState(true); + toggle = setState.bind(this, () => false); + + const value = useSyncExternalStore( + useCallback(() => { + return () => {}; + }, [state]), + () => (state ? 'yep' : 'nope') + ); + + return

{value}

; + }; + + act(() => { + render(, scratch); + }); + expect(scratch.innerHTML).to.equal('

yep

'); + + toggle(); + rerender(); + + expect(scratch.innerHTML).to.equal('

nope

'); }); }); }); From dbed283ea3eba6258123099dc4d3e0ccebf32dd6 Mon Sep 17 00:00:00 2001 From: jdecroock Date: Fri, 2 Sep 2022 12:27:44 +0200 Subject: [PATCH 5/7] backport #3663 --- compat/src/index.js | 29 +++++++++++++++++++------- compat/test/browser/hooks.test.js | 34 ++++++++++++++++++++++++++++--- 2 files changed, 53 insertions(+), 10 deletions(-) diff --git a/compat/src/index.js b/compat/src/index.js index b3407574f3..62eafa5df7 100644 --- a/compat/src/index.js +++ b/compat/src/index.js @@ -137,24 +137,39 @@ export function useTransition() { // styles/... before it attaches export const useInsertionEffect = useLayoutEffect; +/** + * This is taken from https://github.com/facebook/react/blob/main/packages/use-sync-external-store/src/useSyncExternalStoreShimClient.js#L84 + * on a high level this cuts out the warnings, ... and attempts a smaller implementation + */ export function useSyncExternalStore(subscribe, getSnapshot) { - const [state, setState] = useState(getSnapshot); - const value = getSnapshot(); + const [{ _instance }, forceUpdate] = useState({ + _instance: { _value: value, _getSnapshot: getSnapshot } + }); + useLayoutEffect(() => { - if (value !== state) { - setState(() => value); + _instance._value = value; + _instance._getSnapshot = getSnapshot; + + if (_instance._value !== getSnapshot()) { + forceUpdate({ _instance }); } }, [subscribe, value, getSnapshot]); useEffect(() => { + if (_instance._value !== _instance._getSnapshot()) { + forceUpdate({ _instance }); + } + return subscribe(() => { - setState(() => getSnapshot()); + if (_instance._value !== _instance._getSnapshot()) { + forceUpdate({ _instance }); + } }); - }, [subscribe, getSnapshot]); + }, [subscribe]); - return state; + return value; } export * from 'preact/hooks'; diff --git a/compat/test/browser/hooks.test.js b/compat/test/browser/hooks.test.js index 825cb07509..3255704756 100644 --- a/compat/test/browser/hooks.test.js +++ b/compat/test/browser/hooks.test.js @@ -93,7 +93,7 @@ describe('React-18-hooks', () => { }); expect(scratch.innerHTML).to.equal('

hello world

'); expect(subscribe).to.be.calledOnce; - expect(getSnapshot).to.be.calledTwice; + expect(getSnapshot).to.be.calledThrice; }); it('subscribes and rerenders when called', () => { @@ -121,7 +121,7 @@ describe('React-18-hooks', () => { }); expect(scratch.innerHTML).to.equal('

hello world

'); expect(subscribe).to.be.calledOnce; - expect(getSnapshot).to.be.calledTwice; + expect(getSnapshot).to.be.calledThrice; called = true; flush(); @@ -154,7 +154,7 @@ describe('React-18-hooks', () => { }); expect(scratch.innerHTML).to.equal('

value: 0

'); expect(subscribe).to.be.calledOnce; - expect(getSnapshot).to.be.calledTwice; + expect(getSnapshot).to.be.calledThrice; flush(); rerender(); @@ -162,6 +162,34 @@ describe('React-18-hooks', () => { expect(scratch.innerHTML).to.equal('

value: 0

'); }); + it('should work with changing getSnapshot', () => { + let flush; + const subscribe = sinon.spy(cb => { + flush = cb; + return () => {}; + }); + + let i = 0; + const App = () => { + const value = useSyncExternalStore(subscribe, () => { + return i; + }); + return

value: {value}

; + }; + + act(() => { + render(, scratch); + }); + expect(scratch.innerHTML).to.equal('

value: 0

'); + expect(subscribe).to.be.calledOnce; + + i++; + flush(); + rerender(); + + expect(scratch.innerHTML).to.equal('

value: 1

'); + }); + it('works with useCallback', () => { let toggle; const App = () => { From 8af20c0c20c288ce84356198f586e4b84fba7fb9 Mon Sep 17 00:00:00 2001 From: jdecroock Date: Fri, 2 Sep 2022 12:30:35 +0200 Subject: [PATCH 6/7] backport #3690 --- follow-ups.md | 1 + src/index.d.ts | 62 ++++++++++++++++++++++++++++++++++++++++------ test/ts/preact.tsx | 20 ++++++++++++++- 3 files changed, 75 insertions(+), 8 deletions(-) diff --git a/follow-ups.md b/follow-ups.md index 8e87180688..ffbc3d3b97 100644 --- a/follow-ups.md +++ b/follow-ups.md @@ -21,6 +21,7 @@ PR's that weren't backported yet, do they work? - https://github.com/preactjs/preact/pull/3280 Not merged yet need some input - https://github.com/preactjs/preact/pull/3222 Same as above - Make this work https://github.com/preactjs/preact/pull/3306 +- https://github.com/preactjs/preact/pull/3696 ## Root node follow ups diff --git a/src/index.d.ts b/src/index.d.ts index bfbb7bfdf3..0aca917a6e 100644 --- a/src/index.d.ts +++ b/src/index.d.ts @@ -183,11 +183,35 @@ export abstract class Component { // ----------------------------------- export function createElement( + type: 'input', + props: + | (JSXInternal.DOMAttributes & + ClassAttributes) + | null, + ...children: ComponentChildren[] +): VNode; +export function createElement< + P extends JSXInternal.HTMLAttributes, + T extends HTMLElement +>( + type: keyof JSXInternal.IntrinsicElements, + props: (ClassAttributes & P) | null, + ...children: ComponentChildren[] +): VNode; +export function createElement< + P extends JSXInternal.SVGAttributes, + T extends HTMLElement +>( + type: keyof JSXInternal.IntrinsicElements, + props: (ClassAttributes & P) | null, + ...children: ComponentChildren[] +): VNode; +export function createElement( type: string, props: - | (JSXInternal.HTMLAttributes & - JSXInternal.SVGAttributes & - Record) + | (ClassAttributes & + JSXInternal.HTMLAttributes & + JSXInternal.SVGAttributes) | null, ...children: ComponentChildren[] ): VNode; @@ -201,11 +225,35 @@ export namespace createElement { } export function h( + type: 'input', + props: + | (JSXInternal.DOMAttributes & + ClassAttributes) + | null, + ...children: ComponentChildren[] +): VNode; +export function h< + P extends JSXInternal.HTMLAttributes, + T extends HTMLElement +>( + type: keyof JSXInternal.IntrinsicElements, + props: (ClassAttributes & P) | null, + ...children: ComponentChildren[] +): VNode; +export function h< + P extends JSXInternal.SVGAttributes, + T extends HTMLElement +>( + type: keyof JSXInternal.IntrinsicElements, + props: (ClassAttributes & P) | null, + ...children: ComponentChildren[] +): VNode; +export function h( type: string, props: - | (JSXInternal.HTMLAttributes & - JSXInternal.SVGAttributes & - Record) + | (ClassAttributes & + JSXInternal.HTMLAttributes & + JSXInternal.SVGAttributes) | null, ...children: ComponentChildren[] ): VNode; @@ -224,7 +272,7 @@ export namespace h { export function render( vnode: ComponentChild, - parent: Element | Document | ShadowRoot | DocumentFragment, + parent: Element | Document | ShadowRoot | DocumentFragment ): void; export function hydrate( vnode: ComponentChild, diff --git a/test/ts/preact.tsx b/test/ts/preact.tsx index 9779d41770..ad39d4bfc4 100644 --- a/test/ts/preact.tsx +++ b/test/ts/preact.tsx @@ -5,7 +5,8 @@ import { ComponentProps, FunctionalComponent, AnyComponent, - h + h, + createRef } from '../../'; interface DummyProps { @@ -295,3 +296,20 @@ let elementProps: ComponentProps<'button'> = { // Typing of style property const acceptsNumberAsLength =
; const acceptsStringAsLength =
; + +// Refs should work on elements +const ref = createRef(); +createElement('div', { ref: ref }, 'hi'); +h('div', { ref: ref }, 'hi'); + +// Refs should work on functions +const functionRef = createRef(); +const RefComponentTest = () =>

hi

; +createElement(RefComponentTest, { ref: functionRef }, 'hi'); +h(RefComponentTest, { ref: functionRef }, 'hi'); + +// Should accept onInput +const onInput = (e: h.JSX.TargetedEvent) => {}; +; +createElement('input', { onInput: onInput }); +h('input', { onInput: onInput }); From 70e58654a9957ee7994e244100d4a9b6ff463ef8 Mon Sep 17 00:00:00 2001 From: jdecroock Date: Fri, 2 Sep 2022 12:41:56 +0200 Subject: [PATCH 7/7] backport useState optim --- hooks/src/index.js | 50 +++++++++++-- hooks/test/browser/useState.test.js | 109 ++++++++++++++++++++++------ 2 files changed, 128 insertions(+), 31 deletions(-) diff --git a/hooks/src/index.js b/hooks/src/index.js index 8149f92167..847f40526f 100644 --- a/hooks/src/index.js +++ b/hooks/src/index.js @@ -180,15 +180,49 @@ export function useReducer(reducer, initialState, init) { ]; hookState._internal = currentInternal; - currentInternal._component.shouldComponentUpdate = () => { - if (!hookState._nextValue) return true; - - const currentValue = hookState._value[0]; - hookState._value = hookState._nextValue; - hookState._nextValue = undefined; + if (!currentInternal.data._hasScuFromHooks) { + currentInternal.data._hasScuFromHooks = true; + const prevScu = currentInternal._component.shouldComponentUpdate; + + // This SCU has the purpose of bailing out after repeated updates + // to stateful hooks. + // we store the next value in _nextValue[0] and keep doing that for all + // state setters, if we have next states and + // all next states within a component end up being equal to their original state + // we are safe to bail out for this specific component. + currentInternal._component.shouldComponentUpdate = function(p, s, c) { + if (!hookState._internal.data.__hooks) return true; + + const stateHooks = hookState._internal.data.__hooks._list.filter( + x => x._internal + ); + const allHooksEmpty = stateHooks.every(x => !x._nextValue); + // When we have no updated hooks in the component we invoke the previous SCU or + // traverse the VDOM tree further. + if (allHooksEmpty) { + return prevScu ? prevScu.call(this, p, s, c) : true; + } - return currentValue !== hookState._value[0]; - }; + // We check whether we have components with a nextValue set that + // have values that aren't equal to one another this pushes + // us to update further down the tree + let shouldUpdate = false; + stateHooks.forEach(hookItem => { + if (hookItem._nextValue) { + const currentValue = hookItem._value[0]; + hookItem._value = hookItem._nextValue; + hookItem._nextValue = undefined; + if (currentValue !== hookItem._value[0]) shouldUpdate = true; + } + }); + + return shouldUpdate + ? prevScu + ? prevScu.call(this, p, s, c) + : true + : false; + }; + } } return hookState._nextValue || hookState._value; diff --git a/hooks/test/browser/useState.test.js b/hooks/test/browser/useState.test.js index 771a96ada1..64b5e59af8 100644 --- a/hooks/test/browser/useState.test.js +++ b/hooks/test/browser/useState.test.js @@ -1,4 +1,4 @@ -import { act, setupRerender } from 'preact/test-utils'; +import { setupRerender, act } from 'preact/test-utils'; import { createElement, render, createContext } from 'preact'; import { useState, useContext, useEffect } from 'preact/hooks'; import { setupScratch, teardown } from '../../../test/_util/helpers'; @@ -55,7 +55,7 @@ describe('useState', () => { let lastState; let doSetState; - const Comp = sinon.spy(function Comp() { + const Comp = sinon.spy(() => { const [state, setState] = useState(0); lastState = state; doSetState = setState; @@ -81,7 +81,7 @@ describe('useState', () => { let lastState; let doSetState; - const Comp = sinon.spy(function Comp() { + const Comp = sinon.spy(() => { const [state, setState] = useState(0); lastState = state; doSetState = setState; @@ -176,26 +176,6 @@ describe('useState', () => { expect(scratch.innerHTML).to.equal('

hi

'); }); - it('should render a second time when the render function updates state', () => { - const calls = []; - const App = () => { - const [greeting, setGreeting] = useState('bye'); - - if (greeting === 'bye') { - setGreeting('hi'); - } - - calls.push(greeting); - - return

{greeting}

; - }; - - render(, scratch); - expect(calls.length).to.equal(2); - expect(calls).to.deep.equal(['bye', 'hi']); - expect(scratch.textContent).to.equal('hi'); - }); - it('should handle queued useState', () => { function Message({ message, onClose }) { const [isVisible, setVisible] = useState(Boolean(message)); @@ -232,6 +212,89 @@ describe('useState', () => { expect(scratch.innerHTML).to.equal(''); }); + it('should render a second time when the render function updates state', () => { + const calls = []; + const App = () => { + const [greeting, setGreeting] = useState('bye'); + + if (greeting === 'bye') { + setGreeting('hi'); + } + + calls.push(greeting); + + return

{greeting}

; + }; + + act(() => { + render(, scratch); + }); + expect(calls.length).to.equal(2); + expect(calls).to.deep.equal(['bye', 'hi']); + expect(scratch.textContent).to.equal('hi'); + }); + + // https://github.com/preactjs/preact/issues/3669 + it('correctly updates with multiple state updates', () => { + let simulateClick; + function TestWidget() { + const [saved, setSaved] = useState(false); + const [, setSaving] = useState(false); + + simulateClick = () => { + setSaving(true); + setSaved(true); + setSaving(false); + }; + + return
{saved ? 'Saved!' : 'Unsaved!'}
; + } + + render(, scratch); + expect(scratch.innerHTML).to.equal('
Unsaved!
'); + + act(() => { + simulateClick(); + }); + + expect(scratch.innerHTML).to.equal('
Saved!
'); + }); + + // https://github.com/preactjs/preact/issues/3674 + it('ensure we iterate over all hooks', () => { + let open, close; + + function TestWidget() { + const [, setCounter] = useState(0); + const [isOpen, setOpen] = useState(false); + + open = () => { + setCounter(42); + setOpen(true); + }; + + close = () => { + setOpen(false); + }; + + return
{isOpen ? 'open' : 'closed'}
; + } + + render(, scratch); + expect(scratch.innerHTML).to.equal('
closed
'); + + act(() => { + open(); + }); + + expect(scratch.innerHTML).to.equal('
open
'); + + act(() => { + close(); + }); + expect(scratch.innerHTML).to.equal('
closed
'); + }); + it('does not loop when states are equal after batches', () => { const renderSpy = sinon.spy(); const Context = createContext(null);