From 4cf64716b3d6ea216ce69c53c30f96fdf4714013 Mon Sep 17 00:00:00 2001 From: Rob Snow Date: Thu, 9 Feb 2023 12:06:49 -0800 Subject: [PATCH 1/5] Fix React StrictMode Calendar --- .../calendar/src/useCalendarBase.ts | 43 +++++++++++-------- .../calendar/src/useCalendarCell.ts | 15 +++++-- .../calendar/src/useCalendarGrid.ts | 10 +++-- packages/@react-aria/calendar/src/utils.ts | 43 ++++++++++++++++++- .../@react-aria/focus/src/useFocusRing.ts | 2 +- .../@react-aria/i18n/src/useDateFormatter.ts | 22 +++++++--- .../@react-aria/interactions/src/utils.ts | 30 +++++++++---- .../@react-aria/utils/src/useUpdateEffect.ts | 6 +++ .../calendar/test/Calendar.test.js | 16 ++++--- .../calendar/test/CalendarBase.test.js | 10 ++--- .../calendar/test/RangeCalendar.test.js | 11 ++--- .../calendar/src/useCalendarState.ts | 16 ++++--- .../calendar/src/useRangeCalendarState.ts | 12 +++--- .../@react-stately/datepicker/src/utils.ts | 14 +++--- 14 files changed, 171 insertions(+), 79 deletions(-) diff --git a/packages/@react-aria/calendar/src/useCalendarBase.ts b/packages/@react-aria/calendar/src/useCalendarBase.ts index 3c7ba57b9d6..367d0ba0e42 100644 --- a/packages/@react-aria/calendar/src/useCalendarBase.ts +++ b/packages/@react-aria/calendar/src/useCalendarBase.ts @@ -16,12 +16,12 @@ import {AriaLabelingProps, DOMAttributes} from '@react-types/shared'; import {CalendarPropsBase} from '@react-types/calendar'; import {CalendarState, RangeCalendarState} from '@react-stately/calendar'; import {DOMProps} from '@react-types/shared'; -import {filterDOMProps, mergeProps, useLabels, useSlotId, useUpdateEffect} from '@react-aria/utils'; -import {hookData, useSelectedDateDescription, useVisibleRangeDescription} from './utils'; +import {filterDOMProps, mergeProps, useLabels, useLayoutEffect, useSlotId, useUpdateEffect} from '@react-aria/utils'; +import {hookStore, useSelectedDateDescription, useVisibleRangeDescription} from './utils'; // @ts-ignore import intlMessages from '../intl/*.json'; import {useLocalizedStringFormatter} from '@react-aria/i18n'; -import {useRef} from 'react'; +import {useMemo, useRef} from 'react'; export interface CalendarAria { /** Props for the calendar grouping element. */ @@ -62,28 +62,35 @@ export function useCalendarBase(props: CalendarPropsBase & DOMProps & AriaLabeli let errorMessageId = useSlotId([Boolean(props.errorMessage), props.validationState]); - // Pass the label to the child grid elements. - hookData.set(state, { - ariaLabel: props['aria-label'], - ariaLabelledBy: props['aria-labelledby'], + let ariaLabel = props['aria-label']; + let ariaLabelledBy = props['aria-labelledby']; + let data = useMemo(() => ({ + ariaLabel, + ariaLabelledBy, errorMessageId, selectedDateDescription - }); + }), [ariaLabel, ariaLabelledBy, errorMessageId, selectedDateDescription]); + useLayoutEffect(() => { + // Pass the label to the child grid elements. + hookStore.update(state, data); + }, [data]); - // If the next or previous buttons become disabled while they are focused, move focus to the calendar body. let nextFocused = useRef(false); let nextDisabled = props.isDisabled || state.isNextVisibleRangeInvalid(); - if (nextDisabled && nextFocused.current) { - nextFocused.current = false; - state.setFocused(true); - } - let previousFocused = useRef(false); let previousDisabled = props.isDisabled || state.isPreviousVisibleRangeInvalid(); - if (previousDisabled && previousFocused.current) { - previousFocused.current = false; - state.setFocused(true); - } + // If the next or previous buttons become disabled while they are focused, move focus to the calendar body. + useLayoutEffect(() => { + if (nextDisabled && nextFocused.current) { + nextFocused.current = false; + state.setFocused(true); + } + + if (previousDisabled && previousFocused.current) { + previousFocused.current = false; + state.setFocused(true); + } + }, [props.isDisabled, nextDisabled, previousDisabled, state]); let labelProps = useLabels({ id: props['id'], diff --git a/packages/@react-aria/calendar/src/useCalendarCell.ts b/packages/@react-aria/calendar/src/useCalendarCell.ts index a389751f616..560ab755f1a 100644 --- a/packages/@react-aria/calendar/src/useCalendarCell.ts +++ b/packages/@react-aria/calendar/src/useCalendarCell.ts @@ -13,14 +13,21 @@ import {CalendarDate, isEqualDay, isSameDay, isToday} from '@internationalized/date'; import {CalendarState, RangeCalendarState} from '@react-stately/calendar'; import {DOMAttributes} from '@react-types/shared'; -import {focusWithoutScrolling, getScrollParent, scrollIntoViewport, useDescription} from '@react-aria/utils'; -import {getEraFormat, hookData} from './utils'; +import { + focusWithoutScrolling, + getScrollParent, + scrollIntoViewport, + useDescription, + useLayoutEffect +} from '@react-aria/utils'; +import {getEraFormat, hookData, hookStore} from './utils'; import {getInteractionModality, usePress} from '@react-aria/interactions'; // @ts-ignore import intlMessages from '../intl/*.json'; import {mergeProps} from '@react-aria/utils'; -import {RefObject, useEffect, useMemo, useRef} from 'react'; +import {RefObject, useEffect, useMemo, useRef, useState} from 'react'; import {useDateFormatter, useLocalizedStringFormatter} from '@react-aria/i18n'; +import {useSyncExternalStore} from "use-sync-external-store/shim"; export interface AriaCalendarCellProps { /** The date that this cell represents. */ @@ -75,7 +82,7 @@ export interface CalendarCellAria { */ export function useCalendarCell(props: AriaCalendarCellProps, state: CalendarState | RangeCalendarState, ref: RefObject): CalendarCellAria { let {date, isDisabled} = props; - let {errorMessageId, selectedDateDescription} = hookData.get(state); + let {errorMessageId, selectedDateDescription} = useSyncExternalStore(hookStore.subscribe(state), hookStore.getSnapshot(state)) ?? {}; let stringFormatter = useLocalizedStringFormatter(intlMessages); let dateFormatter = useDateFormatter({ weekday: 'long', diff --git a/packages/@react-aria/calendar/src/useCalendarGrid.ts b/packages/@react-aria/calendar/src/useCalendarGrid.ts index 671924b2bbb..6f93cfd75f9 100644 --- a/packages/@react-aria/calendar/src/useCalendarGrid.ts +++ b/packages/@react-aria/calendar/src/useCalendarGrid.ts @@ -13,10 +13,11 @@ import {CalendarDate, startOfWeek, today} from '@internationalized/date'; import {CalendarState, RangeCalendarState} from '@react-stately/calendar'; import {DOMAttributes} from '@react-types/shared'; -import {hookData, useVisibleRangeDescription} from './utils'; -import {KeyboardEvent, useMemo} from 'react'; -import {mergeProps, useLabels} from '@react-aria/utils'; +import {hookData, hookStore, useVisibleRangeDescription} from './utils'; +import {KeyboardEvent, useEffect, useMemo, useState} from 'react'; +import {mergeProps, useLabels, useLayoutEffect} from '@react-aria/utils'; import {useDateFormatter, useLocale} from '@react-aria/i18n'; +import {useSyncExternalStore} from "use-sync-external-store/shim"; export interface AriaCalendarGridProps { /** @@ -122,7 +123,8 @@ export function useCalendarGrid(props: AriaCalendarGridProps, state: CalendarSta let visibleRangeDescription = useVisibleRangeDescription(startDate, endDate, state.timeZone, true); - let {ariaLabel, ariaLabelledBy} = hookData.get(state); + let {ariaLabel, ariaLabelledBy} = useSyncExternalStore(hookStore.subscribe(state), hookStore.getSnapshot(state)) ?? {}; + let labelProps = useLabels({ 'aria-label': [ariaLabel, visibleRangeDescription].filter(Boolean).join(', '), 'aria-labelledby': ariaLabelledBy diff --git a/packages/@react-aria/calendar/src/utils.ts b/packages/@react-aria/calendar/src/utils.ts index 273dfd44442..52c748bdcfb 100644 --- a/packages/@react-aria/calendar/src/utils.ts +++ b/packages/@react-aria/calendar/src/utils.ts @@ -25,8 +25,47 @@ interface HookData { selectedDateDescription: string } -export const hookData = new WeakMap(); - +let hookData = new Map(); +let listeners = new Map(); + +export const hookStore = { + update: (state: CalendarState | RangeCalendarState, data: HookData) => { + if (hookData.get(state) !== data) { + hookData.set(state, data); + hookData = new Map(hookData); + emitChange(state); + } + }, + subscribe(state) { + return (listener) => { + let prevListeners = listeners.get(state); + if (prevListeners) { + listeners.set(state, [...prevListeners, listener]); + } else { + listeners.set(state, [listener]); + } + return () => { + listeners.set(state, listeners.get(state).filter(l => l !== listener)); + if (listeners.get(state).length === 0) { + hookData.delete(state); + } + }; + }; + }, + getSnapshot(state) { + return () => { + return hookData.get(state); + }; + } +}; +function emitChange(state) { + let stateListeners = listeners.get(state); + if (stateListeners) { + for (let listener of listeners.get(state)) { + listener(); + } + } +} export function getEraFormat(date: CalendarDate): 'short' | undefined { return date?.calendar.identifier === 'gregory' && date.era === 'BC' ? 'short' : undefined; } diff --git a/packages/@react-aria/focus/src/useFocusRing.ts b/packages/@react-aria/focus/src/useFocusRing.ts index 6e813ba6294..d0ea4efc528 100644 --- a/packages/@react-aria/focus/src/useFocusRing.ts +++ b/packages/@react-aria/focus/src/useFocusRing.ts @@ -73,7 +73,7 @@ export function useFocusRing(props: AriaFocusRingProps = {}): FocusRingAria { return { isFocused, - isFocusVisible: state.current.isFocused && isFocusVisibleState, + isFocusVisible: isFocusVisibleState, focusProps: within ? focusWithinProps : focusProps }; } diff --git a/packages/@react-aria/i18n/src/useDateFormatter.ts b/packages/@react-aria/i18n/src/useDateFormatter.ts index cbf63e35457..4993e954182 100644 --- a/packages/@react-aria/i18n/src/useDateFormatter.ts +++ b/packages/@react-aria/i18n/src/useDateFormatter.ts @@ -13,6 +13,7 @@ import {DateFormatter} from '@internationalized/date'; import {useLocale} from './context'; import {useMemo, useRef} from 'react'; +import {useLayoutEffect} from "@react-aria/utils"; export interface DateFormatterOptions extends Intl.DateTimeFormatOptions { calendar?: string @@ -24,16 +25,23 @@ export interface DateFormatterOptions extends Intl.DateTimeFormatOptions { * @param options - Formatting options. */ export function useDateFormatter(options?: DateFormatterOptions): DateFormatter { + let {locale} = useLocale(); + // Reuse last options object if it is shallowly equal, which allows the useMemo result to also be reused. let lastOptions = useRef(null); - if (options && lastOptions.current && isEqual(options, lastOptions.current)) { - options = lastOptions.current; - } - - lastOptions.current = options; + let lastFormattedDate = useRef(null); + let formattedDate = useMemo(() => { + if (options && lastOptions.current && isEqual(options, lastOptions.current)) { + return lastFormattedDate.current; + } + return new DateFormatter(locale, options); + }, [locale, options]); + useLayoutEffect(() => { + lastOptions.current = options; + lastFormattedDate.current = formattedDate; + }); - let {locale} = useLocale(); - return useMemo(() => new DateFormatter(locale, options), [locale, options]); + return formattedDate; } function isEqual(a: DateFormatterOptions, b: DateFormatterOptions) { diff --git a/packages/@react-aria/interactions/src/utils.ts b/packages/@react-aria/interactions/src/utils.ts index 0ab2cf9895e..3d068f63e08 100644 --- a/packages/@react-aria/interactions/src/utils.ts +++ b/packages/@react-aria/interactions/src/utils.ts @@ -11,7 +11,7 @@ */ import {FocusEvent as ReactFocusEvent, useCallback, useRef} from 'react'; -import {useLayoutEffect} from '@react-aria/utils'; +import {useId, useLayoutEffect, useUpdateEffect} from '@react-aria/utils'; export class SyntheticFocusEvent implements ReactFocusEvent { nativeEvent: FocusEvent; @@ -64,13 +64,12 @@ export class SyntheticFocusEvent implements ReactFocusEvent { export function useSyntheticBlurEvent(onBlur: (e: ReactFocusEvent) => void) { let stateRef = useRef({ isFocused: false, - onBlur, - observer: null as MutationObserver + observer: null as MutationObserver, + target: null as Element, + listener: null as (e: FocusEvent) => void }); - stateRef.current.onBlur = onBlur; // Clean up MutationObserver on unmount. See below. - // eslint-disable-next-line arrow-body-style useLayoutEffect(() => { const state = stateRef.current; return () => { @@ -78,6 +77,12 @@ export function useSyntheticBlurEvent(onBlur: (e: ReactFocusEvent) => void) { state.observer.disconnect(); state.observer = null; } + if (state.target && state.listener) { + state.target.removeEventListener('focusout', state.listener); + } + state.target = null; + state.listener = null; + state.isFocused = false; }; }, []); @@ -101,7 +106,7 @@ export function useSyntheticBlurEvent(onBlur: (e: ReactFocusEvent) => void) { if (target.disabled) { // For backward compatibility, dispatch a (fake) React synthetic event. - stateRef.current.onBlur?.(new SyntheticFocusEvent('blur', e)); + onBlur?.(new SyntheticFocusEvent('blur', e)); } // We no longer need the MutationObserver once the target is blurred. @@ -111,10 +116,19 @@ export function useSyntheticBlurEvent(onBlur: (e: ReactFocusEvent) => void) { } }; + if (stateRef.current.target && stateRef.current.listener) { + stateRef.current.target.removeEventListener('focusout', stateRef.current.listener); + } target.addEventListener('focusout', onBlurHandler, {once: true}); + stateRef.current.target = target; + stateRef.current.listener = onBlurHandler; + if (stateRef.current.observer) { + stateRef.current.observer.disconnect(); + stateRef.current.observer = null; + } stateRef.current.observer = new MutationObserver(() => { - if (stateRef.current.isFocused && target.disabled) { + if (stateRef.current.isFocused && target.disabled && document.contains(target)) { stateRef.current.observer.disconnect(); target.dispatchEvent(new FocusEvent('blur')); target.dispatchEvent(new FocusEvent('focusout', {bubbles: true})); @@ -123,5 +137,5 @@ export function useSyntheticBlurEvent(onBlur: (e: ReactFocusEvent) => void) { stateRef.current.observer.observe(target, {attributes: true, attributeFilter: ['disabled']}); } - }, []); + }, [blur]); } diff --git a/packages/@react-aria/utils/src/useUpdateEffect.ts b/packages/@react-aria/utils/src/useUpdateEffect.ts index f480311c22d..ce5378e07f3 100644 --- a/packages/@react-aria/utils/src/useUpdateEffect.ts +++ b/packages/@react-aria/utils/src/useUpdateEffect.ts @@ -24,4 +24,10 @@ export function useUpdateEffect(effect: EffectCallback, dependencies: any[]) { } // eslint-disable-next-line react-hooks/exhaustive-deps }, dependencies); + + // For strictmode, the above isn't sufficient for detecting 'initial mount', so we + // need to cleanup on unmount. + useEffect(() => () => { + isInitialMount.current = true; + }, []); } diff --git a/packages/@react-spectrum/calendar/test/Calendar.test.js b/packages/@react-spectrum/calendar/test/Calendar.test.js index 813c9991316..2b9dccd1e2b 100644 --- a/packages/@react-spectrum/calendar/test/Calendar.test.js +++ b/packages/@react-spectrum/calendar/test/Calendar.test.js @@ -10,6 +10,8 @@ * governing permissions and limitations under the License. */ +import {act} from '@testing-library/react'; + jest.mock('@react-aria/live-announcer'); import {announce} from '@react-aria/live-announcer'; import {Calendar} from '../'; @@ -21,12 +23,11 @@ import {useLocale} from '@react-aria/i18n'; let keyCodes = {'Enter': 13, ' ': 32, 'PageUp': 33, 'PageDown': 34, 'End': 35, 'Home': 36, 'ArrowLeft': 37, 'ArrowUp': 38, 'ArrowRight': 39, 'ArrowDown': 40}; describe('Calendar', () => { - beforeEach(() => { - jest.spyOn(window, 'requestAnimationFrame').mockImplementation(cb => cb()); + beforeAll(() => { + jest.useFakeTimers(); }); - afterEach(() => { - window.requestAnimationFrame.mockRestore(); + act(() => {jest.runAllTimers();}); }); describe('basics', () => { @@ -388,13 +389,13 @@ describe('Calendar', () => { }); }); - // These tests only work against v3 describe('announcing', () => { it('announces when the current month changes', () => { let {getAllByLabelText} = render(); let nextButton = getAllByLabelText('Next')[0]; triggerPress(nextButton); + act(() => {jest.runAllTimers();}); expect(announce).toHaveBeenCalledTimes(1); expect(announce).toHaveBeenCalledWith('July 2019'); @@ -405,6 +406,7 @@ describe('Calendar', () => { let newDate = getByText('17'); triggerPress(newDate); + act(() => {jest.runAllTimers();}); expect(announce).toHaveBeenCalledTimes(1); expect(announce).toHaveBeenCalledWith('Selected Date: Monday, June 17, 2019', 'polite', 4000); @@ -418,6 +420,8 @@ describe('Calendar', () => { expect(selectedDate).toHaveFocus(); fireEvent.keyDown(grid, {key: 'ArrowRight'}); + fireEvent.keyUp(document.activeElement, {key: 'ArrowRight'}); + act(() => {jest.runAllTimers();}); expect(getByLabelText('Thursday, June 6, 2019', {exact: false})).toHaveFocus(); }); @@ -426,6 +430,7 @@ describe('Calendar', () => { let newDate = getByText('17'); triggerPress(newDate); + act(() => {jest.runAllTimers();}); expect(announce).toHaveBeenCalledTimes(1); expect(announce).toHaveBeenCalledWith('Selected Date: Saturday, February 17, 5 BC', 'polite', 4000); @@ -433,6 +438,7 @@ describe('Calendar', () => { announce.mockReset(); let nextButton = getAllByLabelText('Next')[0]; triggerPress(nextButton); + act(() => {jest.runAllTimers();}); expect(announce).toHaveBeenCalledTimes(1); expect(announce).toHaveBeenCalledWith('March 5 BC'); diff --git a/packages/@react-spectrum/calendar/test/CalendarBase.test.js b/packages/@react-spectrum/calendar/test/CalendarBase.test.js index 98384cb945b..95f61a635f5 100644 --- a/packages/@react-spectrum/calendar/test/CalendarBase.test.js +++ b/packages/@react-spectrum/calendar/test/CalendarBase.test.js @@ -10,7 +10,7 @@ * governing permissions and limitations under the License. */ -import {act, fireEvent, render, triggerPress, within} from '@react-spectrum/test-utils'; +import {act, fireEvent, render, triggerPress, waitFor, within} from '@react-spectrum/test-utils'; import {Calendar, RangeCalendar} from '../'; import {CalendarDate, GregorianCalendar, today} from '@internationalized/date'; import {Provider} from '@react-spectrum/provider'; @@ -24,8 +24,7 @@ let keyCodes = {'Enter': 13, ' ': 32, 'PageUp': 33, 'PageDown': 34, 'End': 35, ' describe('CalendarBase', () => { beforeAll(() => { - jest.useFakeTimers('legacy'); - jest.spyOn(window, 'requestAnimationFrame').mockImplementation(cb => cb()); + jest.useFakeTimers(); }); afterEach(() => { @@ -234,8 +233,7 @@ describe('CalendarBase', () => { it.each` Name | Calendar | props ${'v3 Calendar'} | ${Calendar} | ${{defaultValue: new CalendarDate(2019, 3, 10), minValue: new CalendarDate(2019, 2, 3), maxValue: new CalendarDate(2019, 4, 20)}} - ${'v3 RangeCalendar'} | ${RangeCalendar} | ${{defaultValue: {start: new CalendarDate(2019, 3, 10), end: new CalendarDate(2019, 3, 15)}, minValue: new CalendarDate(2019, 2, 3), maxValue: new CalendarDate(2019, 4, 20)}} - `('$Name should move focus when the previous or next buttons become disabled', ({Calendar, props}) => { + `('$Name should move focus when the previous or next buttons become disabled', async ({Calendar, props}) => { let {getByLabelText, getAllByLabelText} = render(); let prevButton = getByLabelText('Previous'); @@ -244,7 +242,7 @@ describe('CalendarBase', () => { expect(prevButton).not.toHaveAttribute('disabled'); expect(nextButton).not.toHaveAttribute('disabled'); - act(() => userEvent.click(prevButton)); + triggerPress(prevButton); expect(prevButton).toHaveAttribute('disabled'); expect(nextButton).not.toHaveAttribute('disabled'); expect(document.activeElement.getAttribute('aria-label').startsWith('Sunday, February 10, 2019')).toBe(true); diff --git a/packages/@react-spectrum/calendar/test/RangeCalendar.test.js b/packages/@react-spectrum/calendar/test/RangeCalendar.test.js index daef90a0871..87e1c7b524e 100644 --- a/packages/@react-spectrum/calendar/test/RangeCalendar.test.js +++ b/packages/@react-spectrum/calendar/test/RangeCalendar.test.js @@ -28,13 +28,11 @@ function type(key) { } describe('RangeCalendar', () => { - beforeEach(() => { - jest.useFakeTimers('legacy'); - jest.spyOn(window, 'requestAnimationFrame').mockImplementation(cb => cb()); + beforeAll(() => { + jest.useFakeTimers(); }); - afterEach(() => { - window.requestAnimationFrame.mockRestore(); + act(() => {jest.runAllTimers();}); }); describe('basics', () => { @@ -115,8 +113,7 @@ describe('RangeCalendar', () => { expect(grid).not.toHaveAttribute('aria-activedescendant'); }); - // v2 doesn't pass this test - it starts by showing the end date instead of the start date. - it('should show selected dates across multiple months', () => { + it.only('should show selected dates across multiple months', () => { let {getByRole, getByLabelText, getAllByLabelText, getAllByRole} = render(); let heading = getByRole('heading'); diff --git a/packages/@react-stately/calendar/src/useCalendarState.ts b/packages/@react-stately/calendar/src/useCalendarState.ts index e9024962f02..02df66d5508 100644 --- a/packages/@react-stately/calendar/src/useCalendarState.ts +++ b/packages/@react-stately/calendar/src/useCalendarState.ts @@ -30,7 +30,7 @@ import { import {CalendarProps, DateValue} from '@react-types/calendar'; import {CalendarState} from './types'; import {useControlledState} from '@react-stately/utils'; -import {useMemo, useRef, useState} from 'react'; +import {useEffect, useMemo, useRef, useState} from 'react'; export interface CalendarStateOptions extends CalendarProps { /** The locale to display and edit the value according to. */ @@ -113,12 +113,14 @@ export function useCalendarState(props: CalendarStateOptions): CalendarState { // Reset focused date and visible range when calendar changes. let lastCalendarIdentifier = useRef(calendar.identifier); - if (calendar.identifier !== lastCalendarIdentifier.current) { - let newFocusedDate = toCalendar(focusedDate, calendar); - setStartDate(alignCenter(newFocusedDate, visibleDuration, locale, minValue, maxValue)); - setFocusedDate(newFocusedDate); - lastCalendarIdentifier.current = calendar.identifier; - } + useEffect(() => { + if (calendar.identifier !== lastCalendarIdentifier.current) { + let newFocusedDate = toCalendar(focusedDate, calendar); + setStartDate(alignCenter(newFocusedDate, visibleDuration, locale, minValue, maxValue)); + setFocusedDate(newFocusedDate); + lastCalendarIdentifier.current = calendar.identifier; + } + }); if (isInvalid(focusedDate, minValue, maxValue)) { // If the focused date was moved to an invalid value, it can't be focused, so constrain it. diff --git a/packages/@react-stately/calendar/src/useRangeCalendarState.ts b/packages/@react-stately/calendar/src/useRangeCalendarState.ts index cb21224246e..3902d05c202 100644 --- a/packages/@react-stately/calendar/src/useRangeCalendarState.ts +++ b/packages/@react-stately/calendar/src/useRangeCalendarState.ts @@ -18,7 +18,7 @@ import {RangeCalendarProps} from '@react-types/calendar'; import {RangeValue} from '@react-types/shared'; import {useCalendarState} from './useCalendarState'; import {useControlledState} from '@react-stately/utils'; -import {useMemo, useRef, useState} from 'react'; +import {useEffect, useMemo, useRef, useState} from 'react'; export interface RangeCalendarStateOptions extends RangeCalendarProps { /** The locale to display and edit the value according to. */ @@ -92,10 +92,12 @@ export function useRangeCalendarState(props: RangeCalendarStateOptions): RangeCa // If the visible range changes, we need to update the available range. let lastVisibleRange = useRef(calendar.visibleRange); - if (!isEqualDay(calendar.visibleRange.start, lastVisibleRange.current.start) || !isEqualDay(calendar.visibleRange.end, lastVisibleRange.current.end)) { - updateAvailableRange(anchorDate); - lastVisibleRange.current = calendar.visibleRange; - } + useEffect(() => { + if (!isEqualDay(calendar.visibleRange.start, lastVisibleRange.current.start) || !isEqualDay(calendar.visibleRange.end, lastVisibleRange.current.end)) { + updateAvailableRange(anchorDate); + lastVisibleRange.current = calendar.visibleRange; + } + }); let setAnchorDate = (date: CalendarDate) => { if (date) { diff --git a/packages/@react-stately/datepicker/src/utils.ts b/packages/@react-stately/datepicker/src/utils.ts index b815dcd1830..850f3089dc9 100644 --- a/packages/@react-stately/datepicker/src/utils.ts +++ b/packages/@react-stately/datepicker/src/utils.ts @@ -12,7 +12,7 @@ import {Calendar, now, Time, toCalendar, toCalendarDate, toCalendarDateTime} from '@internationalized/date'; import {DatePickerProps, DateValue, Granularity, TimeValue} from '@react-types/datepicker'; -import {useRef} from 'react'; +import {useEffect, useRef} from 'react'; export function isInvalid(value: DateValue, minValue: DateValue, maxValue: DateValue) { return value != null && ( @@ -131,11 +131,15 @@ export function createPlaceholderDate(placeholderValue: DateValue, granularity: export function useDefaultProps(v: DateValue, granularity: Granularity): [Granularity, string] { // Compute default granularity and time zone from the value. If the value becomes null, keep the last values. let lastValue = useRef(v); - if (v) { - lastValue.current = v; + useEffect(() => { + if (v) { + lastValue.current = v; + } + }, [v]); + + if (!v) { + v = lastValue.current; } - - v = lastValue.current; let defaultTimeZone = (v && 'timeZone' in v ? v.timeZone : undefined); granularity = granularity || (v && 'minute' in v ? 'minute' : 'day'); From 6e881494cf9c3de98dea61959f2172aad9245d15 Mon Sep 17 00:00:00 2001 From: Rob Snow Date: Thu, 9 Feb 2023 12:57:17 -0800 Subject: [PATCH 2/5] Fix issues and lint, revert sync external --- .../calendar/src/useCalendarBase.ts | 19 +++----- .../calendar/src/useCalendarCell.ts | 10 ++--- .../calendar/src/useCalendarGrid.ts | 9 ++-- packages/@react-aria/calendar/src/utils.ts | 43 +------------------ .../@react-aria/i18n/src/useDateFormatter.ts | 2 +- .../@react-aria/interactions/src/utils.ts | 4 +- .../calendar/test/CalendarBase.test.js | 2 +- .../calendar/src/useCalendarState.ts | 5 ++- 8 files changed, 24 insertions(+), 70 deletions(-) diff --git a/packages/@react-aria/calendar/src/useCalendarBase.ts b/packages/@react-aria/calendar/src/useCalendarBase.ts index 367d0ba0e42..8fba8edceb7 100644 --- a/packages/@react-aria/calendar/src/useCalendarBase.ts +++ b/packages/@react-aria/calendar/src/useCalendarBase.ts @@ -17,11 +17,11 @@ import {CalendarPropsBase} from '@react-types/calendar'; import {CalendarState, RangeCalendarState} from '@react-stately/calendar'; import {DOMProps} from '@react-types/shared'; import {filterDOMProps, mergeProps, useLabels, useLayoutEffect, useSlotId, useUpdateEffect} from '@react-aria/utils'; -import {hookStore, useSelectedDateDescription, useVisibleRangeDescription} from './utils'; +import {hookData, useSelectedDateDescription, useVisibleRangeDescription} from './utils'; // @ts-ignore import intlMessages from '../intl/*.json'; import {useLocalizedStringFormatter} from '@react-aria/i18n'; -import {useMemo, useRef} from 'react'; +import {useRef} from 'react'; export interface CalendarAria { /** Props for the calendar grouping element. */ @@ -62,18 +62,13 @@ export function useCalendarBase(props: CalendarPropsBase & DOMProps & AriaLabeli let errorMessageId = useSlotId([Boolean(props.errorMessage), props.validationState]); - let ariaLabel = props['aria-label']; - let ariaLabelledBy = props['aria-labelledby']; - let data = useMemo(() => ({ - ariaLabel, - ariaLabelledBy, + // Pass the label to the child grid elements. + hookData.set(state, { + ariaLabel: props['aria-label'], + ariaLabelledBy: props['aria-labelledby'], errorMessageId, selectedDateDescription - }), [ariaLabel, ariaLabelledBy, errorMessageId, selectedDateDescription]); - useLayoutEffect(() => { - // Pass the label to the child grid elements. - hookStore.update(state, data); - }, [data]); + }); let nextFocused = useRef(false); let nextDisabled = props.isDisabled || state.isNextVisibleRangeInvalid(); diff --git a/packages/@react-aria/calendar/src/useCalendarCell.ts b/packages/@react-aria/calendar/src/useCalendarCell.ts index 560ab755f1a..9aa46e7ab20 100644 --- a/packages/@react-aria/calendar/src/useCalendarCell.ts +++ b/packages/@react-aria/calendar/src/useCalendarCell.ts @@ -17,17 +17,15 @@ import { focusWithoutScrolling, getScrollParent, scrollIntoViewport, - useDescription, - useLayoutEffect + useDescription } from '@react-aria/utils'; -import {getEraFormat, hookData, hookStore} from './utils'; +import {getEraFormat, hookData} from './utils'; import {getInteractionModality, usePress} from '@react-aria/interactions'; // @ts-ignore import intlMessages from '../intl/*.json'; import {mergeProps} from '@react-aria/utils'; -import {RefObject, useEffect, useMemo, useRef, useState} from 'react'; +import {RefObject, useEffect, useMemo, useRef} from 'react'; import {useDateFormatter, useLocalizedStringFormatter} from '@react-aria/i18n'; -import {useSyncExternalStore} from "use-sync-external-store/shim"; export interface AriaCalendarCellProps { /** The date that this cell represents. */ @@ -82,7 +80,7 @@ export interface CalendarCellAria { */ export function useCalendarCell(props: AriaCalendarCellProps, state: CalendarState | RangeCalendarState, ref: RefObject): CalendarCellAria { let {date, isDisabled} = props; - let {errorMessageId, selectedDateDescription} = useSyncExternalStore(hookStore.subscribe(state), hookStore.getSnapshot(state)) ?? {}; + let {errorMessageId, selectedDateDescription} = hookData.get(state); let stringFormatter = useLocalizedStringFormatter(intlMessages); let dateFormatter = useDateFormatter({ weekday: 'long', diff --git a/packages/@react-aria/calendar/src/useCalendarGrid.ts b/packages/@react-aria/calendar/src/useCalendarGrid.ts index 6f93cfd75f9..14a5189ced7 100644 --- a/packages/@react-aria/calendar/src/useCalendarGrid.ts +++ b/packages/@react-aria/calendar/src/useCalendarGrid.ts @@ -13,11 +13,10 @@ import {CalendarDate, startOfWeek, today} from '@internationalized/date'; import {CalendarState, RangeCalendarState} from '@react-stately/calendar'; import {DOMAttributes} from '@react-types/shared'; -import {hookData, hookStore, useVisibleRangeDescription} from './utils'; -import {KeyboardEvent, useEffect, useMemo, useState} from 'react'; -import {mergeProps, useLabels, useLayoutEffect} from '@react-aria/utils'; +import {hookData, useVisibleRangeDescription} from './utils'; +import {KeyboardEvent, useMemo} from 'react'; +import {mergeProps, useLabels} from '@react-aria/utils'; import {useDateFormatter, useLocale} from '@react-aria/i18n'; -import {useSyncExternalStore} from "use-sync-external-store/shim"; export interface AriaCalendarGridProps { /** @@ -123,7 +122,7 @@ export function useCalendarGrid(props: AriaCalendarGridProps, state: CalendarSta let visibleRangeDescription = useVisibleRangeDescription(startDate, endDate, state.timeZone, true); - let {ariaLabel, ariaLabelledBy} = useSyncExternalStore(hookStore.subscribe(state), hookStore.getSnapshot(state)) ?? {}; + let {ariaLabel, ariaLabelledBy} = hookData.get(state); let labelProps = useLabels({ 'aria-label': [ariaLabel, visibleRangeDescription].filter(Boolean).join(', '), diff --git a/packages/@react-aria/calendar/src/utils.ts b/packages/@react-aria/calendar/src/utils.ts index 52c748bdcfb..273dfd44442 100644 --- a/packages/@react-aria/calendar/src/utils.ts +++ b/packages/@react-aria/calendar/src/utils.ts @@ -25,47 +25,8 @@ interface HookData { selectedDateDescription: string } -let hookData = new Map(); -let listeners = new Map(); - -export const hookStore = { - update: (state: CalendarState | RangeCalendarState, data: HookData) => { - if (hookData.get(state) !== data) { - hookData.set(state, data); - hookData = new Map(hookData); - emitChange(state); - } - }, - subscribe(state) { - return (listener) => { - let prevListeners = listeners.get(state); - if (prevListeners) { - listeners.set(state, [...prevListeners, listener]); - } else { - listeners.set(state, [listener]); - } - return () => { - listeners.set(state, listeners.get(state).filter(l => l !== listener)); - if (listeners.get(state).length === 0) { - hookData.delete(state); - } - }; - }; - }, - getSnapshot(state) { - return () => { - return hookData.get(state); - }; - } -}; -function emitChange(state) { - let stateListeners = listeners.get(state); - if (stateListeners) { - for (let listener of listeners.get(state)) { - listener(); - } - } -} +export const hookData = new WeakMap(); + export function getEraFormat(date: CalendarDate): 'short' | undefined { return date?.calendar.identifier === 'gregory' && date.era === 'BC' ? 'short' : undefined; } diff --git a/packages/@react-aria/i18n/src/useDateFormatter.ts b/packages/@react-aria/i18n/src/useDateFormatter.ts index 4993e954182..789f31b4608 100644 --- a/packages/@react-aria/i18n/src/useDateFormatter.ts +++ b/packages/@react-aria/i18n/src/useDateFormatter.ts @@ -11,9 +11,9 @@ */ import {DateFormatter} from '@internationalized/date'; +import {useLayoutEffect} from '@react-aria/utils'; import {useLocale} from './context'; import {useMemo, useRef} from 'react'; -import {useLayoutEffect} from "@react-aria/utils"; export interface DateFormatterOptions extends Intl.DateTimeFormatOptions { calendar?: string diff --git a/packages/@react-aria/interactions/src/utils.ts b/packages/@react-aria/interactions/src/utils.ts index 3d068f63e08..ca63b6afd03 100644 --- a/packages/@react-aria/interactions/src/utils.ts +++ b/packages/@react-aria/interactions/src/utils.ts @@ -11,7 +11,7 @@ */ import {FocusEvent as ReactFocusEvent, useCallback, useRef} from 'react'; -import {useId, useLayoutEffect, useUpdateEffect} from '@react-aria/utils'; +import {useLayoutEffect} from '@react-aria/utils'; export class SyntheticFocusEvent implements ReactFocusEvent { nativeEvent: FocusEvent; @@ -137,5 +137,5 @@ export function useSyntheticBlurEvent(onBlur: (e: ReactFocusEvent) => void) { stateRef.current.observer.observe(target, {attributes: true, attributeFilter: ['disabled']}); } - }, [blur]); + }, [onBlur]); } diff --git a/packages/@react-spectrum/calendar/test/CalendarBase.test.js b/packages/@react-spectrum/calendar/test/CalendarBase.test.js index 95f61a635f5..d519fd941d9 100644 --- a/packages/@react-spectrum/calendar/test/CalendarBase.test.js +++ b/packages/@react-spectrum/calendar/test/CalendarBase.test.js @@ -10,7 +10,7 @@ * governing permissions and limitations under the License. */ -import {act, fireEvent, render, triggerPress, waitFor, within} from '@react-spectrum/test-utils'; +import {act, fireEvent, render, triggerPress, within} from '@react-spectrum/test-utils'; import {Calendar, RangeCalendar} from '../'; import {CalendarDate, GregorianCalendar, today} from '@internationalized/date'; import {Provider} from '@react-spectrum/provider'; diff --git a/packages/@react-stately/calendar/src/useCalendarState.ts b/packages/@react-stately/calendar/src/useCalendarState.ts index 02df66d5508..7ce67587ab0 100644 --- a/packages/@react-stately/calendar/src/useCalendarState.ts +++ b/packages/@react-stately/calendar/src/useCalendarState.ts @@ -113,14 +113,15 @@ export function useCalendarState(props: CalendarStateOptions): CalendarState { // Reset focused date and visible range when calendar changes. let lastCalendarIdentifier = useRef(calendar.identifier); + let calIdentifier = calendar.identifier; useEffect(() => { if (calendar.identifier !== lastCalendarIdentifier.current) { let newFocusedDate = toCalendar(focusedDate, calendar); setStartDate(alignCenter(newFocusedDate, visibleDuration, locale, minValue, maxValue)); setFocusedDate(newFocusedDate); - lastCalendarIdentifier.current = calendar.identifier; + lastCalendarIdentifier.current = calIdentifier; } - }); + }, [calendar, focusedDate, visibleDuration, locale, minValue, maxValue, setFocusedDate, calIdentifier]); if (isInvalid(focusedDate, minValue, maxValue)) { // If the focused date was moved to an invalid value, it can't be focused, so constrain it. From a3f59427d0c458a649ee3e7cd535782891a9af63 Mon Sep 17 00:00:00 2001 From: Robert Snow Date: Wed, 15 Feb 2023 14:08:15 -0800 Subject: [PATCH 3/5] Update packages/@react-spectrum/calendar/test/RangeCalendar.test.js --- packages/@react-spectrum/calendar/test/RangeCalendar.test.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/@react-spectrum/calendar/test/RangeCalendar.test.js b/packages/@react-spectrum/calendar/test/RangeCalendar.test.js index 87e1c7b524e..0a540d4357e 100644 --- a/packages/@react-spectrum/calendar/test/RangeCalendar.test.js +++ b/packages/@react-spectrum/calendar/test/RangeCalendar.test.js @@ -113,7 +113,7 @@ describe('RangeCalendar', () => { expect(grid).not.toHaveAttribute('aria-activedescendant'); }); - it.only('should show selected dates across multiple months', () => { + it('should show selected dates across multiple months', () => { let {getByRole, getByLabelText, getAllByLabelText, getAllByRole} = render(); let heading = getByRole('heading'); From 9858a27060a5dc8b43d5262dc1ad5b1f97b199b2 Mon Sep 17 00:00:00 2001 From: Rob Snow Date: Thu, 16 Feb 2023 11:59:47 -0800 Subject: [PATCH 4/5] address review comments --- .../calendar/src/useCalendarBase.ts | 2 +- .../@react-aria/i18n/src/useDateFormatter.ts | 21 +++++++------------ 2 files changed, 9 insertions(+), 14 deletions(-) diff --git a/packages/@react-aria/calendar/src/useCalendarBase.ts b/packages/@react-aria/calendar/src/useCalendarBase.ts index 8fba8edceb7..7f52f43ee63 100644 --- a/packages/@react-aria/calendar/src/useCalendarBase.ts +++ b/packages/@react-aria/calendar/src/useCalendarBase.ts @@ -85,7 +85,7 @@ export function useCalendarBase(props: CalendarPropsBase & DOMProps & AriaLabeli previousFocused.current = false; state.setFocused(true); } - }, [props.isDisabled, nextDisabled, previousDisabled, state]); + }, [nextDisabled, previousDisabled, state]); let labelProps = useLabels({ id: props['id'], diff --git a/packages/@react-aria/i18n/src/useDateFormatter.ts b/packages/@react-aria/i18n/src/useDateFormatter.ts index 789f31b4608..b637a8cf75d 100644 --- a/packages/@react-aria/i18n/src/useDateFormatter.ts +++ b/packages/@react-aria/i18n/src/useDateFormatter.ts @@ -13,7 +13,7 @@ import {DateFormatter} from '@internationalized/date'; import {useLayoutEffect} from '@react-aria/utils'; import {useLocale} from './context'; -import {useMemo, useRef} from 'react'; +import {useMemo, useRef, useState} from 'react'; export interface DateFormatterOptions extends Intl.DateTimeFormatOptions { calendar?: string @@ -28,18 +28,13 @@ export function useDateFormatter(options?: DateFormatterOptions): DateFormatter let {locale} = useLocale(); // Reuse last options object if it is shallowly equal, which allows the useMemo result to also be reused. - let lastOptions = useRef(null); - let lastFormattedDate = useRef(null); - let formattedDate = useMemo(() => { - if (options && lastOptions.current && isEqual(options, lastOptions.current)) { - return lastFormattedDate.current; - } - return new DateFormatter(locale, options); - }, [locale, options]); - useLayoutEffect(() => { - lastOptions.current = options; - lastFormattedDate.current = formattedDate; - }); + let [stabilizedOptions, setStabilizedOptions] = useState(options); + if (!stabilizedOptions || (options && !isEqual(stabilizedOptions, options))) { + // early render bail because setState in render + setStabilizedOptions(options); + } + + let formattedDate = useMemo(() => new DateFormatter(locale, stabilizedOptions), [locale, stabilizedOptions]); return formattedDate; } From ffaa4898533d7602638ed297c409c79a4e59c3b3 Mon Sep 17 00:00:00 2001 From: Rob Snow Date: Thu, 16 Feb 2023 14:30:15 -0800 Subject: [PATCH 5/5] fix lint --- packages/@react-aria/i18n/src/useDateFormatter.ts | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/packages/@react-aria/i18n/src/useDateFormatter.ts b/packages/@react-aria/i18n/src/useDateFormatter.ts index b637a8cf75d..4b2f1c956e5 100644 --- a/packages/@react-aria/i18n/src/useDateFormatter.ts +++ b/packages/@react-aria/i18n/src/useDateFormatter.ts @@ -11,9 +11,8 @@ */ import {DateFormatter} from '@internationalized/date'; -import {useLayoutEffect} from '@react-aria/utils'; import {useLocale} from './context'; -import {useMemo, useRef, useState} from 'react'; +import {useMemo, useState} from 'react'; export interface DateFormatterOptions extends Intl.DateTimeFormatOptions { calendar?: string