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

[Typescript] Make types more strict in ra-ui-materialui, part II #9790

Merged
merged 12 commits into from
Apr 27, 2024
Original file line number Diff line number Diff line change
Expand Up @@ -151,14 +151,13 @@ describe('useEditController', () => {

describe('queryOptions', () => {
it('should accept custom client query options', async () => {
const mock = jest
.spyOn(console, 'error')
.mockImplementation(() => {});
jest.spyOn(console, 'error').mockImplementationOnce(() => {});
const getOne = jest
.fn()
.mockImplementationOnce(() => Promise.reject(new Error()));
const onError = jest.fn();
const dataProvider = ({ getOne } as unknown) as DataProvider;

render(
<CoreAdminContext dataProvider={dataProvider}>
<EditController
Expand All @@ -170,11 +169,11 @@ describe('useEditController', () => {
</EditController>
</CoreAdminContext>
);

await waitFor(() => {
expect(getOne).toHaveBeenCalled();
expect(onError).toHaveBeenCalled();
});
mock.mockRestore();
});

it('should accept a meta in query options', async () => {
Expand Down Expand Up @@ -959,6 +958,7 @@ describe('useEditController', () => {

it('should return errors from the update call in pessimistic mode', async () => {
let post = { id: 12 };
jest.spyOn(console, 'error').mockImplementationOnce(() => {});
const update = jest.fn().mockImplementationOnce(() => {
return Promise.reject({ body: { errors: { foo: 'invalid' } } });
});
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -130,10 +130,15 @@ describe('<ReferenceInputBase />', () => {
});

it('should use meta when fetching current value', async () => {
const getList = jest
.fn()
.mockImplementationOnce(() =>
Promise.resolve({ data: [], total: 25 })
);
const getMany = jest
.fn()
.mockImplementationOnce(() => Promise.resolve({ data: [] }));
const dataProvider = testDataProvider({ getMany });
const dataProvider = testDataProvider({ getList, getMany });
render(
<CoreAdminContext dataProvider={dataProvider}>
<Form record={{ post_id: 23 }}>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -43,9 +43,7 @@ describe('useInfiniteListController', () => {

describe('queryOptions', () => {
it('should accept custom client query options', async () => {
const mock = jest
.spyOn(console, 'error')
.mockImplementation(() => {});
jest.spyOn(console, 'error').mockImplementationOnce(() => {});
const getList = jest
.fn()
.mockImplementationOnce(() => Promise.reject(new Error()));
Expand All @@ -65,7 +63,6 @@ describe('useInfiniteListController', () => {
expect(getList).toHaveBeenCalled();
expect(onError).toHaveBeenCalled();
});
mock.mockRestore();
});

it('should accept meta in queryOptions', async () => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -29,9 +29,7 @@ describe('useListController', () => {

describe('queryOptions', () => {
it('should accept custom client query options', async () => {
const mock = jest
.spyOn(console, 'error')
.mockImplementation(() => {});
jest.spyOn(console, 'error').mockImplementationOnce(() => {});
const getList = jest
.fn()
.mockImplementationOnce(() => Promise.reject(new Error()));
Expand All @@ -48,7 +46,6 @@ describe('useListController', () => {
expect(getList).toHaveBeenCalled();
expect(onError).toHaveBeenCalled();
});
mock.mockRestore();
});

it('should accept meta in queryOptions', async () => {
Expand Down
18 changes: 9 additions & 9 deletions packages/ra-core/src/dataProvider/defaultDataProvider.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,30 +3,30 @@ import { DataProvider } from '../types';
// avoids adding a context in tests
export const defaultDataProvider: DataProvider = {
create: async () => {
throw new Error('not implemented');
throw new Error('create is not implemented');
},
delete: async () => {
throw new Error('not implemented');
throw new Error('delete not implemented');
},
deleteMany: async () => {
throw new Error('not implemented');
throw new Error('deleteMany is not implemented');
},
getList: async () => {
throw new Error('not implemented');
throw new Error('getList is not implemented');
},
getMany: async () => {
throw new Error('not implemented');
throw new Error('getMany is not implemented');
},
getManyReference: async () => {
throw new Error('not implemented');
throw new Error('getManyReference is not implemented');
},
getOne: async () => {
throw new Error('not implemented');
throw new Error('getOne is not implemented');
},
update: async () => {
throw new Error('not implemented');
throw new Error('update not implemented');
},
updateMany: async () => {
throw new Error('not implemented');
throw new Error('updateMany not implemented');
},
};
1 change: 1 addition & 0 deletions packages/ra-core/src/form/Form.stories.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -97,6 +97,7 @@ const CustomInput = (props: UseControllerProps) => {
type="text"
aria-invalid={fieldState.invalid}
{...field}
value={field.value ?? ''}
/>
<p>{fieldState.error?.message}</p>
</div>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ export const NotificationContextProvider = ({ children }) => {
}, []);

const takeNotification = useCallback(() => {
if (notifications.length === 0) return;
const [notification, ...rest] = notifications;
setNotifications(rest);
return notification;
Expand Down
1 change: 1 addition & 0 deletions packages/ra-core/src/util/ComponentPropType.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,4 +6,5 @@ export default (props, propName, componentName) => {
`Invalid prop '${propName}' supplied to '${componentName}': the prop is not a valid React component`
);
}
return null;
};
2 changes: 1 addition & 1 deletion packages/ra-ui-materialui/src/auth/Login.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ import { LoginForm as DefaultLoginForm } from './LoginForm';
*/
export const Login = (props: LoginProps) => {
const { children = defaultLoginForm, backgroundImage, ...rest } = props;
const containerRef = useRef<HTMLDivElement>();
const containerRef = useRef<HTMLDivElement>(null);
let backgroundImageLoaded = false;
const checkAuth = useCheckAuth();
const navigate = useNavigate();
Expand Down
10 changes: 2 additions & 8 deletions packages/ra-ui-materialui/src/button/ExportButton.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -92,10 +92,8 @@ const defaultIcon = <DownloadIcon />;
const sanitizeRestProps = ({
resource,
...rest
}: Omit<
ExportButtonProps,
'sort' | 'maxResults' | 'label' | 'exporter' | 'meta'
>) => rest;
}: Omit<ExportButtonProps, 'maxResults' | 'label' | 'exporter' | 'meta'>) =>
adguernier marked this conversation as resolved.
Show resolved Hide resolved
rest;

interface Props {
exporter?: Exporter;
Expand All @@ -115,10 +113,6 @@ ExportButton.propTypes = {
label: PropTypes.string,
maxResults: PropTypes.number,
resource: PropTypes.string,
sort: PropTypes.exact({
field: PropTypes.string,
order: PropTypes.oneOf(['ASC', 'DESC'] as const),
}),
icon: PropTypes.element,
meta: PropTypes.any,
};
2 changes: 0 additions & 2 deletions packages/ra-ui-materialui/src/button/PrevNextButtons.spec.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -15,8 +15,6 @@ import {
describe('<PrevNextButtons />', () => {
beforeEach(() => {
window.scrollTo = jest.fn();
// avoid logs due to the use of ListGuesser
console.log = jest.fn();
});

afterEach(() => {
Expand Down
4 changes: 3 additions & 1 deletion packages/ra-ui-materialui/src/layout/Layout.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,9 @@ export const Layout = (props: LayoutProps) => {
...rest
} = props;

const [errorInfo, setErrorInfo] = useState<ErrorInfo>(null);
const [errorInfo, setErrorInfo] = useState<ErrorInfo | undefined>(
undefined
);

const handleError = (error: Error, info: ErrorInfo) => {
setErrorInfo(info);
Expand Down
74 changes: 44 additions & 30 deletions packages/ra-ui-materialui/src/layout/Notification.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import {
useNotificationContext,
undoableEventEmitter,
useTranslate,
NotificationPayload,
} from 'ra-core';

const defaultAnchorOrigin: SnackbarOrigin = {
Expand Down Expand Up @@ -39,7 +40,9 @@ export const Notification = (props: NotificationProps) => {
} = props;
const { notifications, takeNotification } = useNotificationContext();
const [open, setOpen] = useState(false);
const [messageInfo, setMessageInfo] = React.useState(undefined);
const [currentNotification, setCurrentNotification] = React.useState<
NotificationPayload | undefined
>(undefined);
const translate = useTranslate();

useEffect(() => {
Expand All @@ -50,54 +53,61 @@ export const Notification = (props: NotificationProps) => {
return confirmationMessage;
};

if (messageInfo?.notificationOptions?.undoable) {
if (currentNotification?.notificationOptions?.undoable) {
window.addEventListener('beforeunload', beforeunload);
}

if (notifications.length && !messageInfo) {
if (notifications.length && !currentNotification) {
// Set a new snack when we don't have an active one
setMessageInfo(takeNotification());
setOpen(true);
} else if (notifications.length && messageInfo && open) {
const notification = takeNotification();
if (notification) {
setCurrentNotification(notification);
setOpen(true);
}
} else if (notifications.length && currentNotification && open) {
// Close an active snack when a new one is added
setOpen(false);
}

return () => {
if (messageInfo?.notificationOptions?.undoable) {
if (currentNotification?.notificationOptions?.undoable) {
window.removeEventListener('beforeunload', beforeunload);
}
};
}, [notifications, messageInfo, open, takeNotification]);
}, [notifications, currentNotification, open, takeNotification]);

const handleRequestClose = useCallback(() => {
setOpen(false);
}, [setOpen]);

const handleExited = useCallback(() => {
if (messageInfo && messageInfo.notificationOptions.undoable) {
if (
currentNotification &&
currentNotification.notificationOptions?.undoable
) {
undoableEventEmitter.emit('end', { isUndo: false });
}
setMessageInfo(undefined);
}, [messageInfo]);
setCurrentNotification(undefined);
}, [currentNotification]);

const handleUndo = useCallback(() => {
undoableEventEmitter.emit('end', { isUndo: true });
setOpen(false);
}, []);

if (!messageInfo) return null;
if (!currentNotification) return null;
const {
message,
type: typeFromMessage,
notificationOptions: {
autoHideDuration: autoHideDurationFromMessage,
messageArgs,
multiLine: multilineFromMessage,
undoable,
...options
},
} = messageInfo;
notificationOptions,
} = currentNotification;
const {
autoHideDuration: autoHideDurationFromMessage,
messageArgs,
multiLine: multilineFromMessage,
undoable,
...options
} = notificationOptions || {};

return (
<StyledSnackbar
Expand All @@ -113,7 +123,7 @@ export const Notification = (props: NotificationProps) => {
// as 0 and null are valid values
autoHideDurationFromMessage === undefined
? autoHideDuration
: autoHideDurationFromMessage
: autoHideDurationFromMessage ?? undefined
}
disableWindowBlurListener={undoable}
TransitionProps={{ onExited: handleExited }}
Expand All @@ -140,7 +150,11 @@ export const Notification = (props: NotificationProps) => {
{...rest}
{...options}
>
{message && typeof message !== 'string' ? message : null}
{message &&
typeof message !== 'string' &&
React.isValidElement(message)
? message
: undefined}
</StyledSnackbar>
);
};
Expand All @@ -166,25 +180,25 @@ const StyledSnackbar = styled(Snackbar, {
overridesResolver: (props, styles) => styles.root,
})(({ theme, type }: NotificationProps & { theme?: Theme }) => ({
[`& .${NotificationClasses.success}`]: {
backgroundColor: theme.palette.success.main,
color: theme.palette.success.contrastText,
backgroundColor: theme?.palette.success.main,
color: theme?.palette.success.contrastText,
},

[`& .${NotificationClasses.error}`]: {
backgroundColor: theme.palette.error.main,
color: theme.palette.error.contrastText,
backgroundColor: theme?.palette.error.main,
color: theme?.palette.error.contrastText,
},

[`& .${NotificationClasses.warning}`]: {
backgroundColor: theme.palette.warning.main,
color: theme.palette.warning.contrastText,
backgroundColor: theme?.palette.warning.main,
color: theme?.palette.warning.contrastText,
},

[`& .${NotificationClasses.undo}`]: {
color:
type === 'success'
? theme.palette.success.contrastText
: theme.palette.primary.light,
? theme?.palette.success.contrastText
: theme?.palette.primary.light,
},
[`& .${NotificationClasses.multiLine}`]: {
whiteSpace: 'pre-wrap',
Expand Down
2 changes: 1 addition & 1 deletion packages/ra-ui-materialui/src/layout/Title.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ import { PageTitleConfigurable } from './PageTitleConfigurable';

export const Title = (props: TitleProps) => {
const { defaultTitle, title, preferenceKey, ...rest } = props;
const [container, setContainer] = useState<HTMLElement>(() =>
const [container, setContainer] = useState<HTMLElement | null>(() =>
typeof document !== 'undefined'
? document.getElementById('react-admin-title')
: null
Expand Down
2 changes: 1 addition & 1 deletion packages/ra-ui-materialui/src/layout/UserMenu.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -107,7 +107,7 @@ export const UserMenu = (props: UserMenuProps) => {
<Tooltip title={label && translate(label, { _: 'Profile' })}>
<IconButton
aria-label={label && translate(label, { _: 'Profile' })}
aria-owns={open ? 'menu-appbar' : null}
aria-owns={open ? 'menu-appbar' : undefined}
aria-haspopup={true}
color="inherit"
onClick={handleMenu}
Expand Down
4 changes: 3 additions & 1 deletion packages/ra-ui-materialui/src/layout/UserMenuContext.ts
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,9 @@ import { createContext } from 'react';
* </UserMenu>
* );
*/
export const UserMenuContext = createContext<UserMenuContextValue>(undefined);
export const UserMenuContext = createContext<UserMenuContextValue | undefined>(
undefined
);

export type UserMenuContextValue = {
/**
Expand Down
Loading
Loading