Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Backports x #3707

Merged
merged 7 commits into from
Sep 2, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
32 changes: 26 additions & 6 deletions compat/src/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand Down
4 changes: 2 additions & 2 deletions compat/src/render.js
Original file line number Diff line number Diff line change
Expand Up @@ -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".
Expand Down Expand Up @@ -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;
}
Expand Down
95 changes: 92 additions & 3 deletions compat/test/browser/hooks.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand Down Expand Up @@ -91,7 +93,7 @@ describe('React-18-hooks', () => {
});
expect(scratch.innerHTML).to.equal('<p>hello world</p>');
expect(subscribe).to.be.calledOnce;
expect(getSnapshot).to.be.calledOnce;
expect(getSnapshot).to.be.calledThrice;
});

it('subscribes and rerenders when called', () => {
Expand Down Expand Up @@ -119,13 +121,100 @@ describe('React-18-hooks', () => {
});
expect(scratch.innerHTML).to.equal('<p>hello world</p>');
expect(subscribe).to.be.calledOnce;
expect(getSnapshot).to.be.calledOnce;
expect(getSnapshot).to.be.calledThrice;

called = true;
flush();
rerender();

expect(scratch.innerHTML).to.equal('<p>hello new world</p>');
});

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 <p>{value()}</p>;
};

act(() => {
render(<App />, scratch);
});
expect(scratch.innerHTML).to.equal('<p>value: 0</p>');
expect(subscribe).to.be.calledOnce;
expect(getSnapshot).to.be.calledThrice;

flush();
rerender();

expect(scratch.innerHTML).to.equal('<p>value: 0</p>');
});

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 <p>value: {value}</p>;
};

act(() => {
render(<App />, scratch);
});
expect(scratch.innerHTML).to.equal('<p>value: 0</p>');
expect(subscribe).to.be.calledOnce;

i++;
flush();
rerender();

expect(scratch.innerHTML).to.equal('<p>value: 1</p>');
});

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 <p>{value}</p>;
};

act(() => {
render(<App />, scratch);
});
expect(scratch.innerHTML).to.equal('<p>yep</p>');

toggle();
rerender();

expect(scratch.innerHTML).to.equal('<p>nope</p>');
});
});
});
11 changes: 10 additions & 1 deletion compat/test/browser/svg.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -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"
/>,
Expand All @@ -88,7 +97,7 @@ describe('svg', () => {

expect(serializeHtml(scratch)).to.eql(
sortAttributes(
'<svg clip-path="value" clip-rule="value" clipPathUnits="value" glyph-orientationhorizontal="value" shape-rendering="crispEdges" glyphRef="value" marker-start="value" markerHeight="value" markerUnits="value" markerWidth="value" x1="value" xChannelSelector="value"></svg>'
'<svg clip-path="value" clip-rule="value" clipPathUnits="value" color-interpolation-filters="auto" font-size-adjust="value" glyph-orientation-horizontal="value" glyph-orientation-vertical="value" shape-rendering="crispEdges" glyphRef="value" horiz-adv-x="value" horiz-origin-x="value" marker-start="value" markerHeight="value" markerUnits="value" markerWidth="value" units-per-em="value" vert-adv-y="value" vert-origin-x="value" vert-origin-y="value" x1="value" xChannelSelector="value"></svg>'
)
);
});
Expand Down
1 change: 1 addition & 0 deletions follow-ups.md
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
54 changes: 44 additions & 10 deletions hooks/src/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ let previousInternal;
/** @type {number} */
let currentHook = 0;

/** @type {Array<import('./internal').Component>} */
/** @type {Array<import('./internal').Internal>} */
let afterPaintEffects = [];

let EMPTY = [];
Expand Down Expand Up @@ -180,16 +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;

return currentValue !== hookState._value[0];
};
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;
}

// 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;
Expand Down Expand Up @@ -359,6 +392,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);
Expand Down
Loading