diff --git a/compat/src/index.js b/compat/src/index.js index ba2ea905b3..62eafa5df7 100644 --- a/compat/src/index.js +++ b/compat/src/index.js @@ -137,19 +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 } + }); - // 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. + useLayoutEffect(() => { + _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/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/hooks.test.js b/compat/test/browser/hooks.test.js index f501ae7d91..3255704756 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.calledThrice; }); 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.calledThrice; called = true; flush(); @@ -127,5 +129,92 @@ 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 () => {}; + }); + + const func = () => 'value: ' + i++; + + let i = 0; + const getSnapshot = sinon.spy(() => { + return func; + }); + + const App = () => { + const value = useSyncExternalStore(subscribe, getSnapshot); + return{value()}
; + }; + + act(() => { + render(value: 0
'); + expect(subscribe).to.be.calledOnce; + expect(getSnapshot).to.be.calledThrice; + + flush(); + rerender(); + + 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; + }); + returnvalue: {value}
; + }; + + act(() => { + render(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 = () => { + const [state, setState] = useState(true); + toggle = setState.bind(this, () => false); + + const value = useSyncExternalStore( + useCallback(() => { + return () => {}; + }, [state]), + () => (state ? 'yep' : 'nope') + ); + + return{value}
; + }; + + act(() => { + render(yep
'); + + toggle(); + rerender(); + + expect(scratch.innerHTML).to.equal('nope
'); + }); }); }); 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( - '' + '' ) ); }); 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/hooks/src/index.js b/hooks/src/index.js index 100ae43c37..847f40526f 100644 --- a/hooks/src/index.js +++ b/hooks/src/index.js @@ -14,7 +14,7 @@ let previousInternal; /** @type {number} */ let currentHook = 0; -/** @type {Arrayhi
'); }); - 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({greeting}
; + }; + + act(() => { + render( {
// -----------------------------------
export function createElement(
+ type: 'input',
+ props:
+ | (JSXInternal.DOMAttributes hi