From fd8905a25ec036c35a6fe766e703713ba778182e Mon Sep 17 00:00:00 2001 From: macinjoke Date: Wed, 4 Sep 2019 02:48:21 +0900 Subject: [PATCH 1/2] add useEvent test --- src/__tests__/useEvent.test.ts | 118 +++++++++++++++++++++++++++++++++ 1 file changed, 118 insertions(+) create mode 100644 src/__tests__/useEvent.test.ts diff --git a/src/__tests__/useEvent.test.ts b/src/__tests__/useEvent.test.ts new file mode 100644 index 00000000..ec124600 --- /dev/null +++ b/src/__tests__/useEvent.test.ts @@ -0,0 +1,118 @@ +import { renderHook } from '@testing-library/react-hooks'; +import useEvent, { ListenerType1, ListenerType2 } from '../useEvent'; + +interface Props { + name: string; + handler: (...args: any[]) => void; + target: ListenerType1 | ListenerType2; + options: any; +} + +const propsList1 = [ + { + name: 'name1', + handler: () => {}, + target: { + addEventListener: jest.fn(), + removeEventListener: jest.fn(), + }, + options: { a: 'opt1' }, + }, + { + name: 'name2', + handler: () => {}, + target: { + addEventListener: jest.fn(), + removeEventListener: jest.fn(), + }, + options: { a: 'opt2' }, + }, +]; + +const propsList2 = [ + { + ...propsList1[0], + target: { + on: jest.fn(), + off: jest.fn(), + }, + }, + { + ...propsList1[1], + target: { + on: jest.fn(), + off: jest.fn(), + }, + }, +]; + +it('should call addEventListener/removeEventListener on mount/unmount', () => { + checkOnMountAndUnmount(propsList1[0], 'addEventListener', 'removeEventListener'); +}); + +it('should call on/off on mount/unmount', () => { + checkOnMountAndUnmount(propsList2[0], 'on', 'off'); +}); + +it('should call addEventListener/removeEventListener on deps changes', () => { + checkOnDepsChanges(propsList1[0], propsList1[1], 'addEventListener', 'removeEventListener'); +}); + +it('should call on/off on deps changes', () => { + checkOnDepsChanges(propsList2[0], propsList2[1], 'on', 'off'); +}); + +const checkOnMountAndUnmount = (props: Props, addEventListenerName: string, removeEventListenerName: string) => { + const { unmount } = renderHook((p: Props) => useEvent(p.name, p.handler, p.target, p.options), { + initialProps: props, + }); + expect(props.target[addEventListenerName]).toHaveBeenCalledTimes(1); + expect(props.target[addEventListenerName]).toHaveBeenLastCalledWith(props.name, props.handler, props.options); + unmount(); + expect(props.target[removeEventListenerName]).toHaveBeenCalledTimes(1); + expect(props.target[removeEventListenerName]).toHaveBeenLastCalledWith(props.name, props.handler, props.options); +}; + +const checkOnDepsChanges = ( + props1: Props, + props2: Props, + addEventListenerName: string, + removeEventListenerName: string +) => { + const { rerender } = renderHook((p: Props) => useEvent(p.name, p.handler, p.target, p.options), { + initialProps: props1, + }); + expect(props1.target[addEventListenerName]).toHaveBeenCalledTimes(1); + expect(props1.target[addEventListenerName]).toHaveBeenLastCalledWith(props1.name, props1.handler, props1.options); + + // deps are same as previous + rerender({ name: props1.name, handler: props1.handler, target: props1.target, options: props1.options }); + expect(props1.target[removeEventListenerName]).not.toHaveBeenCalled(); + + // name is different from previous + rerender({ name: props2.name, handler: props1.handler, target: props1.target, options: props1.options }); + expect(props1.target[removeEventListenerName]).toHaveBeenCalledTimes(1); + expect(props1.target[removeEventListenerName]).toHaveBeenLastCalledWith(props1.name, props1.handler, props1.options); + + // handler is different from previous + rerender({ name: props2.name, handler: props2.handler, target: props1.target, options: props1.options }); + expect(props1.target[removeEventListenerName]).toHaveBeenCalledTimes(2); + expect(props1.target[removeEventListenerName]).toHaveBeenLastCalledWith(props2.name, props1.handler, props1.options); + + // options contents is same as previous + rerender({ name: props2.name, handler: props2.handler, target: props1.target, options: { a: 'opt1' } }); + expect(props1.target[removeEventListenerName]).toHaveBeenCalledTimes(2); + + // options is different from previous + rerender({ name: props2.name, handler: props2.handler, target: props1.target, options: props2.options }); + expect(props1.target[removeEventListenerName]).toHaveBeenCalledTimes(3); + expect(props1.target[removeEventListenerName]).toHaveBeenLastCalledWith(props2.name, props2.handler, props1.options); + + // target is different from previous + rerender({ name: props2.name, handler: props2.handler, target: props2.target, options: props2.options }); + expect(props1.target[removeEventListenerName]).toHaveBeenCalledTimes(4); + expect(props1.target[removeEventListenerName]).toHaveBeenLastCalledWith(props2.name, props2.handler, props2.options); + + expect(props2.target[addEventListenerName]).toHaveBeenCalledTimes(1); + expect(props2.target[addEventListenerName]).toHaveBeenLastCalledWith(props2.name, props2.handler, props2.options); +}; From 0fb314863eacaf80772631626a5db416e5f4c698 Mon Sep 17 00:00:00 2001 From: macinjoke Date: Fri, 6 Sep 2019 23:14:34 +0900 Subject: [PATCH 2/2] improve useEvent type inference --- src/useEvent.ts | 39 ++++++++++++++++++++++++++------------- 1 file changed, 26 insertions(+), 13 deletions(-) diff --git a/src/useEvent.ts b/src/useEvent.ts index 3074df6d..b9c1ec4c 100644 --- a/src/useEvent.ts +++ b/src/useEvent.ts @@ -3,25 +3,32 @@ import { isClient } from './util'; export interface ListenerType1 { addEventListener(name: string, handler: (event?: any) => void, ...args: any[]); - - removeEventListener(name: string, handler: (event?: any) => void); + removeEventListener(name: string, handler: (event?: any) => void, ...args: any[]); } export interface ListenerType2 { on(name: string, handler: (event?: any) => void, ...args: any[]); - - off(name: string, handler: (event?: any) => void); + off(name: string, handler: (event?: any) => void, ...args: any[]); } export type UseEventTarget = ListenerType1 | ListenerType2; const defaultTarget = isClient ? window : null; -const useEvent = ( - name: string, - handler?: null | undefined | ((event?: any) => void), - target: null | UseEventTarget = defaultTarget, - options?: any +const isListenerType1 = (target: any): target is ListenerType1 => { + return !!target.addEventListener; +}; +const isListenerType2 = (target: any): target is ListenerType2 => { + return !!target.on; +}; + +type AddEventListener = T extends ListenerType1 ? T['addEventListener'] : T extends ListenerType2 ? T['on'] : never; + +const useEvent = ( + name: Parameters>[0], + handler?: null | undefined | Parameters>[1], + target: null | T | Window = defaultTarget, + options?: Parameters>[2] ) => { useEffect(() => { if (!handler) { @@ -30,11 +37,17 @@ const useEvent = ( if (!target) { return; } - const fn: any = (target as ListenerType1).addEventListener || (target as ListenerType2).on; - fn.call(target, name, handler, options); + if (isListenerType1(target)) { + target.addEventListener(name, handler, options); + } else if (isListenerType2(target)) { + target.on(name, handler, options); + } return () => { - const cleanFn: any = (target as ListenerType1).removeEventListener || (target as ListenerType2).off; - cleanFn.call(target, name, handler, options); + if (isListenerType1(target)) { + target.removeEventListener(name, handler, options); + } else if (isListenerType2(target)) { + target.off(name, handler, options); + } }; }, [name, handler, target, JSON.stringify(options)]); };