-
-
Notifications
You must be signed in to change notification settings - Fork 1.9k
Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
Separate debug tests into multiple files and fix some bugs in debug (#…
…2047) Previously the debug tests attempted to reset the options object before each test (I think at my suggestion originally lol) to reset the debug state before each test. However, by deleting the options properties, it deleted the hooks options and compat options. It also didn't delete every option so some options would continue to work. This behavior lead to tests that weren't resilient to changes and didn't capture bugs. To improve our debug tests, I separated the tests out into different files. Each test file only initializes the debug options once. This behavior better mimics what our users do making our tests more closely match our user's behaviors. Further, some debug behavior changes based on which modules you import (e.g. if you use compat we don't warn on string refs). So separating out the debug tests into different files (one test file imports compat, another doesn't) allows us to simulate these differing behaviors more reliable. If nothing else, the debug.test.js file was pretty big so making it a little smaller is kinda nice :) This PR also fixes a bug where if different instances of the same component wouldn't print warnings since they share the same constructor (more details in the comments of the PR).
- Loading branch information
Showing
5 changed files
with
345 additions
and
309 deletions.
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -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 <p>test</p>; | ||
| } | ||
| } | ||
| const fn = () => | ||
| act(() => | ||
| render( | ||
| <Foo> | ||
| <App /> | ||
| </Foo>, | ||
| 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(<Foo>Hello!</Foo>, 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 <p>{state}</p>; | ||
| }; | ||
| render(<App />, scratch); | ||
| expect(warnings[0]).to.match(/You should provide an array of arguments/); | ||
| render(<App />, scratch); | ||
| expect(warnings[1]).to.be.undefined; | ||
| }); | ||
|
|
||
| it('should warn for argumentless useLayoutEffect hooks', () => { | ||
| const App = () => { | ||
| const [state] = useState('test'); | ||
| useLayoutEffect(() => 'test'); | ||
| return <p>{state}</p>; | ||
| }; | ||
| render(<App />, scratch); | ||
| expect(warnings[0]).to.match(/You should provide an array of arguments/); | ||
| render(<App />, 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 <p>{state}</p>; | ||
| }; | ||
| const fn = () => act(() => render(<App />, 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 <p onClick={cb}>{retiredPeople.map(x => x)}</p>; | ||
| }; | ||
| render(<App />, scratch); | ||
| expect(warnings.length).to.equal(2); | ||
| }); | ||
|
|
||
| it('should warn when non-array args is passed', () => { | ||
| const App = () => { | ||
| const foo = useMemo(() => 'foo', 12); | ||
| return <p>{foo}</p>; | ||
| }; | ||
| render(<App />, scratch); | ||
| expect(warnings[0]).to.match(/without passing arguments/); | ||
| }); | ||
| }); |
Oops, something went wrong.