diff --git a/compat/src/index.js b/compat/src/index.js index 1501a9a03c8..7f7f05b1ae2 100644 --- a/compat/src/index.js +++ b/compat/src/index.js @@ -12,7 +12,6 @@ const CAMEL_PROPS = /^(?:accent|alignment|arabic|baseline|cap|clip|color|fill|fl let oldEventHook = options.event; options.event = e => { - /* istanbul ignore next */ if (oldEventHook) e = oldEventHook(e); e.persist = () => {}; return e.nativeEvent = e; @@ -389,6 +388,10 @@ function setSafeDescriptor(proto, key) { Object.defineProperty(proto, key, { configurable: false, get() { return this['UNSAFE_' + key]; }, + // This `set` is only used if a user sets a lifecycle like cWU + // after setting a lifecycle like UNSAFE_cWU. I doubt anyone + // actually does this in practice so not testing it + /* istanbul ignore next */ set(v) { this['UNSAFE_' + key] = v; } }); } @@ -414,7 +417,6 @@ options.vnode = vnode => { setSafeDescriptor(type.prototype, 'componentWillUpdate'); type._patchedLifecycles = true; } - /* istanbul ignore next */ if (oldVNodeHook) oldVNodeHook(vnode); }; diff --git a/compat/test/browser/compat.test.js b/compat/test/browser/compat.test.js index a658927d200..92760d8cdb2 100644 --- a/compat/test/browser/compat.test.js +++ b/compat/test/browser/compat.test.js @@ -21,9 +21,12 @@ describe('imported compat in preact', () => { scratch.firstChild.click(); expect(spy).to.be.calledOnce; - expect(spy.args[0][0]).to.haveOwnProperty('persist'); - expect(typeof spy.args[0][0].persist).to.equal('function'); - expect(spy.args[0][0]).to.haveOwnProperty('nativeEvent'); + const event = spy.args[0][0]; + expect(event).to.haveOwnProperty('persist'); + expect(event).to.haveOwnProperty('nativeEvent'); + expect(typeof event.persist).to.equal('function'); + + expect(() => event.persist()).to.not.throw(); }); it('should normalize ondoubleclick event', () => { diff --git a/compat/test/browser/forwardRef.test.js b/compat/test/browser/forwardRef.test.js index d872da22e1b..b7a57dc287f 100644 --- a/compat/test/browser/forwardRef.test.js +++ b/compat/test/browser/forwardRef.test.js @@ -239,6 +239,42 @@ describe('forwardRef', () => { expect(renderCount).to.equal(3); }); + it('should bailout if forwardRef is wrapped in memo using function refs', () => { + const Component = props =>
; + + let renderCount = 0; + + const App = memo( + forwardRef((props, ref) => { + renderCount++; + return ; + }), + ); + + const ref = sinon.spy(); + + render(, scratch); + expect(renderCount).to.equal(1); + + expect(ref).to.have.been.called; + + ref.resetHistory(); + render(, scratch); + expect(renderCount).to.equal(1); + + const differentRef = sinon.spy(); + + render(, scratch); + expect(renderCount).to.equal(2); + + expect(ref).to.have.been.calledWith(null); + expect(differentRef).to.have.been.called; + + differentRef.resetHistory(); + render(, scratch); + expect(renderCount).to.equal(3); + }); + it('should pass ref through memo() with custom comparer function', () => { const Foo = props =>
; diff --git a/compat/test/browser/memo.test.js b/compat/test/browser/memo.test.js index bde6288c302..92d9d076f65 100644 --- a/compat/test/browser/memo.test.js +++ b/compat/test/browser/memo.test.js @@ -17,6 +17,7 @@ describe('memo()', () => { }); it('should have isReactComponent flag', () => { + // eslint-disable-next-line react/display-name let App = memo(() =>
foo
); expect(App.prototype.isReactComponent).to.equal(true); }); diff --git a/compat/test/browser/options-compat.test.js b/compat/test/browser/options-compat.test.js new file mode 100644 index 00000000000..de63aedd82c --- /dev/null +++ b/compat/test/browser/options-compat.test.js @@ -0,0 +1,79 @@ +import { vnodeSpy, eventSpy } from '../../../test/_util/optionSpies'; +import { render, createElement, Component, createRef } from '../../src'; +import { setupRerender } from 'preact/test-utils'; +import { setupScratch, teardown, createEvent } from '../../../test/_util/helpers'; + +/** @jsx createElement */ + +describe('compat options', () => { + + /** @type {HTMLDivElement} */ + let scratch; + + /** @type {() => void} */ + let rerender; + + /** @type {() => void} */ + let increment; + + /** @type {import('../../src/index').PropRef} */ + let buttonRef; + + beforeEach(() => { + scratch = setupScratch(); + rerender = setupRerender(); + + vnodeSpy.resetHistory(); + eventSpy.resetHistory(); + + buttonRef = createRef(); + }); + + afterEach(() => { + teardown(scratch); + }); + + class ClassApp extends Component { + constructor() { + super(); + this.state = { count: 0 }; + increment = () => + this.setState(({ count }) => ({ + count: count + 1 + })); + } + + render() { + return ( + + ); + } + } + + it('should call old options on mount', () => { + render(, scratch); + + expect(vnodeSpy).to.have.been.called; + }); + + it('should call old options on event and update', () => { + render(, scratch); + expect(scratch.innerHTML).to.equal(''); + + buttonRef.current.dispatchEvent(createEvent('click')); + rerender(); + expect(scratch.innerHTML).to.equal(''); + + expect(vnodeSpy).to.have.been.called; + expect(eventSpy).to.have.been.called; + }); + + it('should call old options on unmount', () => { + render(, scratch); + render(null, scratch); + + expect(vnodeSpy).to.have.been.called; + }); +}); diff --git a/debug/src/debug.js b/debug/src/debug.js index 5d209e17a12..3435c018ca0 100644 --- a/debug/src/debug.js +++ b/debug/src/debug.js @@ -17,6 +17,8 @@ export function initDebug() { let oldDiffed = options.diffed; let oldVnode = options.vnode; let oldCatchError = options._catchError; + let oldRoot = options._root; + let oldHook = options._hook; const warnedComponents = { useEffect: {}, useLayoutEffect: {}, lazyPropTypes: {} }; options._catchError = (error, vnode, oldVNode) => { @@ -58,6 +60,8 @@ export function initDebug() { Expected a valid HTML node as a second argument to render. Received ${parentNode} instead: render(<${vnode.type.name || vnode.type} />, ${parentNode}); `); + + if (oldRoot) oldRoot(vnode, parentNode); }; options._diff = vnode => { @@ -164,6 +168,8 @@ export function initDebug() { if (!comp) { throw new Error('Hook can only be invoked from render methods.'); } + + if (oldHook) oldHook(comp); }; const warn = (property, err) => ({ @@ -221,34 +227,33 @@ export function initDebug() { if (vnode._component && vnode._component.__hooks) { let hooks = vnode._component.__hooks; - (hooks._list || []).forEach(hook => { - if (hook._callback && (!hook._args || !Array.isArray(hook._args))) { - /* istanbul ignore next */ - console.warn( - `In ${vnode.type.name || vnode.type} you are calling useMemo/useCallback without passing arguments.\n` + - `This is a noop since it will not be able to memoize, it will execute it every render.` - ); - } - }); - if (hooks._pendingEffects && Array.isArray(hooks._pendingEffects)) { + if (Array.isArray(hooks._list)) { + hooks._list.forEach(hook => { + if (hook._callback && (!hook._args || !Array.isArray(hook._args))) { + console.warn( + `In ${getDisplayName(vnode)} you are calling useMemo/useCallback without passing arguments.\n` + + `This is a noop since it will not be able to memoize, it will execute it every render.` + ); + } + }); + } + if (Array.isArray(hooks._pendingEffects)) { hooks._pendingEffects.forEach((effect) => { if ((!effect._args || !Array.isArray(effect._args)) && !warnedComponents.useEffect[vnode.type]) { warnedComponents.useEffect[vnode.type] = true; - /* istanbul ignore next */ console.warn('You should provide an array of arguments as the second argument to the "useEffect" hook.\n\n' + 'Not doing so will invoke this effect on every render.\n\n' + - 'This effect can be found in the render of ' + (vnode.type.name || vnode.type) + '.'); + 'This effect can be found in the render of ' + getDisplayName(vnode) + '.'); } }); } - if (hooks._pendingLayoutEffects && Array.isArray(hooks._pendingLayoutEffects)) { + if (Array.isArray(hooks._pendingLayoutEffects)) { hooks._pendingLayoutEffects.forEach((layoutEffect) => { if ((!layoutEffect._args || !Array.isArray(layoutEffect._args)) && !warnedComponents.useLayoutEffect[vnode.type]) { warnedComponents.useLayoutEffect[vnode.type] = true; - /* istanbul ignore next */ console.warn('You should provide an array of arguments as the second argument to the "useLayoutEffect" hook.\n\n' + 'Not doing so will invoke this effect on every render.\n\n' + - 'This effect can be found in the render of ' + (vnode.type.name || vnode.type) + '.'); + 'This effect can be found in the render of ' + getDisplayName(vnode) + '.'); } }); } diff --git a/debug/src/devtools/index.js b/debug/src/devtools/index.js index cb8d6bc3136..a33af9412ac 100644 --- a/debug/src/devtools/index.js +++ b/debug/src/devtools/index.js @@ -73,6 +73,7 @@ export function initDevTools() { }; if (!hook._renderers) { + // eslint-disable-next-line no-console console.info( 'Preact is not compatible with your version of react-devtools. We ' + 'will address this in future releases.' diff --git a/debug/test/browser/debug.test.js b/debug/test/browser/debug.test.js index 4a801e18365..b635a054482 100644 --- a/debug/test/browser/debug.test.js +++ b/debug/test/browser/debug.test.js @@ -274,6 +274,7 @@ describe('debug', () => { } render(, scratch); + forceUpdate(); expect(console.warn).to.not.be.called; render(null, scratch); @@ -477,6 +478,9 @@ describe('debug', () => { expect(serializeVNode()) .to.equal(''); + + expect(serializeVNode(
)) + .to.equal('
'); }); }); diff --git a/debug/test/browser/options-debug.test.js b/debug/test/browser/options-debug.test.js new file mode 100644 index 00000000000..545d4771708 --- /dev/null +++ b/debug/test/browser/options-debug.test.js @@ -0,0 +1,132 @@ +import { + vnodeSpy, + rootSpy, + beforeDiffSpy, + hookSpy, + afterDiffSpy, + catchErrorSpy +} from '../../../test/_util/optionSpies'; + +import { createElement, render, Component } from 'preact'; +import { useState } from 'preact/hooks'; +import { setupRerender } from 'preact/test-utils'; +import { initDebug } from '../../src/debug'; +import { setupScratch, teardown } from '../../../test/_util/helpers'; + +/** @jsx createElement */ + +describe('debug options', () => { + + /** @type {HTMLDivElement} */ + let scratch; + + /** @type {() => void} */ + let rerender; + + /** @type {(count: number) => void} */ + let setCount; + + before(() => { + initDebug(); + }); + + beforeEach(() => { + scratch = setupScratch(); + rerender = setupRerender(); + + vnodeSpy.resetHistory(); + rootSpy.resetHistory(); + beforeDiffSpy.resetHistory(); + hookSpy.resetHistory(); + afterDiffSpy.resetHistory(); + catchErrorSpy.resetHistory(); + }); + + afterEach(() => { + teardown(scratch); + }); + + class ClassApp extends Component { + constructor() { + super(); + this.state = { count: 0 }; + setCount = count => this.setState({ count }); + } + + render() { + return
{this.state.count}
; + } + } + + it('should call old options on mount', () => { + render(, scratch); + + expect(vnodeSpy).to.have.been.called; + expect(rootSpy).to.have.been.called; + expect(beforeDiffSpy).to.have.been.called; + expect(afterDiffSpy).to.have.been.called; + }); + + it('should call old options on update', () => { + render(, scratch); + + setCount(1); + rerender(); + + expect(vnodeSpy).to.have.been.called; + expect(rootSpy).to.have.been.called; + expect(beforeDiffSpy).to.have.been.called; + expect(afterDiffSpy).to.have.been.called; + }); + + it('should call old options on unmount', () => { + render(, scratch); + render(null, scratch); + + expect(vnodeSpy).to.have.been.called; + expect(rootSpy).to.have.been.called; + expect(beforeDiffSpy).to.have.been.called; + expect(afterDiffSpy).to.have.been.called; + }); + + it('should call old hook options for hook components', () => { + function HookApp() { + const [count, realSetCount] = useState(0); + setCount = realSetCount; + return
{count}
; + } + + render(, scratch); + + expect(hookSpy).to.have.been.called; + }); + + it('should call old options on error', () => { + class ErrorApp extends Component { + constructor() { + super(); + this.state = { error: true }; + } + componentDidCatch() { + this.setState({ error: false }); + } + render() { + return ; + } + } + + function Throw({ error }) { + if (error) { + throw new Error('test'); + } + else { + return
no error
; + } + } + + render(, scratch); + rerender(); + + expect(catchErrorSpy).to.have.been.called; + }); +}); diff --git a/debug/test/browser/options-devtools.test.js b/debug/test/browser/options-devtools.test.js new file mode 100644 index 00000000000..14fc5b12aab --- /dev/null +++ b/debug/test/browser/options-devtools.test.js @@ -0,0 +1,101 @@ +import { + vnodeSpy, + beforeDiffSpy, + afterDiffSpy, + beforeCommitSpy, + unmountSpy +} from '../../../test/_util/optionSpies'; + +import { createElement, render, Component } from 'preact'; +import { useState } from 'preact/hooks'; +import { setupRerender } from 'preact/test-utils'; +import { initDebug } from '../../src/debug'; +import { setupScratch, teardown } from '../../../test/_util/helpers'; + +/** @jsx createElement */ + +describe('devtools options', () => { + + /** @type {HTMLDivElement} */ + let scratch; + + /** @type {() => void} */ + let rerender; + + /** @type {(count: number) => void} */ + let setCount; + + before(() => { + initDebug(); + }); + + beforeEach(() => { + scratch = setupScratch(); + rerender = setupRerender(); + + vnodeSpy.resetHistory(); + beforeDiffSpy.resetHistory(); + afterDiffSpy.resetHistory(); + beforeCommitSpy.resetHistory(); + unmountSpy.resetHistory(); + }); + + afterEach(() => { + teardown(scratch); + }); + + class ClassApp extends Component { + constructor() { + super(); + this.state = { count: 0 }; + setCount = count => this.setState({ count }); + } + + render() { + return
{this.state.count}
; + } + } + + it('should call old options on mount', () => { + render(, scratch); + + expect(vnodeSpy).to.have.been.called; + expect(beforeDiffSpy).to.have.been.called; + expect(afterDiffSpy).to.have.been.called; + expect(beforeCommitSpy).to.have.been.called; + }); + + it('should call old options on update', () => { + render(, scratch); + + setCount(1); + rerender(); + + expect(vnodeSpy).to.have.been.called; + expect(beforeDiffSpy).to.have.been.called; + expect(afterDiffSpy).to.have.been.called; + expect(beforeCommitSpy).to.have.been.called; + }); + + it('should call old options on unmount', () => { + render(, scratch); + render(null, scratch); + + expect(unmountSpy).to.have.been.called; + }); + + it('should call old hook options for hook components', () => { + function HookApp() { + const [count, realSetCount] = useState(0); + setCount = realSetCount; + return
{count}
; + } + + render(, scratch); + + expect(vnodeSpy).to.have.been.called; + expect(beforeDiffSpy).to.have.been.called; + expect(afterDiffSpy).to.have.been.called; + expect(beforeCommitSpy).to.have.been.called; + }); +}); diff --git a/hooks/test/browser/options-hooks.test.js b/hooks/test/browser/options-hooks.test.js new file mode 100644 index 00000000000..8644edf4f33 --- /dev/null +++ b/hooks/test/browser/options-hooks.test.js @@ -0,0 +1,67 @@ +import { + afterDiffSpy, + beforeRenderSpy, + unmountSpy +} from '../../../test/_util/optionSpies'; + +import { setupRerender } from 'preact/test-utils'; +import { createElement, render } from 'preact'; +import { setupScratch, teardown } from '../../../test/_util/helpers'; +import { useState } from '../../src'; + +/** @jsx createElement */ + +describe('hook options', () => { + + /** @type {HTMLDivElement} */ + let scratch; + + /** @type {() => void} */ + let rerender; + + /** @type {() => void} */ + let increment; + + beforeEach(() => { + scratch = setupScratch(); + rerender = setupRerender(); + + afterDiffSpy.resetHistory(); + unmountSpy.resetHistory(); + beforeRenderSpy.resetHistory(); + }); + + afterEach(() => { + teardown(scratch); + }); + + function App() { + const [count, setCount] = useState(0); + increment = () => setCount(prevCount => prevCount + 1); + return
{count}
; + } + + it('should call old options on mount', () => { + render(, scratch); + + expect(beforeRenderSpy).to.have.been.called; + expect(afterDiffSpy).to.have.been.called; + }); + + it('should call old options.diffed on update', () => { + render(, scratch); + + increment(); + rerender(); + + expect(beforeRenderSpy).to.have.been.called; + expect(afterDiffSpy).to.have.been.called; + }); + + it('should call old options on unmount', () => { + render(, scratch); + render(null, scratch); + + expect(unmountSpy).to.have.been.called; + }); +}); diff --git a/test/_util/optionSpies.js b/test/_util/optionSpies.js new file mode 100644 index 00000000000..c400bf25910 --- /dev/null +++ b/test/_util/optionSpies.js @@ -0,0 +1,39 @@ +import { options as rawOptions } from 'preact'; + +/** @type {import('../../src/internal').Options} */ +let options = rawOptions; + +let oldVNode = options.vnode; +let oldEvent = options.event || (e => e); +let oldAfterDiff = options.diffed; +let oldUnmount = options.unmount; + +let oldRoot = options._root; +let oldBeforeDiff = options._diff; +let oldBeforeRender = options._render; +let oldBeforeCommit = options._commit; +let oldHook = options._hook; +let oldCatchError = options._catchError; + +export const vnodeSpy = sinon.spy(oldVNode); +export const eventSpy = sinon.spy(oldEvent); +export const afterDiffSpy = sinon.spy(oldAfterDiff); +export const unmountSpy = sinon.spy(oldUnmount); + +export const rootSpy = sinon.spy(oldRoot); +export const beforeDiffSpy = sinon.spy(oldBeforeDiff); +export const beforeRenderSpy = sinon.spy(oldBeforeRender); +export const beforeCommitSpy = sinon.spy(oldBeforeCommit); +export const hookSpy = sinon.spy(oldHook); +export const catchErrorSpy = sinon.spy(oldCatchError); + +options.vnode = vnodeSpy; +options.event = eventSpy; +options.diffed = afterDiffSpy; +options.unmount = unmountSpy; +options._root = rootSpy; +options._diff = beforeDiffSpy; +options._render = beforeRenderSpy; +options._commit = beforeCommitSpy; +options._hook = hookSpy; +options._catchError = catchErrorSpy;