Skip to content

Commit

Permalink
Fix infinite loop during unit testing
Browse files Browse the repository at this point in the history
In unit testing measuring dimensions is impossible, so toast height
evaluates to `0`, which causes Toaster to endlessly recompute the height
and update the store. Instead we're being more specific and checking
whether height is `undefined`, and memoizing the ref so that it isn't
being called on every render. This also required other tweaks like
properly memoizing handlers like `updateHeight` so that they can be used as hook
dependencies. (We only needed `updateHeight` to be memoized, though.)
  • Loading branch information
silvenon committed Feb 14, 2022
1 parent 82afa93 commit 1dcb2f2
Show file tree
Hide file tree
Showing 5 changed files with 169 additions and 64 deletions.
45 changes: 35 additions & 10 deletions src/components/toaster.tsx
Original file line number Diff line number Diff line change
@@ -1,6 +1,11 @@
import { css, setup } from 'goober';
import * as React from 'react';
import { resolveValue, ToasterProps, ToastPosition } from '../core/types';
import {
resolveValue,
ToastMessageProps,
ToasterProps,
ToastPosition,
} from '../core/types';
import { useToaster } from '../core/use-toaster';
import { createRectRef, prefersReducedMotion } from '../core/utils';
import { ToastBar } from './toast-bar';
Expand Down Expand Up @@ -45,6 +50,30 @@ const activeClass = css`

const DEFAULT_OFFSET = 16;

const ToastMessage: React.FC<ToastMessageProps> = ({
id,
height,
className,
style,
onUpdateHeight,
children,
}) => {
const hasHeight = typeof height === 'number';
const ref = React.useMemo(() => {
return hasHeight
? undefined
: createRectRef((rect) => {
onUpdateHeight(id, rect.height);
});
}, [hasHeight, onUpdateHeight]);

return (
<div ref={ref} className={className} style={style}>
{children}
</div>
);
};

export const Toaster: React.FC<ToasterProps> = ({
reverseOrder,
position = 'top-center',
Expand Down Expand Up @@ -81,18 +110,14 @@ export const Toaster: React.FC<ToasterProps> = ({
});
const positionStyle = getPositionStyle(toastPosition, offset);

const ref = t.height
? undefined
: createRectRef((rect) => {
handlers.updateHeight(t.id, rect.height);
});

return (
<div
ref={ref}
<ToastMessage
id={t.id}
height={t.height}
className={t.visible ? activeClass : ''}
key={t.id}
style={positionStyle}
onUpdateHeight={handlers.updateHeight}
>
{t.type === 'custom' ? (
resolveValue(t.message, t)
Expand All @@ -101,7 +126,7 @@ export const Toaster: React.FC<ToasterProps> = ({
) : (
<ToastBar toast={t} position={toastPosition} />
)}
</div>
</ToastMessage>
);
})}
</div>
Expand Down
14 changes: 7 additions & 7 deletions src/core/store.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,13 +4,13 @@ import { DefaultToastOptions, Toast, ToastType } from './types';
const TOAST_LIMIT = 20;

export enum ActionType {
ADD_TOAST,
UPDATE_TOAST,
UPSERT_TOAST,
DISMISS_TOAST,
REMOVE_TOAST,
START_PAUSE,
END_PAUSE,
ADD_TOAST = 'ADD_TOAST',
UPDATE_TOAST = 'UPDATE_TOAST',
UPSERT_TOAST = 'UPSERT_TOAST',
DISMISS_TOAST = 'DISMISS_TOAST',
REMOVE_TOAST = 'REMOVE_TOAST',
START_PAUSE = 'START_PAUSE',
END_PAUSE = 'END_PAUSE',
}

type Action =
Expand Down
9 changes: 9 additions & 0 deletions src/core/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -73,6 +73,15 @@ export type DefaultToastOptions = ToastOptions &
[key in ToastType]?: ToastOptions;
};

export interface ToastMessageProps {
id: string;
height: number | undefined;
className?: string;
style?: React.CSSProperties;
onUpdateHeight: (id: string, height: number) => void;
children?: React.ReactNode;
}

export interface ToasterProps {
position?: ToastPosition;
toastOptions?: DefaultToastOptions;
Expand Down
100 changes: 53 additions & 47 deletions src/core/use-toaster.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { useEffect, useMemo } from 'react';
import { useEffect, useCallback } from 'react';
import { dispatch, ActionType, useStore } from './store';
import { toast } from './toast';
import { DefaultToastOptions, Toast, ToastPosition } from './types';
Expand Down Expand Up @@ -34,58 +34,64 @@ export const useToaster = (toastOptions?: DefaultToastOptions) => {
};
}, [toasts, pausedAt]);

const handlers = useMemo(
() => ({
startPause: () => {
dispatch({
type: ActionType.START_PAUSE,
time: Date.now(),
});
},
endPause: () => {
if (pausedAt) {
dispatch({ type: ActionType.END_PAUSE, time: Date.now() });
}
},
updateHeight: (toastId: string, height: number) =>
dispatch({
type: ActionType.UPDATE_TOAST,
toast: { id: toastId, height },
}),
calculateOffset: (
toast: Toast,
opts?: {
reverseOrder?: boolean;
gutter?: number;
defaultPosition?: ToastPosition;
}
) => {
const { reverseOrder = false, gutter = 8, defaultPosition } =
opts || {};
const startPause = useCallback(() => {
dispatch({
type: ActionType.START_PAUSE,
time: Date.now(),
});
}, []);

const endPause = useCallback(() => {
if (pausedAt) {
dispatch({ type: ActionType.END_PAUSE, time: Date.now() });
}
}, [pausedAt]);

const updateHeight = useCallback((toastId: string, height: number) => {
dispatch({
type: ActionType.UPDATE_TOAST,
toast: { id: toastId, height },
});
}, []);

const calculateOffset = useCallback(
(
toast: Toast,
opts?: {
reverseOrder?: boolean;
gutter?: number;
defaultPosition?: ToastPosition;
}
) => {
const { reverseOrder = false, gutter = 8, defaultPosition } = opts || {};

const relevantToasts = toasts.filter(
(t) =>
(t.position || defaultPosition) ===
(toast.position || defaultPosition) && t.height
);
const toastIndex = relevantToasts.findIndex((t) => t.id === toast.id);
const toastsBefore = relevantToasts.filter(
(toast, i) => i < toastIndex && toast.visible
).length;
const relevantToasts = toasts.filter(
(t) =>
(t.position || defaultPosition) ===
(toast.position || defaultPosition) && t.height
);
const toastIndex = relevantToasts.findIndex((t) => t.id === toast.id);
const toastsBefore = relevantToasts.filter(
(toast, i) => i < toastIndex && toast.visible
).length;

const offset = relevantToasts
.filter((t) => t.visible)
.slice(...(reverseOrder ? [toastsBefore + 1] : [0, toastsBefore]))
.reduce((acc, t) => acc + (t.height || 0) + gutter, 0);
const offset = relevantToasts
.filter((t) => t.visible)
.slice(...(reverseOrder ? [toastsBefore + 1] : [0, toastsBefore]))
.reduce((acc, t) => acc + (t.height || 0) + gutter, 0);

return offset;
},
}),
[toasts, pausedAt]
return offset;
},
[toasts]
);

return {
toasts,
handlers,
handlers: {
startPause,
endPause,
updateHeight,
calculateOffset,
},
};
};
65 changes: 65 additions & 0 deletions test/toast.test.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,65 @@
import React from 'react';
import { render, screen, act } from '@testing-library/react';
import userEvent from '@testing-library/user-event';
import MatchMediaMock from 'jest-matchmedia-mock';
import toast, { Toaster } from '../src';
import { TOAST_EXPIRE_DISMISS_DELAY } from '../src/core/store';

let matchMedia: MatchMediaMock;

beforeAll(() => {
matchMedia = new MatchMediaMock();
matchMedia.useMediaQuery('(prefers-reduced-motion: reduce)');
});

beforeEach(() => {
jest.useFakeTimers();
});

afterEach((done) => {
act(() => {
jest.runAllTimers();
jest.useRealTimers();
done();
});
});

afterAll(() => {
matchMedia.destroy();
});

test('close notification', async () => {
render(
<>
<Toaster />
<button
type="button"
onClick={() => {
toast((t) => (
<div>
Example
<button
aria-hidden={typeof t.height !== 'number'}
type="button"
onClick={() => toast.dismiss(t.id)}
title={'close'}
>
Close
</button>
</div>
));
}}
>
Notify!
</button>
</>
);
userEvent.click(screen.getByRole('button', { name: /notify/i }));
screen.getByText(/example/i);

userEvent.click(await screen.findByRole('button', { name: /close/i }));
act(() => {
jest.advanceTimersByTime(TOAST_EXPIRE_DISMISS_DELAY);
});
expect(screen.queryByText(/example/i)).not.toBeInTheDocument();
});

0 comments on commit 1dcb2f2

Please sign in to comment.