Skip to content

Commit dc21e59

Browse files
authored
fix: fixed potential error when hook suspends and current result is accessed
BREAKING CHANGE: Adjust types so that react renderer exports don't required extra generic parameter
1 parent 43891e1 commit dc21e59

File tree

14 files changed

+110
-73
lines changed

14 files changed

+110
-73
lines changed

package.json

+1-1
Original file line numberDiff line numberDiff line change
@@ -56,7 +56,7 @@
5656
"docz-theme-default": "1.2.0",
5757
"docz-utils": "2.3.0",
5858
"eslint": "7.17.0",
59-
"kcd-scripts": "7.5.3",
59+
"kcd-scripts": "7.5.4",
6060
"prettier": "^2.2.1",
6161
"react": "17.0.1",
6262
"react-dom": "^17.0.1",

src/__tests__/defaultRenderer.test.ts

+41
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,41 @@
1+
/* eslint-disable @typescript-eslint/no-var-requires */
2+
import { ReactHooksRenderer } from '../types/react'
3+
4+
describe('default renderer', () => {
5+
beforeEach(() => {
6+
jest.resetModules()
7+
})
8+
9+
test('should resolve native renderer as default renderer', () => {
10+
const expectedRenderer = require('../native/pure') as ReactHooksRenderer
11+
const actualRenderer = require('..') as ReactHooksRenderer
12+
13+
expect(actualRenderer).toEqual(expectedRenderer)
14+
})
15+
16+
test('should resolve dom renderer as default renderer', () => {
17+
jest.doMock('react-test-renderer', () => {
18+
throw new Error('missing dependency')
19+
})
20+
21+
const expectedRenderer = require('../dom/pure') as ReactHooksRenderer
22+
const actualRenderer = require('..') as ReactHooksRenderer
23+
24+
expect(actualRenderer).toEqual(expectedRenderer)
25+
})
26+
27+
test('should throw error if a default renderer cannot be resolved', () => {
28+
jest.doMock('react-test-renderer', () => {
29+
throw new Error('missing dependency')
30+
})
31+
32+
jest.doMock('react-dom', () => {
33+
throw new Error('missing dependency')
34+
})
35+
36+
const expectedMessage =
37+
"Could not auto-detect a React renderer. Are you sure you've installed one of the following\n - react-dom\n - react-test-renderer"
38+
39+
expect(() => require('..')).toThrowError(new Error(expectedMessage))
40+
})
41+
})

src/core/cleanup.ts

+5-3
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,6 @@
1-
let cleanupCallbacks: (() => Promise<void> | void)[] = []
1+
import { CleanupCallback } from '../types'
2+
3+
let cleanupCallbacks: Array<CleanupCallback> = []
24

35
async function cleanup() {
46
for (const callback of cleanupCallbacks) {
@@ -7,12 +9,12 @@ async function cleanup() {
79
cleanupCallbacks = []
810
}
911

10-
function addCleanup(callback: () => Promise<void> | void) {
12+
function addCleanup(callback: CleanupCallback) {
1113
cleanupCallbacks = [callback, ...cleanupCallbacks]
1214
return () => removeCleanup(callback)
1315
}
1416

15-
function removeCleanup(callback: () => Promise<void> | void) {
17+
function removeCleanup(callback: CleanupCallback) {
1618
cleanupCallbacks = cleanupCallbacks.filter((cb) => cb !== callback)
1719
}
1820

src/core/index.ts

+9-15
Original file line numberDiff line numberDiff line change
@@ -1,19 +1,18 @@
1-
import { CreateRenderer, Renderer, RenderResult, RenderHook, RenderHookOptions } from '../types'
2-
import { ResultContainer } from '../types/internal'
1+
import { CreateRenderer, Renderer, RenderResult, RenderHookOptions } from '../types'
32

43
import { asyncUtils } from './asyncUtils'
54
import { cleanup, addCleanup, removeCleanup } from './cleanup'
65

7-
function resultContainer<TValue>(): ResultContainer<TValue> {
6+
function resultContainer<TValue>() {
87
const results: Array<{ value?: TValue; error?: Error }> = []
98
const resolvers: Array<() => void> = []
109

1110
const result: RenderResult<TValue> = {
1211
get all() {
13-
return results.map(({ value, error }) => error ?? value)
12+
return results.map(({ value, error }) => error ?? (value as TValue))
1413
},
1514
get current() {
16-
const { value, error } = results[results.length - 1]
15+
const { value, error } = results[results.length - 1] ?? {}
1716
if (error) {
1817
throw error
1918
}
@@ -43,12 +42,12 @@ function resultContainer<TValue>(): ResultContainer<TValue> {
4342
function createRenderHook<
4443
TProps,
4544
TResult,
46-
TOptions extends object,
45+
TRendererOptions extends object,
4746
TRenderer extends Renderer<TProps>
48-
>(createRenderer: CreateRenderer<TProps, TResult, TOptions, TRenderer>) {
49-
const renderHook: RenderHook<TProps, TResult, TOptions, TRenderer> = (
50-
callback,
51-
options = {} as RenderHookOptions<TProps, TOptions>
47+
>(createRenderer: CreateRenderer<TProps, TResult, TRendererOptions, TRenderer>) {
48+
const renderHook = (
49+
callback: (props: TProps) => TResult,
50+
options = {} as RenderHookOptions<TProps> & TRendererOptions
5251
) => {
5352
const { result, setValue, setError, addResolver } = resultContainer<TResult>()
5453
const renderProps = { callback, setValue, setError }
@@ -79,11 +78,6 @@ function createRenderHook<
7978
}
8079
}
8180

82-
// If the function name does not get used before it is returned,
83-
// it's name is removed by babel-plugin-minify-dead-code-elimination.
84-
// This dummy usage works around that.
85-
renderHook.name // eslint-disable-line @typescript-eslint/no-unused-expressions
86-
8781
return renderHook
8882
}
8983

src/dom/__tests__/suspenseHook.test.ts

+6
Original file line numberDiff line numberDiff line change
@@ -46,4 +46,10 @@ describe('suspense hook tests', () => {
4646

4747
expect(result.error).toEqual(new Error('Failed to fetch name'))
4848
})
49+
50+
test('should return undefined if current value is requested before suspension has resolved', async () => {
51+
const { result } = renderHook(() => useFetchName(true))
52+
53+
expect(result.current).toBe(undefined)
54+
})
4955
})

src/dom/pure.ts

+5-5
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,9 @@
11
import ReactDOM from 'react-dom'
22
import { act } from 'react-dom/test-utils'
33

4-
import { RendererProps } from '../types'
5-
import { RendererOptions } from '../types/react'
4+
import { RendererProps, RendererOptions } from '../types/react'
65

7-
import { createRenderHook, cleanup, addCleanup, removeCleanup } from '../core'
6+
import { createRenderHook } from '../core'
87
import { createTestHarness } from '../helpers/createTestHarness'
98

109
function createDomRenderer<TProps, TResult>(
@@ -39,7 +38,8 @@ function createDomRenderer<TProps, TResult>(
3938

4039
const renderHook = createRenderHook(createDomRenderer)
4140

42-
export { renderHook, act, cleanup, addCleanup, removeCleanup }
41+
export { renderHook, act }
42+
43+
export { cleanup, addCleanup, removeCleanup } from '../core'
4344

44-
export * from '../types'
4545
export * from '../types/react'

src/helpers/createTestHarness.tsx

+1-7
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,6 @@
11
import React, { Suspense } from 'react'
22

3-
import { RendererProps } from '../types'
4-
import { WrapperComponent } from '../types/react'
3+
import { RendererProps, WrapperComponent } from '../types/react'
54

65
import { isPromise } from './promises'
76

@@ -40,11 +39,6 @@ function createTestHarness<TProps, TResult>(
4039
return component
4140
}
4241

43-
// If the function name does not get used before it is returned,
44-
// it's name is removed by babel-plugin-minify-dead-code-elimination.
45-
// This dummy usage works around that.
46-
testHarness.name // eslint-disable-line @typescript-eslint/no-unused-expressions
47-
4842
return testHarness
4943
}
5044

src/native/__tests__/suspenseHook.test.ts

+6
Original file line numberDiff line numberDiff line change
@@ -46,4 +46,10 @@ describe('suspense hook tests', () => {
4646

4747
expect(result.error).toEqual(new Error('Failed to fetch name'))
4848
})
49+
50+
test('should return undefined if current value is requested before suspension has resolved', async () => {
51+
const { result } = renderHook(() => useFetchName(true))
52+
53+
expect(result.current).toBe(undefined)
54+
})
4955
})

src/native/pure.ts

+5-5
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,8 @@
11
import { act, create, ReactTestRenderer } from 'react-test-renderer'
22

3-
import { RendererProps } from '../types'
4-
import { RendererOptions } from '../types/react'
3+
import { RendererProps, RendererOptions } from '../types/react'
54

6-
import { createRenderHook, cleanup, addCleanup, removeCleanup } from '../core'
5+
import { createRenderHook } from '../core'
76
import { createTestHarness } from '../helpers/createTestHarness'
87

98
function createNativeRenderer<TProps, TResult>(
@@ -36,7 +35,8 @@ function createNativeRenderer<TProps, TResult>(
3635

3736
const renderHook = createRenderHook(createNativeRenderer)
3837

39-
export { renderHook, act, cleanup, addCleanup, removeCleanup }
38+
export { renderHook, act }
39+
40+
export { cleanup, addCleanup, removeCleanup } from '../core'
4041

41-
export * from '../types'
4242
export * from '../types/react'

src/pure.ts

-1
Original file line numberDiff line numberDiff line change
@@ -36,5 +36,4 @@ const { renderHook, act, cleanup, addCleanup, removeCleanup } = getRenderer()
3636

3737
export { renderHook, act, cleanup, addCleanup, removeCleanup }
3838

39-
export * from './types'
4039
export * from './types/react'

src/server/pure.ts

+5-5
Original file line numberDiff line numberDiff line change
@@ -2,10 +2,9 @@ import ReactDOMServer from 'react-dom/server'
22
import ReactDOM from 'react-dom'
33
import { act } from 'react-dom/test-utils'
44

5-
import { RendererProps } from '../types'
6-
import { RendererOptions } from '../types/react'
5+
import { RendererProps, RendererOptions } from '../types/react'
76

8-
import { createRenderHook, cleanup, addCleanup, removeCleanup } from '../core'
7+
import { createRenderHook } from '../core'
98
import { createTestHarness } from '../helpers/createTestHarness'
109

1110
function createServerRenderer<TProps, TResult>(
@@ -60,7 +59,8 @@ function createServerRenderer<TProps, TResult>(
6059

6160
const renderHook = createRenderHook(createServerRenderer)
6261

63-
export { renderHook, act, cleanup, addCleanup, removeCleanup }
62+
export { renderHook, act }
63+
64+
export { cleanup, addCleanup, removeCleanup } from '../core'
6465

65-
export * from '../types'
6666
export * from '../types/react'

src/types/index.ts

+11-19
Original file line numberDiff line numberDiff line change
@@ -11,13 +11,15 @@ export type RendererProps<TProps, TResult> = {
1111
setValue: (value: TResult) => void
1212
}
1313

14-
export type CreateRenderer<TProps, TResult, TOptions, TRenderer extends Renderer<TProps>> = (
15-
props: RendererProps<TProps, TResult>,
16-
options: TOptions
17-
) => TRenderer
14+
export type CreateRenderer<
15+
TProps,
16+
TResult,
17+
TRendererOptions extends object,
18+
TRenderer extends Renderer<TProps>
19+
> = (props: RendererProps<TProps, TResult>, options: TRendererOptions) => TRenderer
1820

1921
export type RenderResult<TValue> = {
20-
readonly all: (TValue | Error | undefined)[]
22+
readonly all: Array<TValue | Error>
2123
readonly current: TValue
2224
readonly error?: Error
2325
}
@@ -47,23 +49,13 @@ export type RenderHookResult<
4749
Omit<TRenderer, keyof Renderer<TProps>> &
4850
AsyncUtils
4951

50-
export type RenderHookOptions<TProps, TOptions extends object> = TOptions & {
52+
export type RenderHookOptions<TProps> = {
5153
initialProps?: TProps
5254
}
5355

54-
export interface RenderHook<
55-
TProps,
56-
TResult,
57-
TOptions extends object,
58-
TRenderer extends Renderer<TProps> = Renderer<TProps>
59-
> {
60-
(
61-
callback: (props: TProps) => TResult,
62-
options?: RenderHookOptions<TProps, TOptions>
63-
): RenderHookResult<TProps, TResult, TRenderer>
64-
}
65-
66-
export interface Act {
56+
export type Act = {
6757
(callback: () => void | undefined): void
6858
(callback: () => Promise<void | undefined>): Promise<undefined>
6959
}
60+
61+
export type CleanupCallback = () => Promise<void> | void

src/types/internal.ts

-8
This file was deleted.

src/types/react.ts

+15-4
Original file line numberDiff line numberDiff line change
@@ -1,20 +1,31 @@
11
import { ComponentType } from 'react'
22

3-
import { RenderHookOptions, RenderHookResult, Act } from '.'
3+
import {
4+
RenderHookOptions as BaseRenderHookOptions,
5+
RenderHookResult,
6+
Act,
7+
CleanupCallback
8+
} from '.'
49

510
export type WrapperComponent<TProps> = ComponentType<TProps>
611

712
export type RendererOptions<TProps> = {
813
wrapper?: WrapperComponent<TProps>
914
}
1015

16+
export type RenderHookOptions<TProps> = BaseRenderHookOptions<TProps> & {
17+
wrapper?: WrapperComponent<TProps>
18+
}
19+
1120
export type ReactHooksRenderer = {
1221
renderHook: <TProps, TResult>(
1322
callback: (props: TProps) => TResult,
14-
options?: RenderHookOptions<TProps, RendererOptions<TProps>>
23+
options?: RenderHookOptions<TProps>
1524
) => RenderHookResult<TProps, TResult>
1625
act: Act
1726
cleanup: () => void
18-
addCleanup: (callback: () => Promise<void> | void) => () => void
19-
removeCleanup: (callback: () => Promise<void> | void) => void
27+
addCleanup: (callback: CleanupCallback) => () => void
28+
removeCleanup: (callback: CleanupCallback) => void
2029
}
30+
31+
export * from '.'

0 commit comments

Comments
 (0)