Skip to content

Commit

Permalink
fix(hooks): suppressRefError improvement (#1618)
Browse files Browse the repository at this point in the history
  • Loading branch information
silviuaavram authored Aug 12, 2024
1 parent 59246b6 commit 59d3498
Show file tree
Hide file tree
Showing 4 changed files with 60 additions and 13 deletions.
14 changes: 14 additions & 0 deletions src/hooks/useCombobox/__tests__/getInputProps.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -2085,6 +2085,7 @@ describe('getInputProps', () => {

describe('non production errors', () => {
beforeEach(() => {
// usually it is disabled by test utils.
const {useGetterPropsCalledChecker} = jest.requireActual('../../utils')
jest
.spyOn(utils, 'useGetterPropsCalledChecker')
Expand Down Expand Up @@ -2138,6 +2139,19 @@ describe('getInputProps', () => {
)
})

test('will not be displayed if element ref is not set and suppressRefError is true', () => {
renderHook(() => {
const {getInputProps, getMenuProps} = useCombobox({
items,
})

getMenuProps({}, {suppressRefError: true})
getInputProps({}, {suppressRefError: true})
})

expect(console.error).not.toHaveBeenCalled()
})

test('will not be displayed if called with a correct ref', () => {
const refFn = jest.fn()
const inputNode = {}
Expand Down
14 changes: 14 additions & 0 deletions src/hooks/useCombobox/__tests__/getMenuProps.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -135,6 +135,7 @@ describe('getMenuProps', () => {

describe('non production errors', () => {
beforeEach(() => {
// usally disabled by test utils.
const {useGetterPropsCalledChecker} = jest.requireActual('../../utils')
jest
.spyOn(utils, 'useGetterPropsCalledChecker')
Expand Down Expand Up @@ -191,6 +192,19 @@ describe('getMenuProps', () => {
)
})

test('will not be displayed if element ref is not set and suppressRefError is true', () => {
renderHook(() => {
const {getInputProps, getMenuProps} = useCombobox({
items,
})

getMenuProps({}, {suppressRefError: true})
getInputProps({}, {suppressRefError: true})
})

expect(console.error).not.toHaveBeenCalled()
})

test('will not be displayed if called with a correct ref', () => {
const refFn = jest.fn()
const menuNode = {}
Expand Down
18 changes: 18 additions & 0 deletions src/hooks/useMultipleSelection/__tests__/getDropdownProps.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -244,6 +244,7 @@ describe('getDropdownProps', () => {

describe('non production errors', () => {
beforeEach(() => {
// usually disabled by test utils.
const {useGetterPropsCalledChecker} = jest.requireActual('../../utils')
jest
.spyOn(utils, 'useGetterPropsCalledChecker')
Expand All @@ -262,6 +263,23 @@ describe('getDropdownProps', () => {
)
})

test('will not be displayed if getDropdownProps is not called on subsequent renders', () => {
let firstRender = true
const {rerender} = renderHook(() => {
const {getDropdownProps} = useMultipleSelection()

// eslint-disable-next-line jest/no-if, jest/no-conditional-in-test
if (firstRender) {
firstRender = false
getDropdownProps({}, {suppressRefError: true})
}
})

rerender()

expect(console.error).not.toHaveBeenCalled()
})

test('will be displayed if element ref is not set and suppressRefError is false', () => {
renderHook(() => {
const {getDropdownProps} = useMultipleSelection()
Expand Down
27 changes: 14 additions & 13 deletions src/hooks/utils.js
Original file line number Diff line number Diff line change
Expand Up @@ -448,39 +448,40 @@ let useGetterPropsCalledChecker = () => noop
/* istanbul ignore next */
if (process.env.NODE_ENV !== 'production') {
useGetterPropsCalledChecker = (...propKeys) => {
const isInitialMountRef = useRef(true)
const getterPropsCalledRef = useRef(
propKeys.reduce((acc, propKey) => {
acc[propKey] = {}

return acc
}, {}),
)

useEffect(() => {
Object.keys(getterPropsCalledRef.current).forEach(propKey => {
const propCallInfo = getterPropsCalledRef.current[propKey]
if (isInitialMountRef.current) {
if (!Object.keys(propCallInfo).length) {
// eslint-disable-next-line no-console
console.error(
`downshift: You forgot to call the ${propKey} getter function on your component / element.`,
)
return
}

if (!Object.keys(propCallInfo).length) {
// eslint-disable-next-line no-console
console.error(
`downshift: You forgot to call the ${propKey} getter function on your component / element.`,
)
return
}

const {suppressRefError, refKey, elementRef} = propCallInfo

if ((!elementRef || !elementRef.current) && !suppressRefError) {
if (suppressRefError) {
return
}

if (!elementRef?.current) {
// eslint-disable-next-line no-console
console.error(
`downshift: The ref prop "${refKey}" from ${propKey} was not applied correctly on your element.`,
)
}
})

isInitialMountRef.current = false
})
}, [])

const setGetterPropCallInfo = useCallback(
(propKey, suppressRefError, refKey, elementRef) => {
Expand Down

0 comments on commit 59d3498

Please sign in to comment.