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

Backport 10.14 release line #4263

Merged
merged 14 commits into from
Jan 23, 2024
18 changes: 17 additions & 1 deletion benches/src/util.js
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand Down Expand Up @@ -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;
Expand Down
2 changes: 1 addition & 1 deletion compat/mangle.json
Original file line number Diff line number Diff line change
Expand Up @@ -18,4 +18,4 @@
}
}
}
}
}
2 changes: 1 addition & 1 deletion compat/src/render.js
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
20 changes: 19 additions & 1 deletion compat/test/browser/cloneElement.test.js
Original file line number Diff line number Diff line change
@@ -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', () => {
Expand Down Expand Up @@ -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 <div style={{ color: props.color }}>thing</div>;
}
}
Example.defaultProps = { color: 'blue' };

const element = <Example color="red" />;
const clone = cloneElement(element, { color: undefined });
expect(clone.props.color).to.equal('blue');
});
});
15 changes: 15 additions & 0 deletions compat/test/browser/render.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -495,6 +495,21 @@ describe('compat render', () => {
);
});

it('shouldnot transform imageSrcSet', () => {

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

shouldnot or should not?

render(
<link
rel="preload"
as="image"
href="preact.jpg"
imageSrcSet="preact_400px.jpg 400w"
/>,
scratch
);
expect(scratch.innerHTML).to.equal(
'<link rel="preload" as="image" href="preact.jpg" imagesrcset="preact_400px.jpg 400w">'
);
});

it("should support react-relay's usage of __SECRET_INTERNALS_DO_NOT_USE_OR_YOU_WILL_BE_FIRED", () => {
const Ctx = createContext('foo');

Expand Down
2 changes: 1 addition & 1 deletion debug/mangle.json
Original file line number Diff line number Diff line change
Expand Up @@ -18,4 +18,4 @@
}
}
}
}
}
1 change: 1 addition & 0 deletions debug/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
},
"exports": {
".": {
"types": "./src/index.d.ts",
"browser": "./dist/debug.mjs",
"umd": "./dist/debug.umd.js",
"import": "./dist/debug.mjs",
Expand Down
37 changes: 29 additions & 8 deletions debug/src/debug.js
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ export function initDebug() {
useEffect: new WeakMap(),
useLayoutEffect: new WeakMap(),
lazyPropTypes: new WeakMap()
};
};
const deprecations = [];

options._catchError = (error, vnode, oldVNode) => {
Expand Down Expand Up @@ -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;
Expand All @@ -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
Expand All @@ -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.`
);
}
}
}
}
}
}
};
}

Expand Down
74 changes: 74 additions & 0 deletions debug/test/browser/validateHookArgs.test.js
Original file line number Diff line number Diff line change
@@ -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 (
<button type="button" onClick={() => setValue(NaN)}>
Set to NaN
</button>
);
};

it(`should error if ${name} is mounted with NaN as an argument`, async () => {
expect(() =>
render(<TestComponent initialValue={NaN} />, 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(<TestComponent initialValue={0} />, 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]);
});
});
2 changes: 1 addition & 1 deletion devtools/mangle.json
Original file line number Diff line number Diff line change
Expand Up @@ -18,4 +18,4 @@
}
}
}
}
}
2 changes: 1 addition & 1 deletion hooks/mangle.json
Original file line number Diff line number Diff line change
Expand Up @@ -18,4 +18,4 @@
}
}
}
}
}
23 changes: 22 additions & 1 deletion hooks/test/browser/useImperativeHandle.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -180,7 +180,7 @@ describe('useImperativeHandle', () => {
expect(() => render(<Comp />, 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' }));

Expand All @@ -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 <p>Test</p>;
}

render(<Comp />, scratch);
expect(createHandleSpy).to.have.been.calledOnce;
expect(ref).to.have.been.calledWith(handle);

ref.resetHistory();

render(<div />, scratch);
expect(createHandleSpy).to.have.been.calledOnce;
expect(ref).to.have.been.calledWith(null);
});
});
2 changes: 1 addition & 1 deletion jsx-runtime/mangle.json
Original file line number Diff line number Diff line change
Expand Up @@ -18,4 +18,4 @@
}
}
}
}
}
10 changes: 10 additions & 0 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down
3 changes: 2 additions & 1 deletion src/create-element.js
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import { UNDEFINED } from './constants';
import options from './options';
import { isArray } from './util';

let vnodeId = 0;

Expand Down Expand Up @@ -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);
}

Expand Down
3 changes: 2 additions & 1 deletion src/diff/children.js
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -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);
}
Expand Down
5 changes: 3 additions & 2 deletions src/diff/mount.js
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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 {
Expand Down
Loading
Loading