Skip to content

Commit

Permalink
Merge pull request #9790 from marmelab/typescript-fix-ra-ui-mui-stric…
Browse files Browse the repository at this point in the history
…t-null-checks

[Typescript] Make types more strict in ra-ui-materialui, part II
  • Loading branch information
fzaninotto authored Apr 27, 2024
2 parents 5f33e00 + 64a6891 commit 5a9a7cb
Show file tree
Hide file tree
Showing 31 changed files with 240 additions and 187 deletions.
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'>) =>
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

0 comments on commit 5a9a7cb

Please sign in to comment.