Skip to content

Commit afacdcc

Browse files
joeluong-sfccwjhsf
andauthored
@W-14629134@ Fix error component regression (#1609)
* pass intl object as prop instead of calling hook * add proptypes * add unit tests * remove unused vars caught by linter * Update packages/template-retail-react-app/app/components/_error/index.test.js Co-authored-by: Will Harney <62956339+wjhsf@users.noreply.github.com> Signed-off-by: Joel Uong <88680517+joeluong-sfcc@users.noreply.github.com> * update unit tests * remove intl prop from LockIcon --------- Signed-off-by: Joel Uong <88680517+joeluong-sfcc@users.noreply.github.com> Co-authored-by: Will Harney <62956339+wjhsf@users.noreply.github.com>
1 parent d4dcf24 commit afacdcc

File tree

5 files changed

+71
-11
lines changed

5 files changed

+71
-11
lines changed

packages/template-retail-react-app/app/components/_error/index.test.js

Lines changed: 8 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -5,21 +5,24 @@
55
* For full license text, see the LICENSE file in the repo root or https://opensource.org/licenses/BSD-3-Clause
66
*/
77
import React from 'react'
8-
import {screen} from '@testing-library/react'
9-
import {renderWithProviders} from '@salesforce/retail-react-app/app/utils/test-utils'
108
import Error from '@salesforce/retail-react-app/app/components/_error/index'
9+
// !!! ----- WARNING ----- WARNING ----- WARNING ----- !!!
10+
// Tests use render instead of renderWithProviders because
11+
// error component is rendered outside provider tree
12+
// !!! ----------------------------------------------- !!!
13+
import {screen, render} from '@testing-library/react'
1114

1215
test('Error renders without errors', () => {
13-
expect(renderWithProviders(<Error />)).toBeDefined()
16+
expect(render(<Error />)).toBeDefined()
1417
})
1518

1619
test('Error status 500', () => {
17-
renderWithProviders(<Error status={500} />)
20+
render(<Error status={500} />)
1821
expect(screen.getByRole('heading', {level: 2})).toHaveTextContent("This page isn't working")
1922
})
2023

2124
test('Error status 500 with stack trace', () => {
22-
renderWithProviders(<Error status={500} stack={'Stack trace error message'} />)
25+
render(<Error status={500} stack={'Stack trace error message'} />)
2326
expect(screen.getByRole('heading', {level: 2})).toHaveTextContent("This page isn't working")
2427
expect(screen.getByText(/stack trace error message/i)).toBeInTheDocument()
2528
})

packages/template-retail-react-app/app/components/icons/index.jsx

Lines changed: 25 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -4,8 +4,9 @@
44
* SPDX-License-Identifier: BSD-3-Clause
55
* For full license text, see the LICENSE file in the repo root or https://opensource.org/licenses/BSD-3-Clause
66
*/
7-
import React, {forwardRef} from 'react'
8-
import {useIntl, defineMessage} from 'react-intl'
7+
import React, {forwardRef, useContext} from 'react'
8+
import {defineMessage, IntlContext} from 'react-intl'
9+
import PropTypes from 'prop-types'
910
import {Icon, useTheme} from '@salesforce/retail-react-app/app/components/shared/ui'
1011

1112
// Our own SVG imports. These will be extracted to a single sprite sheet by the
@@ -85,7 +86,9 @@ VisaSymbol.viewBox = VisaSymbol.viewBox || '0 0 38 22'
8586
* @param {string} name - the filename of the imported svg (does not include extension)
8687
* @param {Object} passProps - props that will be passed onto the underlying Icon component
8788
* @param {Object} localizationAttributes - attributes with localized values that will be passed
88-
* onto the underlying Icon component, use `defineMessage` to create localized string
89+
* onto the underlying Icon component, use `defineMessage` to create localized string.
90+
* Additionally, if the icon is rendered outside the provider tree, you'll also need to
91+
* pass an intl object from react-intl as a prop to translate the messages.
8992
*/
9093
/* istanbul ignore next */
9194
export const icon = (name, passProps, localizationAttributes) => {
@@ -95,8 +98,21 @@ export const icon = (name, passProps, localizationAttributes) => {
9598
.replace(/-/g, '')
9699
const component = forwardRef((props, ref) => {
97100
const theme = useTheme()
98-
const intl = useIntl()
101+
// NOTE: We want to avoid `useIntl` here because that throws when <IntlProvider> is not in
102+
// the component ancestry, but we only enforce `intl` if we have `localizationAttributes`.
103+
let intl = useContext(IntlContext)
99104
if (localizationAttributes) {
105+
if (props?.intl) {
106+
const {intl: intlProp, ...otherProps} = props
107+
// Allow `props.intl` to take precedence over the intl we found
108+
intl = intlProp
109+
props = otherProps
110+
}
111+
if (!intl) {
112+
throw new Error(
113+
'To localize messages, you must either have <IntlProvider> in the component ancestry or provide `intl` as a prop'
114+
)
115+
}
100116
Object.keys(localizationAttributes).forEach((key) => {
101117
passProps[key] = intl.formatMessage(localizationAttributes[key])
102118
})
@@ -108,6 +124,11 @@ export const icon = (name, passProps, localizationAttributes) => {
108124
</Icon>
109125
)
110126
})
127+
128+
component.propTypes = {
129+
intl: PropTypes.object
130+
}
131+
111132
component.displayName = `${displayName}Icon`
112133
return component
113134
}

packages/template-retail-react-app/app/components/icons/index.test.js

Lines changed: 38 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,10 +5,21 @@
55
* For full license text, see the LICENSE file in the repo root or https://opensource.org/licenses/BSD-3-Clause
66
*/
77
import React from 'react'
8+
import {useIntl} from 'react-intl'
89
import {within} from '@testing-library/dom'
10+
import {render} from '@testing-library/react'
911
import {renderWithProviders} from '@salesforce/retail-react-app/app/utils/test-utils'
1012
import * as Icons from '@salesforce/retail-react-app/app/components/icons/index'
1113

14+
jest.mock('react-intl', () => ({
15+
...jest.requireActual('react-intl'),
16+
useIntl: jest.fn()
17+
}))
18+
19+
beforeEach(() => {
20+
jest.clearAllMocks()
21+
})
22+
1223
test('renders svg icons with Chakra Icon component', () => {
1324
renderWithProviders(<Icons.CheckIcon />)
1425
const svg = document.querySelector('.chakra-icon')
@@ -18,3 +29,30 @@ test('renders svg icons with Chakra Icon component', () => {
1829
expect(svg).toHaveAttribute('viewBox', '0 0 24 24')
1930
expect(use).toHaveAttribute('xlink:href', '#check')
2031
})
32+
33+
test('uses intl from context when rendered with providers', () => {
34+
renderWithProviders(<Icons.LockIcon />)
35+
expect(useIntl).toHaveBeenCalled()
36+
})
37+
38+
// the icon component can exist outside the provider tree via the error component
39+
// therefore we cannot use the useIntl hook because the <IntlProvider> component
40+
// will not exist in the component tree, so we pass the intl object as a prop
41+
test('uses intl from props when rendered outside provider tree', () => {
42+
const mockIntl = {
43+
formatMessage: jest.fn()
44+
}
45+
46+
// render without providers
47+
render(<Icons.LockIcon intl={mockIntl} />)
48+
49+
expect(mockIntl.formatMessage).toHaveBeenCalled()
50+
expect(useIntl).not.toHaveBeenCalled()
51+
})
52+
53+
test('throws error when rendered outside provider tree and no intl prop is passed', async () => {
54+
const errorMsg =
55+
'To localize messages, you must either have <IntlProvider> in the component ancestry or provide `intl` as a prop'
56+
// render without providers
57+
expect(() => render(<Icons.LockIcon />)).toThrow(errorMsg)
58+
})

packages/template-retail-react-app/app/components/promo-popover/index.jsx

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,6 @@ import {
1616
PopoverContent,
1717
PopoverHeader,
1818
PopoverTrigger,
19-
Portal,
2019
Text
2120
} from '@salesforce/retail-react-app/app/components/shared/ui'
2221
import {InfoIcon} from '@salesforce/retail-react-app/app/components/icons'

packages/template-retail-react-app/app/pages/cart/partials/cart-secondary-button-group.jsx

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,6 @@
77
import React from 'react'
88
import PropTypes from 'prop-types'
99
import {
10-
Box,
1110
Button,
1211
ButtonGroup,
1312
Checkbox,

0 commit comments

Comments
 (0)