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

[RFR] [BC Break] Reference hooks cleanup #3446

Merged
merged 9 commits into from
Jul 24, 2019
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
25 changes: 24 additions & 1 deletion UPGRADE.md
Original file line number Diff line number Diff line change
Expand Up @@ -331,4 +331,27 @@ When using custom component with ReferenceInputController, you should rename the
+ />
+ )}
+ </ReferenceInputController>
```
```

## `loadedOnce` prop renamed as `loaded`

The `List`, `ReferenceArrayfield` and `ReferenceManyField` used to inject an `loadedOnce` prop to their child. This prop has been renamed to `loaded`.

As a consequence, the components usually used as children of these 3 components now accept a `loaded` prop instead of `loadedOnce`. This concerns `Datagrid`, `SingleFieldList`, and `GridList`.

This change is transparent unless you use a custom view component inside a `List`, `ReferenceArrayfield` or `ReferenceManyField`.

```diff
const PostList = props => (
<List {...props}>
<MyListView />
</List>
)

-const MyListView = ({ loadedOnce, ...props }) => (
+const MyListView = ({ loaded, ...props }) => (
- if (!loadedOnce) return null;
+ if (!loaded) return null;
// rest of the view
);
```
4 changes: 2 additions & 2 deletions examples/demo/src/products/GridList.js
Original file line number Diff line number Diff line change
Expand Up @@ -106,7 +106,7 @@ const LoadedGridList = ({ ids, data, basePath, width }) => {
);
};

const GridList = ({ loadedOnce, ...props }) =>
loadedOnce ? <LoadedGridList {...props} /> : <LoadingGridList {...props} />;
Copy link
Collaborator

Choose a reason for hiding this comment

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

Isn't it a breaking change too for custom views ?

Copy link
Member Author

Choose a reason for hiding this comment

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

Not sure I understand your remark, but yes, it is a breaking change, hence the new section in the upgrade guide.

Copy link
Collaborator

Choose a reason for hiding this comment

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

The new section says nothing about loadedOnce, only isLoaded

Copy link
Member Author

Choose a reason for hiding this comment

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

my bad, it's loadedOnce, not isLoaded. I'll fix the upgrade guide right away.

const GridList = ({ loaded, ...props }) =>
loaded ? <LoadedGridList {...props} /> : <LoadingGridList {...props} />;

export default withWidth()(GridList);
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ import { crudGetManyAccumulate } from '../../actions';

describe('<ReferenceArrayFieldController />', () => {
afterEach(cleanup);
it('should set the loadedOnce prop to false when related records are not yet fetched', () => {
it('should set the loaded prop to false when related records are not yet fetched', () => {
const children = jest.fn().mockReturnValue('child');

renderWithRedux(
Expand All @@ -32,14 +32,14 @@ describe('<ReferenceArrayFieldController />', () => {
);
expect(children.mock.calls[0][0]).toEqual({
currentSort: { field: 'id', order: 'ASC' },
loadedOnce: false,
loaded: false,
referenceBasePath: '',
data: null,
ids: [1, 2],
});
});

it('should set the loadedOnce prop to true when at least one related record is found', () => {
it('should set the loaded prop to true when at least one related record is found', () => {
const children = jest.fn().mockReturnValue('child');

renderWithRedux(
Expand Down Expand Up @@ -70,7 +70,7 @@ describe('<ReferenceArrayFieldController />', () => {

expect(children.mock.calls[0][0]).toEqual({
currentSort: { field: 'id', order: 'ASC' },
loadedOnce: true,
loaded: true,
referenceBasePath: '',
data: {
2: {
Expand Down Expand Up @@ -109,7 +109,7 @@ describe('<ReferenceArrayFieldController />', () => {
);
expect(children.mock.calls[0][0]).toEqual({
currentSort: { field: 'id', order: 'ASC' },
loadedOnce: true,
loaded: true,
referenceBasePath: '',
data: {
1: { id: 1, title: 'hello' },
Expand Down Expand Up @@ -146,7 +146,7 @@ describe('<ReferenceArrayFieldController />', () => {
);
expect(children.mock.calls[0][0]).toEqual({
currentSort: { field: 'id', order: 'ASC' },
loadedOnce: true,
loaded: true,
referenceBasePath: '',
data: {
'abc-1': { id: 'abc-1', title: 'hello' },
Expand Down
Original file line number Diff line number Diff line change
@@ -1,10 +1,10 @@
import { FunctionComponent, ReactNode, ReactElement } from 'react';

import useReferenceArray from './useReferenceArray';
import useReferenceArrayFieldController from './useReferenceArrayFieldController';
import { Identifier, RecordMap, Record, Sort } from '../..';

interface ChildrenFuncParams {
loadedOnce: boolean;
loaded: boolean;
ids: Identifier[];
data: RecordMap;
referenceBasePath: string;
Expand All @@ -21,36 +21,9 @@ interface Props {
}

/**
* A container component that fetches records from another resource specified
* by an array of *ids* in current record.
*
* You must define the fields to be passed to the iterator component as children.
*
* @example Display all the products of the current order as datagrid
* // order = {
* // id: 123,
* // product_ids: [456, 457, 458],
* // }
* <ReferenceArrayField label="Products" reference="products" source="product_ids">
* <Datagrid>
* <TextField source="id" />
* <TextField source="description" />
* <NumberField source="price" options={{ style: 'currency', currency: 'USD' }} />
* <EditButton />
* </Datagrid>
* </ReferenceArrayField>
*
* @example Display all the categories of the current product as a list of chips
* // product = {
* // id: 456,
* // category_ids: [11, 22, 33],
* // }
* <ReferenceArrayField label="Categories" reference="categories" source="category_ids">
* <SingleFieldList>
* <ChipField source="name" />
* </SingleFieldList>
* </ReferenceArrayField>
* Render prop version of the useReferenceArrayFieldController hook.
*
* @see useReferenceArrayFieldController
*/
const ReferenceArrayFieldController: FunctionComponent<Props> = ({
resource,
Expand All @@ -65,7 +38,7 @@ const ReferenceArrayFieldController: FunctionComponent<Props> = ({
field: 'id',
order: 'ASC',
},
...useReferenceArray({
...useReferenceArrayFieldController({
resource,
reference,
basePath,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ import ReferenceManyFieldController from './ReferenceManyFieldController';
import renderWithRedux from '../../util/renderWithRedux';

describe('<ReferenceManyFieldController />', () => {
it('should set loadedOnce to false when related records are not yet fetched', () => {
it('should set loaded to false when related records are not yet fetched', () => {
const children = jest.fn().mockReturnValue('children');
const { dispatch } = renderWithRedux(
<ReferenceManyFieldController
Expand Down
Original file line number Diff line number Diff line change
@@ -1,15 +1,15 @@
import { ReactElement, FunctionComponent } from 'react';

import { Record, Sort, RecordMap, Identifier } from '../../types';
import useReferenceMany from './useReferenceMany';
import useReferenceManyFieldController from './useReferenceManyFieldController';
import useSortState from '../useSortState';
import usePaginationState from '../usePaginationState';

interface ChildrenFuncParams {
currentSort: Sort;
data: RecordMap;
ids: Identifier[];
loadedOnce: boolean;
loaded: boolean;
page: number;
perPage: number;
referenceBasePath: string;
Expand All @@ -36,50 +36,9 @@ interface Props {
const defaultPerPage = 25;

/**
* Render related records to the current one.
* Render prop version of the useReferenceManyFieldController hook.
*
* You must define the fields to be passed to the iterator component as children.
*
* @example Display all the comments of the current post as a datagrid
* <ReferenceManyField reference="comments" target="post_id">
* <Datagrid>
* <TextField source="id" />
* <TextField source="body" />
* <DateField source="created_at" />
* <EditButton />
* </Datagrid>
* </ReferenceManyField>
*
* @example Display all the books by the current author, only the title
* <ReferenceManyField reference="books" target="author_id">
* <SingleFieldList>
* <ChipField source="title" />
* </SingleFieldList>
* </ReferenceManyField>
*
* By default, restricts the possible values to 25. You can extend this limit
* by setting the `perPage` prop.
*
* @example
* <ReferenceManyField perPage={10} reference="comments" target="post_id">
* ...
* </ReferenceManyField>
*
* By default, orders the possible values by id desc. You can change this order
* by setting the `sort` prop (an object with `field` and `order` properties).
*
* @example
* <ReferenceManyField sort={{ field: 'created_at', order: 'DESC' }} reference="comments" target="post_id">
* ...
* </ReferenceManyField>
*
* Also, you can filter the query used to populate the possible values. Use the
* `filter` prop for that.
*
* @example
* <ReferenceManyField filter={{ is_published: true }} reference="comments" target="post_id">
* ...
* </ReferenceManyField>
* @see useReferenceManyFieldController
*/
export const ReferenceManyFieldController: FunctionComponent<Props> = ({
resource,
Expand All @@ -100,10 +59,10 @@ export const ReferenceManyFieldController: FunctionComponent<Props> = ({
const {
data,
ids,
loadedOnce,
loaded,
referenceBasePath,
total,
} = useReferenceMany({
} = useReferenceManyFieldController({
resource,
reference,
record,
Expand All @@ -120,7 +79,7 @@ export const ReferenceManyFieldController: FunctionComponent<Props> = ({
currentSort: sort,
data,
ids,
loadedOnce,
loaded,
page,
perPage,
referenceBasePath,
Expand Down
8 changes: 4 additions & 4 deletions packages/ra-core/src/controller/field/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,14 +2,14 @@ import ReferenceArrayFieldController from './ReferenceArrayFieldController';
import ReferenceFieldController from './ReferenceFieldController';
import ReferenceManyFieldController from './ReferenceManyFieldController';
import getResourceLinkPath from './getResourceLinkPath';
import useReferenceArray from './useReferenceArray';
import useReferenceMany from './useReferenceMany';
import useReferenceArrayFieldController from './useReferenceArrayFieldController';
import useReferenceManyFieldController from './useReferenceManyFieldController';

export {
useReferenceArray,
useReferenceArrayFieldController,
ReferenceArrayFieldController,
ReferenceFieldController,
getResourceLinkPath,
useReferenceMany,
useReferenceManyFieldController,
ReferenceManyFieldController,
};
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ import { getReferencesByIds } from '../../reducer/admin/references/oneToMany';
import { ReduxState, Record, RecordMap, Identifier } from '../../types';

interface ReferenceArrayProps {
loadedOnce: boolean;
loaded: boolean;
ids: Identifier[];
data: RecordMap;
referenceBasePath: string;
Expand All @@ -25,7 +25,7 @@ interface Option {
/**
* @typedef ReferenceArrayProps
* @type {Object}
* @property {boolean} loadedOnce: boolean indicating if the reference has already beeen loaded
* @property {boolean} loaded: boolean indicating if the reference has already beeen loaded
* @property {Array} ids: the list of ids.
* @property {Object} data: Object holding the reference data by their ids
* @property {string} referenceBasePath basePath of the reference
Expand All @@ -37,7 +37,7 @@ interface Option {
*
* @example
*
* const { loadedOnce, data, ids, referenceBasePath, currentSort } = useReferenceArray({
* const { loaded, data, ids, referenceBasePath, currentSort } = useReferenceArrayFieldController({
* basePath: 'resource';
* record: { referenceIds: ['id1', 'id2']};
* reference: 'reference';
Expand All @@ -56,7 +56,7 @@ interface Option {
*
* @returns {ReferenceProps} The reference props
*/
const useReferenceArray = ({
const useReferenceArrayFieldController = ({
resource,
reference,
basePath,
Expand All @@ -76,7 +76,7 @@ const useReferenceArray = ({

return {
// eslint-disable-next-line eqeqeq
loadedOnce: data != undefined,
loaded: data != undefined,
ids,
data,
referenceBasePath,
Expand All @@ -93,4 +93,4 @@ const getReferenceArray = ({ record, source, reference }) => (
};
};

export default useReferenceArray;
export default useReferenceArrayFieldController;
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ import { Record, Sort, RecordMap, Identifier } from '../../types';
interface ReferenceManyProps {
data: RecordMap;
ids: Identifier[];
loadedOnce: boolean;
loaded: boolean;
referenceBasePath: string;
total: number;
}
Expand All @@ -25,7 +25,7 @@ interface Options {
data?: RecordMap;
filter?: any;
ids?: any[];
loadedOnce?: boolean;
loaded?: boolean;
page: number;
perPage: number;
record?: Record;
Expand All @@ -44,7 +44,7 @@ const defaultFilter = {};
* @type {Object}
* @property {Object} data: the referenced records dictionary by their ids.
* @property {Object} ids: the list of referenced records ids.
* @property {boolean} loadedOnce: boolean indicating if the references has already be loaded loaded
* @property {boolean} loaded: boolean indicating if the references has already be loaded loaded
* @property {string | false} referenceBasePath base path of the related record
*/

Expand All @@ -56,7 +56,7 @@ const defaultFilter = {};
*
* @example
*
* const { isLoading, referenceRecord, resourceLinkPath } = useReferenceMany({
* const { isLoading, referenceRecord, resourceLinkPath } = useReferenceManyFieldController({
* resource
* reference: 'users',
* record: {
Expand All @@ -83,7 +83,7 @@ const defaultFilter = {};
*
* @returns {ReferenceManyProps} The reference many props
*/
const useReferenceMany = ({
const useReferenceManyFieldController = ({
resource,
reference,
record,
Expand Down Expand Up @@ -139,7 +139,7 @@ const useReferenceMany = ({
return {
data,
ids,
loadedOnce: typeof ids !== 'undefined',
loaded: typeof ids !== 'undefined',
referenceBasePath,
total,
};
Expand Down Expand Up @@ -177,4 +177,4 @@ const selectData = (reference, relatedTo) => state =>
const selectIds = relatedTo => state => getIds(state, relatedTo);
const selectTotal = relatedTo => state => getTotal(state, relatedTo);

export default useReferenceMany;
export default useReferenceManyFieldController;
Loading