Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix infinite loop during unit testing #154

Closed
wants to merge 3 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions .github/workflows/main.yml
Original file line number Diff line number Diff line change
Expand Up @@ -20,8 +20,8 @@ jobs:
- name: Lint
run: yarn lint

# - name: Test
# run: yarn test --ci --coverage --maxWorkers=2
- name: Test
run: yarn test --ci --coverage --maxWorkers=2

- name: Build
run: yarn build
6 changes: 6 additions & 0 deletions jest.config.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
/** @type {import('ts-jest/dist/types').InitialOptionsTsJest} */
module.exports = {
preset: 'ts-jest',
testEnvironment: 'jsdom',
setupFilesAfterEnv: ['<rootDir>/test/setup.ts'],
};
48,174 changes: 36,740 additions & 11,434 deletions package-lock.json

Large diffs are not rendered by default.

12 changes: 10 additions & 2 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@
"start": "tsdx watch",
"build": "tsdx build",
"lint": "tsdx lint",
"test": "echo \"No test specified\"",
"test": "jest",
"prepare": "tsdx build",
"size": "size-limit",
"analyze": "size-limit --why"
Expand Down Expand Up @@ -60,11 +60,19 @@
],
"devDependencies": {
"@size-limit/preset-small-lib": "^4.9.1",
"@testing-library/jest-dom": "^5.16.1",
"@testing-library/react": "^12.1.2",
"@testing-library/user-event": "^13.5.0",
"@types/jest": "^27.4.0",
"@types/react": "^17.0.0",
"@types/react-dom": "^17.0.0",
"csstype": "^3.0.7",
"jest": "^27.4.7",
"jest-matchmedia-mock": "^1.1.0",
"size-limit": "^4.9.1",
"tsdx": "^0.14.1"
"ts-jest": "^27.1.3",
"tsdx": "^0.14.1",
"typescript": "^3.9.10"
},
"dependencies": {
"goober": "^2.1.1"
Expand Down
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
4 changes: 3 additions & 1 deletion src/core/store.ts
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,8 @@ interface State {

const toastTimeouts = new Map<Toast['id'], ReturnType<typeof setTimeout>>();

export const TOAST_EXPIRE_DISMISS_DELAY = 1000;

const addToRemoveQueue = (toastId: string) => {
if (toastTimeouts.has(toastId)) {
return;
Expand All @@ -61,7 +63,7 @@ const addToRemoveQueue = (toastId: string) => {
type: ActionType.REMOVE_TOAST,
toastId: toastId,
});
}, 1000);
}, TOAST_EXPIRE_DISMISS_DELAY);

toastTimeouts.set(toastId, timeout);
};
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(() => {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I memoized all of these handlers individually because I needed updateHeight to be properly memoized instead of changing every time one of the other handlers change.

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,
},
};
};
1 change: 1 addition & 0 deletions test/setup.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
import '@testing-library/jest-dom';
73 changes: 63 additions & 10 deletions test/toast.test.tsx
Original file line number Diff line number Diff line change
@@ -1,12 +1,65 @@
import * as React from 'react';
import * as ReactDOM from 'react-dom';
import { Toaster, toast } from '../src';

describe('it', () => {
it('renders without crashing', () => {
const div = document.createElement('div');
toast('Hello World');
ReactDOM.render(<Toaster />, div);
ReactDOM.unmountComponentAtNode(div);
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();
});
2 changes: 1 addition & 1 deletion tsconfig.json
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
{
// see https://www.typescriptlang.org/tsconfig to better understand tsconfigs
"include": ["src", "types"],
"include": ["src", "types", "../test"],
"compilerOptions": {
"module": "esnext",
"lib": ["dom", "esnext"],
Expand Down