diff --git a/debug/test/browser/debug-hooks.test.js b/debug/test/browser/debug-hooks.test.js index a566d15d3c..76891708e4 100644 --- a/debug/test/browser/debug-hooks.test.js +++ b/debug/test/browser/debug-hooks.test.js @@ -25,19 +25,18 @@ describe('debug with hooks', () => { 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 { + it('should throw an error when using a hook outside a render', () => { + class Foo extends Component { componentWillMount() { useState(); } + render() { + return this.props.children; + } + } + + class App extends Component { render() { return

test

; } @@ -81,6 +80,20 @@ describe('debug with hooks', () => { expect(fn).to.throw(/Hook can only be invoked from render/); }); + it('should throw an error when invoked outside of a component after render', () => { + 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 throw an error when invoked inside an effect callback', () => { function Foo(props) { useEffect(() => { diff --git a/hooks/src/index.js b/hooks/src/index.js index 57cfcbc50d..5a8c4fdeb9 100644 --- a/hooks/src/index.js +++ b/hooks/src/index.js @@ -5,6 +5,13 @@ let currentIndex; /** @type {import('./internal').Component} */ let currentComponent; +/** + * Keep track of the previous component so that we can set + * `currentComponent` to `null` and throw when a hook is invoked + * outside of render + * @type {import('./internal').Component} + */ +let previousComponent; /** @type {number} */ let currentHook = 0; @@ -12,6 +19,7 @@ let currentHook = 0; /** @type {Array} */ let afterPaintEffects = []; +let oldBeforeDiff = options._diff; let oldBeforeRender = options._render; let oldAfterDiff = options.diffed; let oldCommit = options._commit; @@ -20,6 +28,11 @@ let oldBeforeUnmount = options.unmount; const RAF_TIMEOUT = 100; let prevRaf; +options._diff = vnode => { + currentComponent = null; + if (oldBeforeDiff) oldBeforeDiff(vnode); +}; + options._render = vnode => { if (oldBeforeRender) oldBeforeRender(vnode); @@ -41,6 +54,7 @@ options.diffed = vnode => { if (c && c.__hooks && c.__hooks._pendingEffects.length) { afterPaint(afterPaintEffects.push(c)); } + currentComponent = previousComponent; }; options._commit = (vnode, commitQueue) => {