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 III #9795

Merged
merged 8 commits into from
Apr 29, 2024
Merged
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
3 changes: 1 addition & 2 deletions docs/SimpleFormIterator.md
Original file line number Diff line number Diff line change
Expand Up @@ -13,12 +13,11 @@ This component provides a UI for editing arrays of objects, one row per object.
Your browser does not support the video tag.
</video>


`<SimpleFormIterator>` lets users edit, add, remove and reorder sub-records. It is designed to be used as a child of [`<ArrayInput>`](./ArrayInput.md) or [`<ReferenceManyInput>`](./ReferenceManyInput.md). You can also use it within an `ArrayInputContext` containing a *field array*, i.e. the value returned by [react-hook-form's `useFieldArray` hook](https://react-hook-form.com/docs/usefieldarray).

## Usage

`<SimpleFormIterator>` requires no prop by default. It expects an array of inputs as children. It renders these inputs once per row and takes care of setting a different source for each row.
`<SimpleFormIterator>` requires no prop by default. It expects an array of inputs as children. It renders these inputs once per row and takes care of setting a different source for each row.

```jsx
import {
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
import get from 'lodash/get';
import { useMemo } from 'react';
import { RaRecord, SortPayload } from '../../types';
import { useGetManyAggregate } from '../../dataProvider';
import { ListControllerResult, useList } from '../list';
Expand Down Expand Up @@ -72,12 +71,7 @@ export const useReferenceArrayFieldController = <
const notify = useNotify();
const value = get(record, source);
const { meta, ...otherQueryOptions } = queryOptions;

const ids = useMemo(() => {
Copy link
Contributor

Choose a reason for hiding this comment

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

chore: 'useMemo' is imported line 2 but no more used‏, we may could remove it

Copy link
Member Author

Choose a reason for hiding this comment

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

Right, fixed

if (Array.isArray(value)) return value;
console.warn(`Value of field '${source}' is not an array.`, value);
return emptyArray;
}, [value, source]);
const ids = Array.isArray(value) ? value : emptyArray;

const {
data,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -170,7 +170,7 @@ export const useReferenceArrayInputController = <
: undefined,
hasPreviousPage: pageInfo ? pageInfo.hasPreviousPage : params.page > 1,
isFromReference: true,
};
} as ChoicesContextValue<RecordType>;
};

const EmptyArray = [];
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -195,7 +195,7 @@ export const useReferenceInputController = <RecordType extends RaRecord = any>(
: undefined,
hasPreviousPage: pageInfo ? pageInfo.hasPreviousPage : params.page > 1,
isFromReference: true,
};
} as ChoicesContextValue<RecordType>;
};

export interface UseReferenceInputControllerParams<
Expand Down
16 changes: 8 additions & 8 deletions packages/ra-core/src/controller/list/useList.spec.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,7 @@ describe('<useList />', () => {
{ id: 2, title: 'world' },
{ id: 1, title: 'hello' },
],
error: undefined,
Copy link
Member Author

Choose a reason for hiding this comment

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

this is to make the output compatible with the ListControllerResult, which is using useQueryResult signature. In it, the empty error is null, not undefined.

error: null,
total: 2,
})
);
Expand All @@ -79,7 +79,7 @@ describe('<useList />', () => {
{ id: 1, title: 'hello' },
{ id: 2, title: 'world' },
],
error: undefined,
error: null,
total: 2,
})
);
Expand Down Expand Up @@ -120,7 +120,7 @@ describe('<useList />', () => {
],
page: 2,
perPage: 5,
error: undefined,
error: null,
total: 7,
})
);
Expand Down Expand Up @@ -159,7 +159,7 @@ describe('<useList />', () => {
isFetching: true,
isLoading: false,
data: [{ id: 2, title: 'world' }],
error: undefined,
error: null,
total: 1,
})
);
Expand Down Expand Up @@ -188,7 +188,7 @@ describe('<useList />', () => {
isFetching: false,
isLoading: false,
data: [{ id: 2, title: 'world' }],
error: undefined,
error: null,
total: 1,
})
);
Expand Down Expand Up @@ -223,7 +223,7 @@ describe('<useList />', () => {
{ id: 3, items: 'four' },
{ id: 4, items: ['five'] },
],
error: undefined,
error: null,
total: 3,
})
);
Expand Down Expand Up @@ -258,7 +258,7 @@ describe('<useList />', () => {
{ id: 3, items: 'four' },
{ id: 4, items: ['five'] },
],
error: undefined,
error: null,
total: 2,
})
);
Expand Down Expand Up @@ -287,7 +287,7 @@ describe('<useList />', () => {
isFetching: false,
isLoading: false,
data: [{ id: 2, title: { name: 'world' } }],
error: undefined,
error: null,
total: 1,
})
);
Expand Down
4 changes: 2 additions & 2 deletions packages/ra-core/src/controller/list/useList.ts
Original file line number Diff line number Diff line change
Expand Up @@ -262,9 +262,9 @@ export const useList = <RecordType extends RaRecord = any>(

return {
sort,
data: finalItems?.data,
data: pendingState ? undefined : finalItems?.data ?? [],
defaultTitle: '',
error,
error: error ?? null,
displayedFilters,
filterValues,
hasNextPage:
Expand Down
4 changes: 2 additions & 2 deletions packages/ra-core/src/controller/list/useListController.ts
Original file line number Diff line number Diff line change
Expand Up @@ -485,7 +485,7 @@ export interface ListControllerLoadingResult<RecordType extends RaRecord = any>
error: null;
isPending: true;
}
export interface ListControllerLoadingErrorResult<
Copy link
Member Author

Choose a reason for hiding this comment

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

this name didn't make sense

export interface ListControllerErrorResult<
RecordType extends RaRecord = any,
TError = Error
> extends ListControllerBaseResult<RecordType> {
Expand Down Expand Up @@ -513,6 +513,6 @@ export interface ListControllerSuccessResult<RecordType extends RaRecord = any>

export type ListControllerResult<RecordType extends RaRecord = any> =
| ListControllerLoadingResult<RecordType>
| ListControllerLoadingErrorResult<RecordType>
| ListControllerErrorResult<RecordType>
| ListControllerRefetchErrorResult<RecordType>
| ListControllerSuccessResult<RecordType>;
37 changes: 20 additions & 17 deletions packages/ra-core/src/form/FormDataConsumer.spec.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ import {
ArrayInput,
} from 'ra-ui-materialui';
import expect from 'expect';
import { ResourceContextProvider } from '..';

describe('FormDataConsumerView', () => {
it('does not call its children function with scopedFormData if it did not receive a source containing an index', () => {
Expand Down Expand Up @@ -88,22 +89,24 @@ describe('FormDataConsumerView', () => {
let globalScopedFormData;
render(
<AdminContext dataProvider={testDataProvider()}>
<SimpleForm>
<ArrayInput source="authors">
<SimpleFormIterator>
<TextInput source="name" />
<FormDataConsumer>
{({ scopedFormData }) => {
globalScopedFormData = scopedFormData;
return scopedFormData &&
scopedFormData.name ? (
<TextInput source="role" />
) : null;
}}
</FormDataConsumer>
</SimpleFormIterator>
</ArrayInput>
</SimpleForm>
<ResourceContextProvider value="posts">
<SimpleForm>
<ArrayInput source="authors">
<SimpleFormIterator>
<TextInput source="name" />
<FormDataConsumer>
{({ scopedFormData }) => {
globalScopedFormData = scopedFormData;
return scopedFormData &&
scopedFormData.name ? (
<TextInput source="role" />
) : null;
}}
</FormDataConsumer>
</SimpleFormIterator>
</ArrayInput>
</SimpleForm>
</ResourceContextProvider>
</AdminContext>
);

Expand All @@ -114,7 +117,7 @@ describe('FormDataConsumerView', () => {
expect(globalScopedFormData).toEqual({ name: null });

fireEvent.change(
screen.getByLabelText('resources.undefined.fields.authors.name'),
screen.getByLabelText('resources.posts.fields.authors.name'),
{
target: { value: 'a' },
}
Expand Down
57 changes: 49 additions & 8 deletions packages/ra-core/src/form/choices/ChoicesContext.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,24 +11,19 @@ export const ChoicesContext = createContext<ChoicesContextValue | undefined>(
undefined
);

export type ChoicesContextValue<RecordType extends RaRecord = any> = {
allChoices?: RecordType[];
availableChoices?: RecordType[];
export type ChoicesContextBaseValue<RecordType extends RaRecord = any> = {
displayedFilters: any;
error?: any;
filter?: FilterPayload;
filterValues: any;
hasNextPage?: boolean;
hasPreviousPage?: boolean;
hideFilter: (filterName: string) => void;
isFetching: boolean;
isLoading: boolean;
isPending: boolean;
page: number;
perPage: number;
refetch: (() => void) | UseGetListHookValue<RecordType>['refetch'];
resource: string;
selectedChoices?: RecordType[];
setFilters: (
filters: any,
displayedFilters?: any,
Expand All @@ -39,7 +34,53 @@ export type ChoicesContextValue<RecordType extends RaRecord = any> = {
setSort: (sort: SortPayload) => void;
showFilter: (filterName: string, defaultValue: any) => void;
sort: SortPayload;
source?: string;
total?: number;
source: string;
isFromReference: boolean;
};

export interface ChoicesContextLoadingResult<RecordType extends RaRecord = any>
extends ChoicesContextBaseValue<RecordType> {
allChoices: undefined;
availableChoices: undefined;
selectedChoices: undefined;
total: undefined;
error: null;
isPending: true;
}
export interface ChoicesContextErrorResult<
RecordType extends RaRecord = any,
TError = Error
> extends ChoicesContextBaseValue<RecordType> {
allChoices: undefined;
availableChoices: undefined;
selectedChoices: undefined;
total: undefined;
error: TError;
isPending: false;
}
export interface ChoicesContextRefetchErrorResult<
RecordType extends RaRecord = any,
TError = Error
> extends ChoicesContextBaseValue<RecordType> {
allChoices: RecordType[];
availableChoices: RecordType[];
selectedChoices: RecordType[];
total: number;
error: TError;
isPending: false;
}
export interface ChoicesContextSuccessResult<RecordType extends RaRecord = any>
extends ChoicesContextBaseValue<RecordType> {
allChoices: RecordType[];
availableChoices: RecordType[];
selectedChoices: RecordType[];
total: number;
error: null;
isPending: false;
}

export type ChoicesContextValue<RecordType extends RaRecord = any> =
| ChoicesContextLoadingResult<RecordType>
| ChoicesContextErrorResult<RecordType>
| ChoicesContextRefetchErrorResult<RecordType>
| ChoicesContextSuccessResult<RecordType>;
4 changes: 3 additions & 1 deletion packages/ra-core/src/form/choices/useChoicesContext.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,11 +9,13 @@ export const useChoicesContext = <ChoicesType extends RaRecord = RaRecord>(
const context = useContext(ChoicesContext) as ChoicesContextValue<
ChoicesType
>;
// @ts-ignore cannot satisfy the type of useList because of ability to pass partial options
const { data, ...list } = useList<ChoicesType>({
data: options.choices,
isLoading: options.isLoading ?? false,
isPending: options.isPending ?? false,
isFetching: options.isFetching ?? false,
error: options.error,
// When not in a ChoicesContext, paginating does not make sense (e.g. AutocompleteInput).
perPage: Infinity,
});
Expand Down Expand Up @@ -54,5 +56,5 @@ export const useChoicesContext = <ChoicesType extends RaRecord = RaRecord>(
return context;
}, [context, data, list, options]);

return result;
return result as ChoicesContextValue<ChoicesType>;
};
31 changes: 17 additions & 14 deletions packages/ra-core/src/form/useFormGroup.spec.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ import { fireEvent, render, screen, waitFor } from '@testing-library/react';
import expect from 'expect';
import { FormGroupContextProvider } from './FormGroupContextProvider';
import { testDataProvider } from '../dataProvider';
import { ResourceContextProvider } from '..';

describe('useFormGroup', () => {
test.each([
Expand Down Expand Up @@ -150,20 +151,22 @@ describe('useFormGroup', () => {
];
render(
<AdminContext dataProvider={testDataProvider()}>
<SimpleForm>
<FormGroupContextProvider name="backlinks">
<IsDirty />
<ArrayInput
defaultValue={backlinksDefaultValue}
source="backlinks"
>
<SimpleFormIterator>
<TextInput source="url" />
<TextInput source="date" />
</SimpleFormIterator>
</ArrayInput>
</FormGroupContextProvider>
</SimpleForm>
<ResourceContextProvider value="posts">
<SimpleForm>
<FormGroupContextProvider name="backlinks">
<IsDirty />
<ArrayInput
defaultValue={backlinksDefaultValue}
source="backlinks"
>
<SimpleFormIterator>
<TextInput source="url" />
<TextInput source="date" />
</SimpleFormIterator>
</ArrayInput>
</FormGroupContextProvider>
</SimpleForm>
</ResourceContextProvider>
</AdminContext>
);

Expand Down
2 changes: 1 addition & 1 deletion packages/ra-core/src/form/useSuggestions.ts
Original file line number Diff line number Diff line change
Expand Up @@ -87,7 +87,7 @@ const escapeRegExp = value =>
export interface UseSuggestionsOptions extends UseChoicesOptions {
allowCreate?: boolean;
allowDuplicates?: boolean;
choices: any[];
choices?: any[];
createText?: string;
createValue?: any;
limitChoicesToValue?: boolean;
Expand Down
5 changes: 3 additions & 2 deletions packages/ra-core/src/routing/useRestoreScrollPosition.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,8 @@
import { useEffect } from 'react';
import { useStore } from '../store';
import { debounce } from 'lodash';
Copy link
Member Author

Choose a reason for hiding this comment

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

This change is unrelated to the PR, but I noticed that the bundles for the demos were embarking the entire lodash, so I figured I'd fix this regression.

import { useLocation } from 'react-router';
import debounce from 'lodash/debounce';

import { useStore } from '../store';

/**
* A hook that tracks the scroll position and restores it when the component mounts.
Expand Down
1 change: 1 addition & 0 deletions packages/ra-data-fakerest/src/index.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,7 @@ describe('ra-data-fakerest', () => {
expect(data).toEqual([]);
});
it('should return an error when requesting a nonexisting id', async () => {
jest.spyOn(console, 'error').mockImplementationOnce(() => {});
const dataProvider = fakerestDataProvider({
posts: [
{ id: 0, title: 'Hello, world!' },
Expand Down
Loading
Loading