From 15fc95548e0bc06c0d93dc37de094eeab3ad31ed Mon Sep 17 00:00:00 2001 From: Derek Brans Date: Wed, 17 Apr 2024 12:47:43 -0400 Subject: [PATCH] fix: Intl.NumberFormat locale RangeError related to Simulation Details (#24068) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## **Description** **Problem:** The locale returned by `getCurrentLocale` is not meant to be used with the [Intl API](https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Intl). **To fix this, this PR will:** * Rename `getCurrentLocale` => `getLocaleNotSafeForIntl` to alert developers in the future. * Add `getIntlLocale` which converts the locale returned from `getLocaleNotSafeForIntl` into a BCP 47 Language Tag which can be used with the Intl API. * Add tests to ensure that `getIntlLocale` works with all our locales going forward. [![Open in GitHub Codespaces](https://github.com/codespaces/badge.svg)](https://codespaces.new/MetaMask/metamask-extension/pull/24068?quickstart=1) ## **Related issues** Fixes: https://github.com/MetaMask/metamask-extension/issues/24067 ## **Manual testing steps** 1. Go to this page... 2. 3. ## **Screenshots/Recordings** ### **Before** ### **After** ## **Pre-merge author checklist** - [ ] I’ve followed [MetaMask Coding Standards](https://github.com/MetaMask/metamask-extension/blob/develop/.github/guidelines/CODING_GUIDELINES.md). - [ ] I've completed the PR template to the best of my ability - [ ] I’ve included tests if applicable - [ ] I’ve documented my code using [JSDoc](https://jsdoc.app/) format if applicable - [ ] I’ve applied the right labels on the PR (see [labeling guidelines](https://github.com/MetaMask/metamask-extension/blob/develop/.github/guidelines/LABELING_GUIDELINES.md)). Not required for external contributors. ## **Pre-merge reviewer checklist** - [ ] I've manually tested the PR (e.g. pull and build branch, run the app, test code being changed). - [ ] I confirm that this PR addresses all acceptance criteria described in the ticket it closes and includes the necessary testing evidence such as recordings and or screenshots. --- ui/ducks/locale/locale.js | 22 ++++++++++++++++++++++ ui/ducks/locale/locale.test.ts | 26 ++++++++++++++++++++++++++ ui/hooks/useFiatFormatter.test.ts | 19 +++++++------------ ui/hooks/useFiatFormatter.ts | 4 ++-- 4 files changed, 57 insertions(+), 14 deletions(-) create mode 100644 ui/ducks/locale/locale.test.ts diff --git a/ui/ducks/locale/locale.js b/ui/ducks/locale/locale.js index 50e944fa42c2..4af29e87e111 100644 --- a/ui/ducks/locale/locale.js +++ b/ui/ducks/locale/locale.js @@ -1,3 +1,4 @@ +import { createSelector } from 'reselect'; import * as actionConstants from '../../store/actionConstants'; export default function reduceLocaleMessages(state = {}, { type, payload }) { @@ -13,8 +14,29 @@ export default function reduceLocaleMessages(state = {}, { type, payload }) { } } +/** + * This selector returns a code from file://./../../../app/_locales/index.json. + * + * NOT SAFE FOR INTL API USE. Use getIntlLocale instead for that. + * + * @param state + * @returns {string} the user's selected locale. + * These codes are not safe to use with the Intl API. + */ export const getCurrentLocale = (state) => state.localeMessages.currentLocale; +/** + * This selector returns a + * [BCP 47 Language Tag](https://en.wikipedia.org/wiki/IETF_language_tag) + * for use with the Intl API. + * + * @returns {Intl.UnicodeBCP47LocaleIdentifier} the user's selected locale. + */ +export const getIntlLocale = createSelector( + getCurrentLocale, + (locale) => Intl.getCanonicalLocales(locale.replace(/_/gu, '-'))[0], +); + export const getCurrentLocaleMessages = (state) => state.localeMessages.current; export const getEnLocaleMessages = (state) => state.localeMessages.en; diff --git a/ui/ducks/locale/locale.test.ts b/ui/ducks/locale/locale.test.ts new file mode 100644 index 000000000000..7bb4d2b8e344 --- /dev/null +++ b/ui/ducks/locale/locale.test.ts @@ -0,0 +1,26 @@ +import locales from '../../../app/_locales/index.json'; +import { getIntlLocale } from './locale'; + +const createMockStateWithLocale = (locale: string) => ({ + localeMessages: { currentLocale: locale }, +}); + +describe('getIntlLocale', () => { + it('returns the canonical BCP 47 language tag for the currently selected locale', () => { + const mockState = createMockStateWithLocale('ab-cd'); + + expect(getIntlLocale(mockState)).toBe('ab-CD'); + }); + + it('throws an error if locale cannot be made into BCP 47 language tag', () => { + const mockState = createMockStateWithLocale('xxxinvalid-locale'); + + expect(() => getIntlLocale(mockState)).toThrow(); + }); + + it.each(locales)('handles all supported locales – "%s"', (locale) => { + const mockState = createMockStateWithLocale(locale.code); + + expect(() => getIntlLocale(mockState)).not.toThrow(); + }); +}); diff --git a/ui/hooks/useFiatFormatter.test.ts b/ui/hooks/useFiatFormatter.test.ts index 870e2c44b325..d9776c840c54 100644 --- a/ui/hooks/useFiatFormatter.test.ts +++ b/ui/hooks/useFiatFormatter.test.ts @@ -1,34 +1,30 @@ import { renderHook } from '@testing-library/react-hooks'; -import { getCurrentLocale } from '../ducks/locale/locale'; +import { getIntlLocale } from '../ducks/locale/locale'; import { getCurrentCurrency } from '../selectors'; import { useFiatFormatter } from './useFiatFormatter'; -// Mock the getCurrentLocale and getCurrentCurrency functions jest.mock('react-redux', () => ({ useSelector: jest.fn((selector) => selector()), })); jest.mock('../ducks/locale/locale', () => ({ - getCurrentLocale: jest.fn(), + getIntlLocale: jest.fn(), })); jest.mock('../selectors', () => ({ getCurrentCurrency: jest.fn(), })); -const mockGetCurrentLocale = getCurrentLocale as jest.Mock; +const mockGetIntlLocale = getIntlLocale as unknown as jest.Mock; const mockGetCurrentCurrency = getCurrentCurrency as jest.Mock; describe('useFiatFormatter', () => { beforeEach(() => { - // Clear the mock implementations before each test - mockGetCurrentLocale.mockClear(); - mockGetCurrentCurrency.mockClear(); + jest.clearAllMocks(); }); it('should return a function that formats fiat amount correctly', () => { - // Mock the getCurrentLocale and getCurrentCurrency functions - mockGetCurrentLocale.mockReturnValue('en-US'); + mockGetIntlLocale.mockReturnValue('en-US'); mockGetCurrentCurrency.mockReturnValue('USD'); const { result } = renderHook(() => useFiatFormatter()); @@ -40,13 +36,12 @@ describe('useFiatFormatter', () => { }); it('should use the current locale and currency from the mocked functions', () => { - // Mock the getCurrentLocale and getCurrentCurrency functions - mockGetCurrentLocale.mockReturnValue('fr-FR'); + mockGetIntlLocale.mockReturnValue('fr-FR'); mockGetCurrentCurrency.mockReturnValue('EUR'); renderHook(() => useFiatFormatter()); - expect(getCurrentLocale).toHaveBeenCalledTimes(1); + expect(getIntlLocale).toHaveBeenCalledTimes(1); expect(getCurrentCurrency).toHaveBeenCalledTimes(1); }); }); diff --git a/ui/hooks/useFiatFormatter.ts b/ui/hooks/useFiatFormatter.ts index 83225f63d49a..4d8f1aeaf0c8 100644 --- a/ui/hooks/useFiatFormatter.ts +++ b/ui/hooks/useFiatFormatter.ts @@ -1,5 +1,5 @@ import { useSelector } from 'react-redux'; -import { getCurrentLocale } from '../ducks/locale/locale'; +import { getIntlLocale } from '../ducks/locale/locale'; import { getCurrentCurrency } from '../selectors'; /** @@ -18,7 +18,7 @@ import { getCurrentCurrency } from '../selectors'; type FiatFormatter = (fiatAmount: number) => string; export const useFiatFormatter = (): FiatFormatter => { - const locale = useSelector(getCurrentLocale); + const locale = useSelector(getIntlLocale); const fiatCurrency = useSelector(getCurrentCurrency); return (fiatAmount: number) => {