diff --git a/debug/src/debug.js b/debug/src/debug.js index 3435c018ca..648b7f7270 100644 --- a/debug/src/debug.js +++ b/debug/src/debug.js @@ -3,6 +3,8 @@ import { getDisplayName } from './devtools/custom'; import { options, Component } from 'preact'; import { ELEMENT_NODE, DOCUMENT_NODE, DOCUMENT_FRAGMENT_NODE } from './constants'; +const isWeakMapSupported = typeof WeakMap === 'function'; + function getClosestDomNodeParent(parent) { if (!parent) return {}; if (typeof parent.type === 'function') { @@ -19,7 +21,11 @@ export function initDebug() { let oldCatchError = options._catchError; let oldRoot = options._root; let oldHook = options._hook; - const warnedComponents = { useEffect: {}, useLayoutEffect: {}, lazyPropTypes: {} }; + const warnedComponents = !isWeakMapSupported ? null : ({ + useEffect: new WeakMap(), + useLayoutEffect: new WeakMap(), + lazyPropTypes: new WeakMap() + }); options._catchError = (error, vnode, oldVNode) => { let component = vnode && vnode._component; @@ -58,7 +64,7 @@ export function initDebug() { } if (!isValid) throw new Error(` Expected a valid HTML node as a second argument to render. - Received ${parentNode} instead: render(<${vnode.type.name || vnode.type} />, ${parentNode}); + Received ${parentNode} instead: render(<${getDisplayName(vnode)} />, ${parentNode}); `); if (oldRoot) oldRoot(vnode, parentNode); @@ -147,12 +153,12 @@ export function initDebug() { // Check prop-types if available if (typeof vnode.type==='function' && vnode.type.propTypes) { - if (vnode.type.displayName === 'Lazy' && !warnedComponents.lazyPropTypes[vnode.type]) { + if (vnode.type.displayName === 'Lazy' && warnedComponents && !warnedComponents.lazyPropTypes.has(vnode.type)) { const m = 'PropTypes are not supported on lazy(). Use propTypes on the wrapped component itself. '; try { const lazyVNode = vnode.type(); - warnedComponents.lazyPropTypes[vnode.type] = true; - console.warn(m + 'Component wrapped in lazy() is ' + (lazyVNode.type.displayName || lazyVNode.type.name)); + warnedComponents.lazyPropTypes.set(vnode.type, true); + console.warn(m + 'Component wrapped in lazy() is ' + getDisplayName(lazyVNode)); } catch (promise) { console.warn(m + 'We will log the wrapped component\'s name once it is loaded.'); @@ -223,8 +229,6 @@ export function initDebug() { }); } - if (oldDiffed) oldDiffed(vnode); - if (vnode._component && vnode._component.__hooks) { let hooks = vnode._component.__hooks; if (Array.isArray(hooks._list)) { @@ -239,8 +243,8 @@ export function initDebug() { } 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; + if (!Array.isArray(effect._args) && warnedComponents && !warnedComponents.useEffect.has(vnode.type)) { + warnedComponents.useEffect.set(vnode.type, true); 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 ' + getDisplayName(vnode) + '.'); @@ -249,8 +253,8 @@ export function initDebug() { } 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; + if (!Array.isArray(layoutEffect._args) && warnedComponents && !warnedComponents.useLayoutEffect.has(vnode.type)) { + warnedComponents.useLayoutEffect.set(vnode.type, true); 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 ' + getDisplayName(vnode) + '.'); @@ -259,6 +263,8 @@ export function initDebug() { } } + if (oldDiffed) oldDiffed(vnode); + if (vnode._children != null) { const keys = []; for (let i = 0; i < vnode._children.length; i++) { diff --git a/debug/test/browser/debug-hooks.test.js b/debug/test/browser/debug-hooks.test.js new file mode 100644 index 0000000000..074f1d9153 --- /dev/null +++ b/debug/test/browser/debug-hooks.test.js @@ -0,0 +1,136 @@ +import { createElement, render, Component } from 'preact'; +import { + useState, + useEffect, + useLayoutEffect, + useMemo, + useCallback +} from 'preact/hooks'; +import { initDebug } from '../../src/debug'; +import { act } from 'preact/test-utils'; +import { setupScratch, teardown } from '../../../test/_util/helpers'; + +/** @jsx createElement */ + +initDebug(); + +describe('debug with hooks', () => { + let scratch; + let errors = []; + let warnings = []; + + beforeEach(() => { + errors = []; + warnings = []; + scratch = setupScratch(); + sinon.stub(console, 'error').callsFake(e => errors.push(e)); + sinon.stub(console, 'warn').callsFake(w => warnings.push(w)); + }); + + afterEach(() => { + (console.error).restore(); + console.warn.restore(); + teardown(scratch); + }); + + // TODO: Fix this test. It only passed before because App was the first component + // into render so currentComponent in hooks/index.js wasn't set yet. However, + // any children under App wouldn't have thrown the error if they did what App + // did because currentComponent would be set to App. + // In other words, hooks never clear currentComponent so once it is set, it won't + // be unset + it.skip('should throw an error when using a hook outside a render', () => { + const Foo = props => props.children; + class App extends Component { + componentWillMount() { + useState(); + } + + render() { + return

test

; + } + } + const fn = () => + act(() => + render( + + + , + scratch + ) + ); + expect(fn).to.throw(/Hook can only be invoked from render/); + }); + + // TODO: Fix this test. It only passed before because render was never called. + // Once render is called, currentComponent is set and never unset so calls to + // hooks outside of components would still work. + it.skip('should throw an error when invoked outside of a component', () => { + function Foo(props) { + useEffect(() => {}); // Pretend to use a hook + return props.children; + } + + const fn = () => + act(() => { + render(Hello!, scratch); + useState(); + }); + expect(fn).to.throw(/Hook can only be invoked from render/); + }); + + it('should warn for argumentless useEffect hooks', () => { + const App = () => { + const [state] = useState('test'); + useEffect(() => 'test'); + return

{state}

; + }; + render(, scratch); + expect(warnings[0]).to.match(/You should provide an array of arguments/); + render(, scratch); + expect(warnings[1]).to.be.undefined; + }); + + it('should warn for argumentless useLayoutEffect hooks', () => { + const App = () => { + const [state] = useState('test'); + useLayoutEffect(() => 'test'); + return

{state}

; + }; + render(, scratch); + expect(warnings[0]).to.match(/You should provide an array of arguments/); + render(, scratch); + expect(warnings[1]).to.be.undefined; + }); + + it('should not warn for argumented effect hooks', () => { + const App = () => { + const [state] = useState('test'); + useLayoutEffect(() => 'test', []); + useEffect(() => 'test', [state]); + return

{state}

; + }; + const fn = () => act(() => render(, scratch)); + expect(fn).to.not.throw(); + }); + + it('should warn for useless useMemo calls', () => { + const App = () => { + const [people] = useState([40, 20, 60, 80]); + const retiredPeople = useMemo(() => people.filter(x => x >= 60)); + const cb = useCallback(() => () => 'test'); + return

{retiredPeople.map(x => x)}

; + }; + render(, scratch); + expect(warnings.length).to.equal(2); + }); + + it('should warn when non-array args is passed', () => { + const App = () => { + const foo = useMemo(() => 'foo', 12); + return

{foo}

; + }; + render(, scratch); + expect(warnings[0]).to.match(/without passing arguments/); + }); +}); diff --git a/debug/test/browser/debug-suspense.test.js b/debug/test/browser/debug-suspense.test.js new file mode 100644 index 0000000000..55d3609d22 --- /dev/null +++ b/debug/test/browser/debug-suspense.test.js @@ -0,0 +1,176 @@ +import { createElement, render, lazy, Suspense } from 'preact/compat'; +import { initDebug } from '../../src/debug'; +import { setupRerender } from 'preact/test-utils'; +import { + setupScratch, + teardown, + serializeHtml +} from '../../../test/_util/helpers'; + +/** @jsx createElement */ + +initDebug(); + +describe('debug with suspense', () => { + let scratch; + let errors = []; + let warnings = []; + + beforeEach(() => { + errors = []; + warnings = []; + scratch = setupScratch(); + sinon.stub(console, 'error').callsFake(e => errors.push(e)); + sinon.stub(console, 'warn').callsFake(w => warnings.push(w)); + }); + + afterEach(() => { + console.error.restore(); + console.warn.restore(); + teardown(scratch); + }); + + it('should throw on missing ', () => { + function Foo() { + throw Promise.resolve(); + } + + expect(() => render(, scratch)).to.throw; + }); + + it('should throw an error when using lazy and missing Suspense', () => { + const Foo = () =>
Foo
; + const LazyComp = lazy( + () => new Promise(resolve => resolve({ default: Foo })) + ); + const fn = () => { + render(, scratch); + }; + + expect(fn).to.throw(/Missing Suspense/gi); + }); + + describe('PropTypes', () => { + it('should validate propTypes inside lazy()', () => { + const rerender = setupRerender(); + + function Baz(props) { + return

{props.unhappy}

; + } + + Baz.propTypes = { + unhappy: function alwaysThrows(obj, key) { + if (obj[key] === 'signal') throw Error('got prop inside lazy()'); + } + }; + + const loader = Promise.resolve({ default: Baz }); + const LazyBaz = lazy(() => loader); + + const suspense = ( + fallback...}> + + + ); + render(suspense, scratch); + + expect(console.error).to.not.be.called; + + return loader + .then(() => Promise.all(suspense._component._suspensions)) + .then(() => { + rerender(); + expect(errors.length).to.equal(1); + expect(errors[0].includes('got prop')).to.equal(true); + expect(serializeHtml(scratch)).to.equal('

signal

'); + }); + }); + + describe('warn for PropTypes on lazy()', () => { + it('should log the function name', () => { + const loader = Promise.resolve({ + default: function MyLazyLoaded() { + return
Hi there
; + } + }); + const FakeLazy = lazy(() => loader); + FakeLazy.propTypes = {}; + const suspense = ( + fallback...}> + + + ); + render(suspense, scratch); + + return loader + .then(() => Promise.all(suspense._component._suspensions)) + .then(() => { + expect(console.warn).to.be.calledTwice; + expect(warnings[1].includes('MyLazyLoaded')).to.equal(true); + }); + }); + + it('should log the displayName', () => { + function MyLazyLoadedComponent() { + return
Hi there
; + } + MyLazyLoadedComponent.displayName = 'HelloLazy'; + const loader = Promise.resolve({ default: MyLazyLoadedComponent }); + const FakeLazy = lazy(() => loader); + FakeLazy.propTypes = {}; + const suspense = ( + fallback...}> + + + ); + render(suspense, scratch); + + return loader + .then(() => Promise.all(suspense._component._suspensions)) + .then(() => { + expect(console.warn).to.be.calledTwice; + expect(warnings[1].includes('HelloLazy')).to.equal(true); + }); + }); + + it('should not log a component if lazy throws', () => { + const loader = Promise.reject(new Error('Hey there')); + const FakeLazy = lazy(() => loader); + FakeLazy.propTypes = {}; + render( + fallback...}> + + , + scratch + ); + + return loader.catch(() => { + expect(console.warn).to.be.calledOnce; + }); + }); + + it("should not log a component if lazy's loader throws", () => { + const FakeLazy = lazy(() => { + throw new Error('Hello'); + }); + FakeLazy.propTypes = {}; + let error; + try { + render( + fallback...}> + + , + scratch + ); + } + catch (e) { + error = e; + } + + expect(console.warn).to.be.calledOnce; + expect(error).not.to.be.undefined; + expect(error.message).to.eql('Hello'); + }); + }); + }); +}); diff --git a/debug/test/browser/debug.test.js b/debug/test/browser/debug.test.js index b635a05448..e52f7bd16d 100644 --- a/debug/test/browser/debug.test.js +++ b/debug/test/browser/debug.test.js @@ -1,11 +1,10 @@ -import { createElement as h, options, render, createRef, Component, Fragment } from 'preact'; -import { lazy, Suspense } from 'preact/compat'; -import { useState, useEffect, useLayoutEffect, useMemo, useCallback } from 'preact/hooks'; -import { act, setupRerender } from 'preact/test-utils'; -import { setupScratch, teardown, clearOptions, serializeHtml } from '../../../test/_util/helpers'; +import { createElement as h, render, createRef, Component, Fragment } from 'preact'; +import { setupScratch, teardown, serializeHtml } from '../../../test/_util/helpers'; import { serializeVNode, initDebug } from '../../src/debug'; import * as PropTypes from 'prop-types'; +initDebug(); + /** @jsx h */ describe('debug', () => { @@ -13,20 +12,12 @@ describe('debug', () => { let errors = []; let warnings = []; - let diffSpy; - beforeEach(() => { errors = []; warnings = []; scratch = setupScratch(); sinon.stub(console, 'error').callsFake(e => errors.push(e)); sinon.stub(console, 'warn').callsFake(w => warnings.push(w)); - - clearOptions(); - diffSpy = sinon.spy(); - options._diff = diffSpy; - - initDebug(); }); afterEach(() => { @@ -37,11 +28,6 @@ describe('debug', () => { teardown(scratch); }); - it('should call previous options', () => { - render(
, scratch); - expect(diffSpy, 'diff').to.have.been.called; - }); - it('should print an error on rendering on undefined parent', () => { let fn = () => render(
, undefined); expect(fn).to.throw(/render/); @@ -58,6 +44,7 @@ describe('debug', () => { let fn = () => render(, 6); expect(fn).to.throw(/ render(, {}); expect(fn).to.throw(/ { expect(fn).to.throw(/createElement/); }); + it('should print an error when component is an array', () => { + let fn = () => render(h([
]), scratch); + expect(fn).to.throw(/createElement/); + }); + + it('should print an error on double jsx conversion', () => { + let Foo =
; + let fn = () => render(h(), scratch); + expect(fn).to.throw(/createElement/); + }); + it('should add __source to the vnode in debug mode.', () => { const vnode = h('div', { __source: { @@ -105,90 +103,6 @@ describe('debug', () => { expect(vnode.props.__self).to.be.undefined; }); - // TODO: Fix this test. It only passed before because App was the first component - // into render so currentComponent in hooks/index.js wasn't set yet. However, - // any children under App wouldn't have thrown the error if they did what App - // did because currentComponent would be set to App. - // In other words, hooks never clear currentComponent so once it is set, it won't - // be unset - it.skip('should throw an error when using a hook outside a render', () => { - const Foo = props => props.children; - class App extends Component { - componentWillMount() { - useState(); - } - - render() { - return

test

; - } - } - const fn = () => act(() => render(, scratch)); - expect(fn).to.throw(/Hook can only be invoked from render/); - }); - - // TODO: Fix this test. It only passed before because render was never called. - // Once render is called, currentComponent is set and never unset so calls to - // hooks outside of components would still work. - it.skip('should throw an error when invoked outside of a component', () => { - function Foo(props) { - useEffect(() => {}); // Pretend to use a hook - return props.children; - } - - const fn = () => act(() => { - render(Hello!, scratch); - useState(); - }); - expect(fn).to.throw(/Hook can only be invoked from render/); - }); - - it('should warn for argumentless useEffect hooks', () => { - const App = () => { - const [state] = useState('test'); - useEffect(() => 'test'); - return ( -

{state}

- ); - }; - render(, scratch); - expect(warnings[0]).to.match(/You should provide an array of arguments/); - render(, scratch); - expect(warnings[1]).to.be.undefined; - }); - - it('should warn for argumentless useLayoutEffect hooks', () => { - const App = () => { - const [state] = useState('test'); - useLayoutEffect(() => 'test'); - return ( -

{state}

- ); - }; - render(, scratch); - expect(warnings[0]).to.match(/You should provide an array of arguments/); - render(, scratch); - expect(warnings[1]).to.be.undefined; - }); - - it('should not warn for argumented effect hooks', () => { - const App = () => { - const [state] = useState('test'); - useLayoutEffect(() => 'test', []); - useEffect(() => 'test', [state]); - return ( -

{state}

- ); - }; - const fn = () => act(() => render(, scratch)); - expect(fn).to.not.throw(); - }); - - it('should print an error on double jsx conversion', () => { - let Foo =
; - let fn = () => render(h(), scratch); - expect(fn).to.throw(/createElement/); - }); - it('should throw errors when accessing certain attributes', () => { const vnode = h('div', null); expect(() => vnode).to.not.throw(); @@ -200,11 +114,6 @@ describe('debug', () => { expect(() => vnode.children = [
]).to.throw(/use vnode.props.children/); }); - it('should print an error when component is an array', () => { - let fn = () => render(h([
]), scratch); - expect(fn).to.throw(/createElement/); - }); - it('should warn when calling setState inside the constructor', () => { class Foo extends Component { constructor(props) { @@ -289,37 +198,9 @@ describe('debug', () => { expect(fn).to.throw(/not valid/); }); - it('should warn for useless useMemo calls', () => { - const App = () => { - const [people] = useState([40, 20, 60, 80]); - const retiredPeople = useMemo(() => people.filter(x => x >= 60)); - const cb = useCallback(() => () => 'test'); - return

{retiredPeople.map(x => x)}

; - }; - render(, scratch); - expect(warnings.length).to.equal(2); - }); - - it('should warn when non-array args is passed', () => { - const App = () => { - const foo = useMemo(() => 'foo', 12); - return

{foo}

; - }; - render(, scratch); - expect(warnings[0]).to.match(/without passing arguments/); - }); - it('should print an error on invalid refs', () => { let fn = () => render(
, scratch); expect(fn).to.throw(/createRef/); - - // Allow strings for compat - let vnode =
; - - /** @type {*} */ - (vnode).$$typeof = 'foo'; - render(vnode, scratch); - expect(console.error).to.not.be.called; }); it('should not print for null as a handler', () => { @@ -352,20 +233,6 @@ describe('debug', () => { expect(console.error).to.not.be.called; }); - it('should throw an error when missing Suspense', () => { - const Foo = () =>
Foo
; - const LazyComp = lazy(() => new Promise(resolve => resolve({ default: Foo }))); - const fn = () => { - render(( - - - - ), scratch); - }; - - expect(fn).to.throw(/Missing Suspense/gi); - }); - describe('duplicate keys', () => { const List = props =>
    {props.children}
; const ListItem = props =>
  • {props.children}
  • ; @@ -634,7 +501,6 @@ describe('debug', () => { }); }); - describe('PropTypes', () => { it('should fail if props don\'t match prop-types', () => { function Foo(props) { @@ -679,126 +545,5 @@ describe('debug', () => { render(, scratch); expect(console.error).to.not.be.called; }); - - it('should validate propTypes inside lazy()', () => { - const rerender = setupRerender(); - - function Baz(props) { - return

    {props.unhappy}

    ; - } - - Baz.propTypes = { - unhappy: function alwaysThrows(obj, key) { if (obj[key] === 'signal') throw Error('got prop inside lazy()'); } - }; - - - const loader = Promise.resolve({ default: Baz }); - const LazyBaz = lazy(() => loader); - - const suspense = ( - fallback...
    }> - - - ); - render(suspense, scratch); - - expect(console.error).to.not.be.called; - - return loader - .then(() => Promise.all(suspense._component._suspensions)) - .then(() => { - rerender(); - expect(errors.length).to.equal(1); - expect(errors[0].includes('got prop')).to.equal(true); - expect(serializeHtml(scratch)).to.equal('

    signal

    '); - }); - }); - - it('should throw on missing ', () => { - function Foo() { - throw Promise.resolve(); - } - - expect(() => render(, scratch)).to.throw; - }); - - describe('warn for PropTypes on lazy()', () => { - it('should log the function name', () => { - const loader = Promise.resolve({ default: function MyLazyLoadedComponent() { return
    Hi there
    ; } }); - const FakeLazy = lazy(() => loader); - FakeLazy.propTypes = {}; - const suspense = ( - fallback...
    } > - - - ); - render(suspense, scratch); - - return loader - .then(() => Promise.all(suspense._component._suspensions)) - .then(() => { - expect(console.warn).to.be.calledTwice; - expect(warnings[1].includes('MyLazyLoadedComponent')).to.equal(true); - }); - }); - - it('should log the displayName', () => { - function MyLazyLoadedComponent() { return
    Hi there
    ; } - MyLazyLoadedComponent.displayName = 'HelloLazy'; - const loader = Promise.resolve({ default: MyLazyLoadedComponent }); - const FakeLazy = lazy(() => loader); - FakeLazy.propTypes = {}; - const suspense = ( - fallback...
    } > - - - ); - render(suspense, scratch); - - return loader - .then(() => Promise.all(suspense._component._suspensions)) - .then(() => { - expect(console.warn).to.be.calledTwice; - expect(warnings[1].includes('HelloLazy')).to.equal(true); - }); - }); - - it('should not log a component if lazy throws', () => { - const loader = Promise.reject(new Error('Hey there')); - const FakeLazy = lazy(() => loader); - FakeLazy.propTypes = {}; - render( - fallback...
    } > - - , - scratch - ); - - return loader.catch(() => { - expect(console.warn).to.be.calledOnce; - }); - }); - - it('should not log a component if lazy\'s loader throws', () => { - const FakeLazy = lazy(() => { throw new Error('Hello'); }); - FakeLazy.propTypes = {}; - let error; - try { - render( - fallback...
    } > - - , - scratch - ); - } - catch (e) { - error = e; - } - - expect(console.warn).to.be.calledOnce; - expect(error).not.to.be.undefined; - expect(error.message).to.eql('Hello'); - }); - }); }); }); diff --git a/debug/test/browser/devtools.test.js b/debug/test/browser/devtools.test.js index 462ecb644c..33d1df598b 100644 --- a/debug/test/browser/devtools.test.js +++ b/debug/test/browser/devtools.test.js @@ -441,33 +441,6 @@ describe('devtools', () => { expect(() => render(null, scratch)).to.not.throw(); }); - it('should not overwrite existing options', () => { - let vnodeSpy = sinon.spy(); - let diffSpy = sinon.spy(); - let diffedSpy = sinon.spy(); - let commitSpy = sinon.spy(); - let unmountSpy = sinon.spy(); - - options.vnode = vnodeSpy; - options._diff = diffSpy; - options.diffed = diffedSpy; - options.unmount = unmountSpy; - options._commit = commitSpy; - - initDevTools(); - - render(
    , scratch); - - expect(vnodeSpy, 'vnode').to.have.been.called; - expect(diffSpy, 'diff').to.have.been.calledTwice; - expect(diffedSpy, 'diffed').to.have.been.calledTwice; - expect(commitSpy, 'commit').to.have.been.calledOnce; - - render(null, scratch); - - expect(unmountSpy, 'unmount').to.have.been.calledOnce; - }); - it('should connect only once', () => { let rid = Object.keys(hook._renderers)[0]; let spy = sinon.spy(hook.helpers[rid], 'markConnected');