From 02c488b1af6a68365f9ccfdd15cbff9625cb77c1 Mon Sep 17 00:00:00 2001 From: thiery Date: Thu, 6 Jun 2019 09:17:40 +0200 Subject: [PATCH 01/48] refactor ReferenceInputController into a function component --- .../field/ReferenceManyFieldController.tsx | 4 +- .../input/ReferenceInputController.tsx | 290 +++++++++--------- .../src/controller/usePaginationState.ts | 14 + .../ra-core/src/controller/useSortState.ts | 6 +- 4 files changed, 168 insertions(+), 146 deletions(-) diff --git a/packages/ra-core/src/controller/field/ReferenceManyFieldController.tsx b/packages/ra-core/src/controller/field/ReferenceManyFieldController.tsx index e040daa14cb..b005c914c78 100644 --- a/packages/ra-core/src/controller/field/ReferenceManyFieldController.tsx +++ b/packages/ra-core/src/controller/field/ReferenceManyFieldController.tsx @@ -91,7 +91,7 @@ export const ReferenceManyFieldController: FunctionComponent = ({ sort: initialSort, children, }) => { - const { sort, setSort } = useSortState(initialSort); + const { sort, setSortField } = useSortState(initialSort); const { page, perPage, setPage, setPerPage } = usePaginationState( initialPerPage ); @@ -124,7 +124,7 @@ export const ReferenceManyFieldController: FunctionComponent = ({ referenceBasePath, setPage, setPerPage, - setSort, + setSort: setSortField, total, }); }; diff --git a/packages/ra-core/src/controller/input/ReferenceInputController.tsx b/packages/ra-core/src/controller/input/ReferenceInputController.tsx index 89b82a9830c..32930f18934 100644 --- a/packages/ra-core/src/controller/input/ReferenceInputController.tsx +++ b/packages/ra-core/src/controller/input/ReferenceInputController.tsx @@ -1,10 +1,18 @@ -import { Component, ReactNode, ComponentType } from 'react'; +import { + ReactNode, + ComponentType, + FunctionComponent, + useState, + ReactElement, + useEffect, + useRef, +} from 'react'; import { connect } from 'react-redux'; import debounce from 'lodash/debounce'; import compose from 'recompose/compose'; import { createSelector } from 'reselect'; -import isEqual from 'lodash/isEqual'; import { WrappedFieldInputProps } from 'redux-form'; +import isEqual from 'lodash/isEqual'; import { crudGetManyAccumulate as crudGetManyAccumulateAction, @@ -19,6 +27,8 @@ import { getStatusForInput as getDataStatus } from './referenceDataStatus'; import withTranslate from '../../i18n/translate'; import { Sort, Translate, Record, Pagination, Dispatch } from '../../types'; import { MatchingReferencesError } from './types'; +import usePaginationState from '../usePaginationState'; +import useSortState from '../useSortState'; const defaultReferenceSource = (resource: string, source: string) => `${resource}@${source}`; @@ -62,11 +72,46 @@ interface EnhancedProps { translate: Translate; } -interface State { - pagination: Pagination; - sort: Sort; - filter: any; -} +const usePrevious = value => { + const ref = useRef(); + + useEffect(() => { + ref.current = value; + }, [value]); + + return ref.current; +}; + +const useFilterState = ({ + filterToQuery = v => v, + initialFilter, + debounceTime = 500, +}) => { + const previousInitialFilter = usePrevious(initialFilter); + const [filter, setFilterValue] = useState(filterToQuery(initialFilter)); + + const setFilter = debounce( + value => setFilterValue(filterToQuery(value)), + debounceTime + ); + const isFirstRender = useRef(true); + + useEffect(() => { + if (isFirstRender.current) { + isFirstRender.current = false; + return; + } + if (isEqual(initialFilter, previousInitialFilter)) { + return; + } + setFilter(initialFilter); + }, [initialFilter]); + + return { + filter, + setFilter, + }; +}; /** * An Input component for choosing a reference record. Useful for foreign keys. @@ -147,152 +192,113 @@ interface State { * * */ -export class UnconnectedReferenceInputController extends Component< - Props & EnhancedProps, - State -> { - public static defaultProps = { - allowEmpty: false, - filter: {}, - filterToQuery: searchText => ({ q: searchText }), - matchingReferences: null, - perPage: 25, - sort: { field: 'id', order: 'DESC' }, - referenceRecord: null, - referenceSource: defaultReferenceSource, // used in tests - }; - - public state: State; - private debouncedSetFilter; - - constructor(props) { - super(props); - const { perPage, sort, filter } = props; - this.state = { pagination: { page: 1, perPage }, sort, filter }; - this.debouncedSetFilter = debounce(this.setFilter.bind(this), 500); - } - - componentDidMount() { - this.fetchReferenceAndOptions(this.props); - } - - componentWillReceiveProps(nextProps: Props & EnhancedProps) { - if ( - (this.props.record || { id: undefined }).id !== - (nextProps.record || { id: undefined }).id - ) { - this.fetchReferenceAndOptions(nextProps); - } else if (this.props.input.value !== nextProps.input.value) { - this.fetchReference(nextProps); - } else if ( - !isEqual(nextProps.filter, this.props.filter) || - !isEqual(nextProps.sort, this.props.sort) || - nextProps.perPage !== this.props.perPage - ) { - this.setState( - state => ({ - filter: nextProps.filter, - pagination: { - ...state.pagination, - perPage: nextProps.perPage, - }, - sort: nextProps.sort, - }), - this.fetchOptions - ); - } - } +export const UnconnectedReferenceInputController: FunctionComponent< + Props & EnhancedProps +> = ({ + input, + referenceRecord, + matchingReferences, + onChange, + children, + translate, + perPage, + filter: initialFilter, + crudGetManyAccumulate, + reference, + crudGetMatchingAccumulate, + filterToQuery, + referenceSource, + resource, + source, +}) => { + const dataStatus = getDataStatus({ + input, + matchingReferences, + referenceRecord, + translate, + }); - setFilter = (filter: any) => { - if (filter !== this.state.filter) { - this.setState( - { filter: this.props.filterToQuery(filter) }, - this.fetchOptions - ); - } - }; + const { pagination, setPagination } = usePaginationState(perPage); + const { sort, setSort } = useSortState(); + const { filter, setFilter } = useFilterState({ + initialFilter, + filterToQuery, + }); - setPagination = (pagination: Pagination) => { - if (pagination !== this.state.pagination) { - this.setState({ pagination }, this.fetchOptions); - } - }; - - setSort = (sort: Sort) => { - if (sort !== this.state.sort) { - this.setState({ sort }, this.fetchOptions); - } - }; - - fetchReference = (props = this.props) => { - const { crudGetManyAccumulate, input, reference } = props; - const id = input.value; - if (id) { - crudGetManyAccumulate(reference, [id]); - } - }; + useEffect( + () => + fetchReference({ + crudGetManyAccumulate, + id: input.value, + reference, + }), + [input.value, reference] + ); - fetchOptions = (props = this.props) => { - const { - crudGetMatchingAccumulate, - filter: filterFromProps, + useEffect( + () => + fetchOptions({ + crudGetMatchingAccumulate, + filter, + reference, + referenceSource, + resource, + source, + pagination, + sort, + }), + [ + filter, reference, referenceSource, resource, source, - } = props; - const { pagination, sort, filter } = this.state; + pagination.page, + pagination.perPage, + sort.field, + sort.order, + ] + ); - crudGetMatchingAccumulate( - reference, - referenceSource(resource, source), - pagination, - sort, - { - ...filterFromProps, - ...filter, - } - ); - }; + return children({ + choices: dataStatus.choices, + error: dataStatus.error, + isLoading: dataStatus.waiting, + onChange, + filter, + setFilter, + pagination, + setPagination, + sort, + setSort, + warning: dataStatus.warning, + }) as ReactElement; +}; - fetchReferenceAndOptions(props) { - this.fetchReference(props); - this.fetchOptions(props); +const fetchReference = ({ crudGetManyAccumulate, id, reference }) => { + if (id) { + crudGetManyAccumulate(reference, [id]); } +}; - render() { - const { - input, - referenceRecord, - matchingReferences, - onChange, - children, - translate, - } = this.props; - const { pagination, sort, filter } = this.state; - - const dataStatus = getDataStatus({ - input, - matchingReferences, - referenceRecord, - translate, - }); - - return children({ - choices: dataStatus.choices, - error: dataStatus.error, - isLoading: dataStatus.waiting, - onChange, - filter, - setFilter: this.debouncedSetFilter, - pagination, - setPagination: this.setPagination, - sort, - setSort: this.setSort, - warning: dataStatus.warning, - }); - } -} +const fetchOptions = ({ + crudGetMatchingAccumulate, + filter, + reference, + referenceSource, + resource, + source, + pagination, + sort, +}) => { + crudGetMatchingAccumulate( + reference, + referenceSource(resource, source), + pagination, + sort, + filter + ); +}; const makeMapStateToProps = () => createSelector( diff --git a/packages/ra-core/src/controller/usePaginationState.ts b/packages/ra-core/src/controller/usePaginationState.ts index 588c6057baa..53e80df70af 100644 --- a/packages/ra-core/src/controller/usePaginationState.ts +++ b/packages/ra-core/src/controller/usePaginationState.ts @@ -1,10 +1,13 @@ import { useState, useEffect } from 'react'; +import { Pagination } from '../types'; interface PaginationProps { page: number; perPage: number; + pagination: Pagination; setPage: (page: number) => void; setPerPage: (perPage: number) => void; + setPagination: (pagination: Pagination) => void; } /** @@ -36,12 +39,23 @@ interface PaginationProps { export default (initialPerPage: number = 25): PaginationProps => { const [page, setPage] = useState(1); const [perPage, setPerPage] = useState(initialPerPage); + + const setPagination = (pagination: Pagination) => { + setPage(pagination.page); + setPerPage(pagination.perPage); + }; + useEffect(() => setPerPage(initialPerPage), [initialPerPage]); return { page, perPage, + pagination: { + page, + perPage, + }, setPage, setPerPage, + setPagination, }; }; diff --git a/packages/ra-core/src/controller/useSortState.ts b/packages/ra-core/src/controller/useSortState.ts index c1b59363521..3b634b5a667 100644 --- a/packages/ra-core/src/controller/useSortState.ts +++ b/packages/ra-core/src/controller/useSortState.ts @@ -7,7 +7,8 @@ import { import { Sort } from '../types'; interface SortProps { - setSort: (field: string) => void; + setSortField: (field: string) => void; + setSort: (sort: Sort) => void; sort: Sort; } @@ -64,7 +65,8 @@ export default ( }, [initialSort.field, initialSort.order]); // eslint-disable-line react-hooks/exhaustive-deps return { - setSort: (field: string) => dispatch(field), + setSort: (sortValue: Sort) => dispatch(sortValue), + setSortField: (field: string) => dispatch(field), sort, }; }; From 4b3f74845cb7f58240c08c97b042799019bd5633 Mon Sep 17 00:00:00 2001 From: thiery Date: Mon, 10 Jun 2019 23:04:04 +0200 Subject: [PATCH 02/48] replace hoc by hook --- .../input/ReferenceInputController.tsx | 121 +++++++++--------- 1 file changed, 57 insertions(+), 64 deletions(-) diff --git a/packages/ra-core/src/controller/input/ReferenceInputController.tsx b/packages/ra-core/src/controller/input/ReferenceInputController.tsx index 32930f18934..07337588999 100644 --- a/packages/ra-core/src/controller/input/ReferenceInputController.tsx +++ b/packages/ra-core/src/controller/input/ReferenceInputController.tsx @@ -7,16 +7,16 @@ import { useEffect, useRef, } from 'react'; -import { connect } from 'react-redux'; +// @ts-ignore +import { useSelector, useDispatch } from 'react-redux'; import debounce from 'lodash/debounce'; -import compose from 'recompose/compose'; import { createSelector } from 'reselect'; import { WrappedFieldInputProps } from 'redux-form'; import isEqual from 'lodash/isEqual'; import { - crudGetManyAccumulate as crudGetManyAccumulateAction, - crudGetMatchingAccumulate as crudGetMatchingAccumulateAction, + crudGetManyAccumulate, + crudGetMatchingAccumulate, } from '../../actions/accumulateActions'; import { getPossibleReferences, @@ -24,9 +24,8 @@ import { getReferenceResource, } from '../../reducer'; import { getStatusForInput as getDataStatus } from './referenceDataStatus'; -import withTranslate from '../../i18n/translate'; -import { Sort, Translate, Record, Pagination, Dispatch } from '../../types'; -import { MatchingReferencesError } from './types'; +import useTranslate from '../../i18n/useTranslate'; +import { Sort, Record, Pagination, Dispatch } from '../../types'; import usePaginationState from '../usePaginationState'; import useSortState from '../useSortState'; @@ -61,15 +60,7 @@ interface Props { resource: string; sort?: Sort; source: string; -} - -interface EnhancedProps { - crudGetMatchingAccumulate: Dispatch; - crudGetManyAccumulate: Dispatch; - matchingReferences?: Record[] | MatchingReferencesError; onChange: () => void; - referenceRecord?: Record; - translate: Translate; } const usePrevious = value => { @@ -192,25 +183,36 @@ const useFilterState = ({ * * */ -export const UnconnectedReferenceInputController: FunctionComponent< - Props & EnhancedProps -> = ({ +export const ReferenceInputController: FunctionComponent = ({ input, - referenceRecord, - matchingReferences, onChange, children, - translate, perPage, filter: initialFilter, - crudGetManyAccumulate, reference, - crudGetMatchingAccumulate, filterToQuery, - referenceSource, + referenceSource = defaultReferenceSource, resource, source, }) => { + const translate = useTranslate(); + const dispatch = useDispatch(); + const matchingReferences = useSelector( + getMatchingReferences({ + referenceSource, + input, + reference, + resource, + source, + }), + [input.value, referenceSource, reference, source, resource] + ); + + const referenceRecord = useSelector( + getSelectedeference({ input, reference }), + [input.value, reference] + ); + const dataStatus = getDataStatus({ input, matchingReferences, @@ -228,7 +230,7 @@ export const UnconnectedReferenceInputController: FunctionComponent< useEffect( () => fetchReference({ - crudGetManyAccumulate, + dispatch, id: input.value, reference, }), @@ -238,7 +240,7 @@ export const UnconnectedReferenceInputController: FunctionComponent< useEffect( () => fetchOptions({ - crudGetMatchingAccumulate, + dispatch, filter, reference, referenceSource, @@ -275,14 +277,14 @@ export const UnconnectedReferenceInputController: FunctionComponent< }) as ReactElement; }; -const fetchReference = ({ crudGetManyAccumulate, id, reference }) => { +const fetchReference = ({ dispatch, id, reference }) => { if (id) { - crudGetManyAccumulate(reference, [id]); + dispatch(crudGetManyAccumulate(reference, [id])); } }; const fetchOptions = ({ - crudGetMatchingAccumulate, + dispatch, filter, reference, referenceSource, @@ -291,45 +293,36 @@ const fetchOptions = ({ pagination, sort, }) => { - crudGetMatchingAccumulate( - reference, - referenceSource(resource, source), - pagination, - sort, - filter + dispatch( + crudGetMatchingAccumulate( + reference, + referenceSource(resource, source), + pagination, + sort, + filter + ) ); }; -const makeMapStateToProps = () => - createSelector( - [ - getReferenceResource, - getPossibleReferenceValues, - (_, props) => props.input.value, - ], - (referenceState, possibleValues, inputId) => ({ - matchingReferences: getPossibleReferences( - referenceState, - possibleValues, - [inputId] - ), - referenceRecord: referenceState && referenceState.data[inputId], - }) - ); +const matchingReferencesSelector = createSelector( + [ + getReferenceResource, + getPossibleReferenceValues, + (_, props) => props.input.value, + ], + (referenceState, possibleValues, inputId) => + getPossibleReferences(referenceState, possibleValues, [inputId]) +); -const ReferenceInputController = compose( - withTranslate, - connect( - makeMapStateToProps(), - { - crudGetManyAccumulate: crudGetManyAccumulateAction, - crudGetMatchingAccumulate: crudGetMatchingAccumulateAction, - } - ) -)(UnconnectedReferenceInputController); +const getMatchingReferences = props => state => + matchingReferencesSelector(state, props); -ReferenceInputController.defaultProps = { - referenceSource: defaultReferenceSource, // used in makeMapStateToProps -}; +const selectedReferenceSelector = createSelector( + [getReferenceResource, (_, props) => props.input.value], + (referenceState, inputId) => referenceState && referenceState.data[inputId] +); + +const getSelectedeference = props => state => + selectedReferenceSelector(state, props); export default ReferenceInputController as ComponentType; From a9acc8a0c179d43e8162eb0fa18fa413bfb5236f Mon Sep 17 00:00:00 2001 From: thiery Date: Thu, 13 Jun 2019 10:54:01 +0200 Subject: [PATCH 03/48] extract useFilterState in its own file --- .../input/ReferenceInputController.tsx | 56 ++----------------- .../ra-core/src/controller/useFilterState.ts | 39 +++++++++++++ 2 files changed, 45 insertions(+), 50 deletions(-) create mode 100644 packages/ra-core/src/controller/useFilterState.ts diff --git a/packages/ra-core/src/controller/input/ReferenceInputController.tsx b/packages/ra-core/src/controller/input/ReferenceInputController.tsx index 07337588999..7a870972324 100644 --- a/packages/ra-core/src/controller/input/ReferenceInputController.tsx +++ b/packages/ra-core/src/controller/input/ReferenceInputController.tsx @@ -2,17 +2,13 @@ import { ReactNode, ComponentType, FunctionComponent, - useState, ReactElement, useEffect, - useRef, } from 'react'; // @ts-ignore import { useSelector, useDispatch } from 'react-redux'; -import debounce from 'lodash/debounce'; import { createSelector } from 'reselect'; import { WrappedFieldInputProps } from 'redux-form'; -import isEqual from 'lodash/isEqual'; import { crudGetManyAccumulate, @@ -25,9 +21,10 @@ import { } from '../../reducer'; import { getStatusForInput as getDataStatus } from './referenceDataStatus'; import useTranslate from '../../i18n/useTranslate'; -import { Sort, Record, Pagination, Dispatch } from '../../types'; +import { Sort, Record, Pagination } from '../../types'; import usePaginationState from '../usePaginationState'; import useSortState from '../useSortState'; +import useFilterState, { Filter } from '../useFilterState'; const defaultReferenceSource = (resource: string, source: string) => `${resource}@${source}`; @@ -39,7 +36,7 @@ interface ChildrenFuncParams { isLoading: boolean; onChange: (value: any) => void; pagination: Pagination; - setFilter: (filter: any) => void; + setFilter: (filter: string) => void; setPagination: (pagination: Pagination) => void; setSort: (sort: Sort) => void; sort: Sort; @@ -50,8 +47,8 @@ interface Props { allowEmpty?: boolean; basePath: string; children: (params: ChildrenFuncParams) => ReactNode; - filter?: object; - filterToQuery: (filter: {}) => any; + filter?: Filter; + filterToQuery: (filter: string) => any; input?: WrappedFieldInputProps; perPage: number; record?: Record; @@ -63,47 +60,6 @@ interface Props { onChange: () => void; } -const usePrevious = value => { - const ref = useRef(); - - useEffect(() => { - ref.current = value; - }, [value]); - - return ref.current; -}; - -const useFilterState = ({ - filterToQuery = v => v, - initialFilter, - debounceTime = 500, -}) => { - const previousInitialFilter = usePrevious(initialFilter); - const [filter, setFilterValue] = useState(filterToQuery(initialFilter)); - - const setFilter = debounce( - value => setFilterValue(filterToQuery(value)), - debounceTime - ); - const isFirstRender = useRef(true); - - useEffect(() => { - if (isFirstRender.current) { - isFirstRender.current = false; - return; - } - if (isEqual(initialFilter, previousInitialFilter)) { - return; - } - setFilter(initialFilter); - }, [initialFilter]); - - return { - filter, - setFilter, - }; -}; - /** * An Input component for choosing a reference record. Useful for foreign keys. * @@ -267,8 +223,8 @@ export const ReferenceInputController: FunctionComponent = ({ error: dataStatus.error, isLoading: dataStatus.waiting, onChange, - filter, setFilter, + filter, pagination, setPagination, sort, diff --git a/packages/ra-core/src/controller/useFilterState.ts b/packages/ra-core/src/controller/useFilterState.ts new file mode 100644 index 00000000000..f814aed33fe --- /dev/null +++ b/packages/ra-core/src/controller/useFilterState.ts @@ -0,0 +1,39 @@ +import { useState } from 'react'; +import debounce from 'lodash/debounce'; + +export interface Filter { + [k: string]: any; +} + +interface UseFilterStateOptions { + filterToQuery: (v: string) => Filter; + initialFilter: Filter; + debounceTime?: number; +} + +interface UseFilterStateProps { + filter: Filter; + setFilter: (v: string) => void; +} + +export default ({ + filterToQuery = v => ({ q: v }), + initialFilter, + debounceTime = 500, +}: UseFilterStateOptions): UseFilterStateProps => { + const [filter, setFilterValue] = useState(initialFilter); + + const setFilter = debounce( + value => + setFilterValue({ + ...initialFilter, + ...filterToQuery(value), + }), + debounceTime + ); + + return { + filter, + setFilter, + }; +}; From 1caedcaee70ee5a5a2cdc519b61218b976cd7053 Mon Sep 17 00:00:00 2001 From: thiery Date: Thu, 13 Jun 2019 14:31:31 +0200 Subject: [PATCH 04/48] rename useReference to useReferenceField and add more generic useReference --- .../field/ReferenceFieldController.tsx | 8 +- .../ra-core/src/controller/field/index.ts | 4 +- .../{useReference.ts => useReferenceField.ts} | 35 +++------ .../input/ReferenceInputController.tsx | 39 ++-------- .../ra-core/src/controller/useReference.ts | 75 +++++++++++++++++++ .../src/field/ReferenceField.js | 6 +- 6 files changed, 101 insertions(+), 66 deletions(-) rename packages/ra-core/src/controller/field/{useReference.ts => useReferenceField.ts} (77%) create mode 100644 packages/ra-core/src/controller/useReference.ts diff --git a/packages/ra-core/src/controller/field/ReferenceFieldController.tsx b/packages/ra-core/src/controller/field/ReferenceFieldController.tsx index f6172e714e1..2a67fccefc6 100644 --- a/packages/ra-core/src/controller/field/ReferenceFieldController.tsx +++ b/packages/ra-core/src/controller/field/ReferenceFieldController.tsx @@ -1,9 +1,7 @@ import { FunctionComponent, ReactNode, ReactElement } from 'react'; import { Record } from '../../types'; -import useReference, { - UseReferenceProps, - LinkToFunctionType, -} from './useReference'; + +import useReferenceField, { UseReferenceProps, LinkToFunctionType } from './useReferenceField'; interface Props { allowEmpty?: boolean; @@ -49,7 +47,7 @@ export const ReferenceFieldController: FunctionComponent = ({ children, ...props }) => { - return children(useReference(props)) as ReactElement; + return children(useReferenceField(props)) as ReactElement; }; export default ReferenceFieldController; diff --git a/packages/ra-core/src/controller/field/index.ts b/packages/ra-core/src/controller/field/index.ts index 82efc593192..05ba1ad6aa2 100644 --- a/packages/ra-core/src/controller/field/index.ts +++ b/packages/ra-core/src/controller/field/index.ts @@ -1,7 +1,7 @@ import ReferenceArrayFieldController from './ReferenceArrayFieldController'; import ReferenceFieldController from './ReferenceFieldController'; import ReferenceManyFieldController from './ReferenceManyFieldController'; -import useReference from './useReference'; +import useReferenceField from './useReferenceField'; import useReferenceArray from './useReferenceArray'; import useReferenceMany from './useReferenceMany'; @@ -9,7 +9,7 @@ export { useReferenceArray, ReferenceArrayFieldController, ReferenceFieldController, - useReference, + useReferenceField, useReferenceMany, ReferenceManyFieldController, }; diff --git a/packages/ra-core/src/controller/field/useReference.ts b/packages/ra-core/src/controller/field/useReferenceField.ts similarity index 77% rename from packages/ra-core/src/controller/field/useReference.ts rename to packages/ra-core/src/controller/field/useReferenceField.ts index 6b0ff2763ff..0585cd11b40 100644 --- a/packages/ra-core/src/controller/field/useReference.ts +++ b/packages/ra-core/src/controller/field/useReferenceField.ts @@ -1,11 +1,8 @@ -import { useEffect } from 'react'; -// @ts-ignore -import { useDispatch, useSelector } from 'react-redux'; import get from 'lodash/get'; -import { crudGetManyAccumulate } from '../../actions'; import { linkToRecord } from '../../util'; -import { Record, ReduxState } from '../../types'; +import { Record } from '../../types'; +import useReference from '../useReference'; export type LinkToFunctionType = (record: Record, reference: string) => string; @@ -15,9 +12,9 @@ interface Option { allowEmpty?: boolean; basePath: string; record?: Record; + source: string; reference: string; resource: string; - source: string; link: LinkToType; linkType?: LinkToType; // deprecated, use link instead } @@ -64,26 +61,22 @@ export interface UseReferenceProps { * * @returns {ReferenceProps} The reference props */ -export const useReference = ({ +export const useReferenceField = ({ allowEmpty = false, basePath, link = 'edit', linkType, - record = { id: '' }, reference, + record = { id: '' }, resource, source, }: Option): UseReferenceProps => { const sourceId = get(record, source); - const referenceRecord = useSelector( - getReferenceRecord(sourceId, reference) - ); - const dispatch = useDispatch(); - useEffect(() => { - if (sourceId !== null && typeof sourceId !== 'undefined') { - dispatch(crudGetManyAccumulate(reference, [sourceId])); - } - }, [sourceId, reference]); // eslint-disable-line react-hooks/exhaustive-deps + const { referenceRecord, isLoading } = useReference({ + id: sourceId, + reference, + allowEmpty, + }); const rootPath = basePath.replace(resource, reference); // Backward compatibility: keep linkType but with warning const getResourceLinkPath = (linkTo: LinkToType) => @@ -104,14 +97,10 @@ export const useReference = ({ ); return { - isLoading: !referenceRecord && !allowEmpty, + isLoading, referenceRecord, resourceLinkPath, }; }; -const getReferenceRecord = (sourceId, reference) => (state: ReduxState) => - state.admin.resources[reference] && - state.admin.resources[reference].data[sourceId]; - -export default useReference; +export default useReferenceField; diff --git a/packages/ra-core/src/controller/input/ReferenceInputController.tsx b/packages/ra-core/src/controller/input/ReferenceInputController.tsx index 7a870972324..3142921ecc9 100644 --- a/packages/ra-core/src/controller/input/ReferenceInputController.tsx +++ b/packages/ra-core/src/controller/input/ReferenceInputController.tsx @@ -10,10 +10,7 @@ import { useSelector, useDispatch } from 'react-redux'; import { createSelector } from 'reselect'; import { WrappedFieldInputProps } from 'redux-form'; -import { - crudGetManyAccumulate, - crudGetMatchingAccumulate, -} from '../../actions/accumulateActions'; +import { crudGetMatchingAccumulate } from '../../actions/accumulateActions'; import { getPossibleReferences, getPossibleReferenceValues, @@ -25,6 +22,7 @@ import { Sort, Record, Pagination } from '../../types'; import usePaginationState from '../usePaginationState'; import useSortState from '../useSortState'; import useFilterState, { Filter } from '../useFilterState'; +import useReference from '../useReference'; const defaultReferenceSource = (resource: string, source: string) => `${resource}@${source}`; @@ -164,10 +162,11 @@ export const ReferenceInputController: FunctionComponent = ({ [input.value, referenceSource, reference, source, resource] ); - const referenceRecord = useSelector( - getSelectedeference({ input, reference }), - [input.value, reference] - ); + const { referenceRecord } = useReference({ + id: input.value, + reference, + allowEmpty: true, + }); const dataStatus = getDataStatus({ input, @@ -183,16 +182,6 @@ export const ReferenceInputController: FunctionComponent = ({ filterToQuery, }); - useEffect( - () => - fetchReference({ - dispatch, - id: input.value, - reference, - }), - [input.value, reference] - ); - useEffect( () => fetchOptions({ @@ -233,12 +222,6 @@ export const ReferenceInputController: FunctionComponent = ({ }) as ReactElement; }; -const fetchReference = ({ dispatch, id, reference }) => { - if (id) { - dispatch(crudGetManyAccumulate(reference, [id])); - } -}; - const fetchOptions = ({ dispatch, filter, @@ -273,12 +256,4 @@ const matchingReferencesSelector = createSelector( const getMatchingReferences = props => state => matchingReferencesSelector(state, props); -const selectedReferenceSelector = createSelector( - [getReferenceResource, (_, props) => props.input.value], - (referenceState, inputId) => referenceState && referenceState.data[inputId] -); - -const getSelectedeference = props => state => - selectedReferenceSelector(state, props); - export default ReferenceInputController as ComponentType; diff --git a/packages/ra-core/src/controller/useReference.ts b/packages/ra-core/src/controller/useReference.ts new file mode 100644 index 00000000000..c7cc36c23da --- /dev/null +++ b/packages/ra-core/src/controller/useReference.ts @@ -0,0 +1,75 @@ +import { useEffect } from 'react'; +// @ts-ignore +import { useDispatch, useSelector } from 'react-redux'; + +import { crudGetManyAccumulate } from '../actions'; +import { Record } from '../types'; +import { getReferenceResource } from '../reducer'; +import { createSelector } from 'reselect'; + +interface Option { + id: string; + reference: string; + allowEmpty?: boolean; +} + +export interface UseReferenceProps { + isLoading: boolean; + referenceRecord: Record; +} + +/** + * @typedef ReferenceProps + * @type {Object} + * @property {boolean} isLoading: boolean indicating if the reference has loaded + * @property {Object} referenceRecord: the referenced record. + */ + +/** + * Fetch reference record, and return it when avaliable + * + * The reference prop sould be the name of one of the components + * added as child. + * + * @example + * + * const { isLoading, referenceRecord } = useReference({ + * id: 7, + * reference: 'users', + * }); + * + * @param {Object} option + * @param {boolean} option.allowEmpty do we allow for no referenced record (default to false) + * @param {string} option.reference The linked resource name + * @param {string} option.id The id of the reference + * + * @returns {ReferenceProps} The reference record + */ +export const useReference = ({ + allowEmpty = false, + reference, + id, +}: Option): UseReferenceProps => { + const referenceRecord = useSelector(getReferenceRecord({ id, reference })); + const dispatch = useDispatch(); + useEffect(() => { + if (id !== null && typeof id !== 'undefined') { + dispatch(crudGetManyAccumulate(reference, [id])); + } + }, [id, reference]); + + return { + isLoading: !referenceRecord && !allowEmpty, + referenceRecord, + }; +}; + +const selectedReferenceSelector = createSelector( + [getReferenceResource, (_, props) => props.id], + (referenceState, id) => referenceState && referenceState.data[id] +); + +const getReferenceRecord = props => state => + selectedReferenceSelector(state, props); + +export default useReference; diff --git a/packages/ra-ui-materialui/src/field/ReferenceField.js b/packages/ra-ui-materialui/src/field/ReferenceField.js index ac291146c4d..a1be5da0d66 100644 --- a/packages/ra-ui-materialui/src/field/ReferenceField.js +++ b/packages/ra-ui-materialui/src/field/ReferenceField.js @@ -2,7 +2,7 @@ import React, { Children } from 'react'; import PropTypes from 'prop-types'; import classnames from 'classnames'; import { withStyles, createStyles } from '@material-ui/core/styles'; -import { useReference } from 'ra-core'; +import { useReferenceField } from 'ra-core'; import LinearProgress from '../layout/LinearProgress'; import Link from '../Link'; @@ -137,9 +137,7 @@ const ReferenceField = ({ children, ...props }) => { throw new Error(' only accepts a single child'); } - const { isLoading, referenceRecord, resourceLinkPath } = useReference( - props - ); + const { isLoading, referenceRecord, resourceLinkPath } = useReferenceField(props); return ( Date: Thu, 13 Jun 2019 15:53:59 +0200 Subject: [PATCH 05/48] add useReferenceSearch hook --- .../input/ReferenceInputController.tsx | 114 +++------------- .../controller/input/useReferenceSearch.ts | 125 ++++++++++++++++++ .../ra-core/src/controller/useFilterState.ts | 8 +- 3 files changed, 151 insertions(+), 96 deletions(-) create mode 100644 packages/ra-core/src/controller/input/useReferenceSearch.ts diff --git a/packages/ra-core/src/controller/input/ReferenceInputController.tsx b/packages/ra-core/src/controller/input/ReferenceInputController.tsx index 3142921ecc9..67d50726c5f 100644 --- a/packages/ra-core/src/controller/input/ReferenceInputController.tsx +++ b/packages/ra-core/src/controller/input/ReferenceInputController.tsx @@ -3,26 +3,14 @@ import { ComponentType, FunctionComponent, ReactElement, - useEffect, } from 'react'; -// @ts-ignore -import { useSelector, useDispatch } from 'react-redux'; -import { createSelector } from 'reselect'; import { WrappedFieldInputProps } from 'redux-form'; -import { crudGetMatchingAccumulate } from '../../actions/accumulateActions'; -import { - getPossibleReferences, - getPossibleReferenceValues, - getReferenceResource, -} from '../../reducer'; import { getStatusForInput as getDataStatus } from './referenceDataStatus'; import useTranslate from '../../i18n/useTranslate'; import { Sort, Record, Pagination } from '../../types'; -import usePaginationState from '../usePaginationState'; -import useSortState from '../useSortState'; -import useFilterState, { Filter } from '../useFilterState'; import useReference from '../useReference'; +import useReferenceSearch from './useReferenceSearch'; const defaultReferenceSource = (resource: string, source: string) => `${resource}@${source}`; @@ -45,7 +33,7 @@ interface Props { allowEmpty?: boolean; basePath: string; children: (params: ChildrenFuncParams) => ReactNode; - filter?: Filter; + filter?: any; filterToQuery: (filter: string) => any; input?: WrappedFieldInputProps; perPage: number; @@ -142,7 +130,7 @@ export const ReferenceInputController: FunctionComponent = ({ onChange, children, perPage, - filter: initialFilter, + filter: permanentFilter, reference, filterToQuery, referenceSource = defaultReferenceSource, @@ -150,17 +138,25 @@ export const ReferenceInputController: FunctionComponent = ({ source, }) => { const translate = useTranslate(); - const dispatch = useDispatch(); - const matchingReferences = useSelector( - getMatchingReferences({ - referenceSource, - input, - reference, - resource, - source, - }), - [input.value, referenceSource, reference, source, resource] - ); + + const { + matchingReferences, + filter, + setFilter, + pagination, + setPagination, + sort, + setSort, + } = useReferenceSearch({ + reference, + referenceSource, + filterToQuery, + filterValue: input.value, + permanentFilter, + perPage, + resource, + source, + }); const { referenceRecord } = useReference({ id: input.value, @@ -175,38 +171,6 @@ export const ReferenceInputController: FunctionComponent = ({ translate, }); - const { pagination, setPagination } = usePaginationState(perPage); - const { sort, setSort } = useSortState(); - const { filter, setFilter } = useFilterState({ - initialFilter, - filterToQuery, - }); - - useEffect( - () => - fetchOptions({ - dispatch, - filter, - reference, - referenceSource, - resource, - source, - pagination, - sort, - }), - [ - filter, - reference, - referenceSource, - resource, - source, - pagination.page, - pagination.perPage, - sort.field, - sort.order, - ] - ); - return children({ choices: dataStatus.choices, error: dataStatus.error, @@ -222,38 +186,4 @@ export const ReferenceInputController: FunctionComponent = ({ }) as ReactElement; }; -const fetchOptions = ({ - dispatch, - filter, - reference, - referenceSource, - resource, - source, - pagination, - sort, -}) => { - dispatch( - crudGetMatchingAccumulate( - reference, - referenceSource(resource, source), - pagination, - sort, - filter - ) - ); -}; - -const matchingReferencesSelector = createSelector( - [ - getReferenceResource, - getPossibleReferenceValues, - (_, props) => props.input.value, - ], - (referenceState, possibleValues, inputId) => - getPossibleReferences(referenceState, possibleValues, [inputId]) -); - -const getMatchingReferences = props => state => - matchingReferencesSelector(state, props); - export default ReferenceInputController as ComponentType; diff --git a/packages/ra-core/src/controller/input/useReferenceSearch.ts b/packages/ra-core/src/controller/input/useReferenceSearch.ts new file mode 100644 index 00000000000..e2aae65b2f7 --- /dev/null +++ b/packages/ra-core/src/controller/input/useReferenceSearch.ts @@ -0,0 +1,125 @@ +import { useEffect } from 'react'; +// @ts-ignore +import { useSelector, useDispatch } from 'react-redux'; +import { createSelector } from 'reselect'; + +import usePaginationState from '../usePaginationState'; +import useSortState from '../useSortState'; +import useFilterState, { Filter } from '../useFilterState'; +import { crudGetMatchingAccumulate } from '../../actions/accumulateActions'; +import { + getPossibleReferences, + getPossibleReferenceValues, + getReferenceResource, +} from '../../reducer'; + +interface UseReferenceSearchOption { + reference: string; + referenceSource: (resource: string, source: string) => string; + resource: string; + source: string; + permanentFilter: Filter; + filterToQuery: (v: string) => Filter; + perPage: number; + filterValue: string; +} + +export default ({ + reference, + referenceSource, + resource, + source, + permanentFilter, + filterToQuery, + perPage, + filterValue, +}: UseReferenceSearchOption) => { + const dispatch = useDispatch(); + + const { pagination, setPagination } = usePaginationState(perPage); + const { sort, setSort } = useSortState(); + const { filter, setFilter } = useFilterState({ + permanentFilter, + filterToQuery, + }); + + useEffect( + () => + fetchOptions({ + dispatch, + filter, + reference, + referenceSource, + resource, + source, + pagination, + sort, + }), + [ + filter, + reference, + referenceSource, + resource, + source, + pagination.page, + pagination.perPage, + sort.field, + sort.order, + ] + ); + + const matchingReferences = useSelector( + getMatchingReferences({ + referenceSource, + filterValue, + reference, + resource, + source, + }), + [filterValue, referenceSource, reference, source, resource] + ); + + return { + matchingReferences, + setFilter, + filter, + pagination, + setPagination, + sort, + setSort, + }; +}; + +const fetchOptions = ({ + dispatch, + filter, + reference, + referenceSource, + resource, + source, + pagination, + sort, +}) => { + dispatch( + crudGetMatchingAccumulate( + reference, + referenceSource(resource, source), + pagination, + sort, + filter + ) + ); +}; + +const matchingReferencesSelector = createSelector( + [ + getReferenceResource, + getPossibleReferenceValues, + (_, props) => props.filterValue, + ], + (referenceState, possibleValues, inputId) => + getPossibleReferences(referenceState, possibleValues, [inputId]) +); + +const getMatchingReferences = props => state => + matchingReferencesSelector(state, props); diff --git a/packages/ra-core/src/controller/useFilterState.ts b/packages/ra-core/src/controller/useFilterState.ts index f814aed33fe..984028a020c 100644 --- a/packages/ra-core/src/controller/useFilterState.ts +++ b/packages/ra-core/src/controller/useFilterState.ts @@ -7,7 +7,7 @@ export interface Filter { interface UseFilterStateOptions { filterToQuery: (v: string) => Filter; - initialFilter: Filter; + permanentFilter: Filter; debounceTime?: number; } @@ -18,15 +18,15 @@ interface UseFilterStateProps { export default ({ filterToQuery = v => ({ q: v }), - initialFilter, + permanentFilter, debounceTime = 500, }: UseFilterStateOptions): UseFilterStateProps => { - const [filter, setFilterValue] = useState(initialFilter); + const [filter, setFilterValue] = useState(permanentFilter); const setFilter = debounce( value => setFilterValue({ - ...initialFilter, + ...permanentFilter, ...filterToQuery(value), }), debounceTime From 8d00d202d61a56ec7a4280ee0987ec61f1a1eaf7 Mon Sep 17 00:00:00 2001 From: thiery Date: Mon, 17 Jun 2019 22:30:59 +0200 Subject: [PATCH 06/48] sipmler useReferenceSearch hook --- .../input/ReferenceInputController.tsx | 33 ++++++------ .../input/useReferenceSearch.spec.tsx | 54 +++++++++++++++++++ .../controller/input/useReferenceSearch.ts | 50 +++++++---------- .../admin/references/possibleValues.ts | 5 +- .../ra-core/src/util/renderHookWithRedux.tsx | 19 +++++++ 5 files changed, 111 insertions(+), 50 deletions(-) create mode 100644 packages/ra-core/src/controller/input/useReferenceSearch.spec.tsx create mode 100644 packages/ra-core/src/util/renderHookWithRedux.tsx diff --git a/packages/ra-core/src/controller/input/ReferenceInputController.tsx b/packages/ra-core/src/controller/input/ReferenceInputController.tsx index 67d50726c5f..237e4ecbb30 100644 --- a/packages/ra-core/src/controller/input/ReferenceInputController.tsx +++ b/packages/ra-core/src/controller/input/ReferenceInputController.tsx @@ -11,6 +11,9 @@ import useTranslate from '../../i18n/useTranslate'; import { Sort, Record, Pagination } from '../../types'; import useReference from '../useReference'; import useReferenceSearch from './useReferenceSearch'; +import usePaginationState from '../usePaginationState'; +import { useSortState } from '..'; +import useFilterState from '../useFilterState'; const defaultReferenceSource = (resource: string, source: string) => `${resource}@${source}`; @@ -34,12 +37,12 @@ interface Props { basePath: string; children: (params: ChildrenFuncParams) => ReactNode; filter?: any; - filterToQuery: (filter: string) => any; + filterToQuery?: (filter: string) => any; input?: WrappedFieldInputProps; - perPage: number; + perPage?: number; record?: Record; reference: string; - referenceSource: typeof defaultReferenceSource; + referenceSource?: typeof defaultReferenceSource; resource: string; sort?: Sort; source: string; @@ -129,7 +132,7 @@ export const ReferenceInputController: FunctionComponent = ({ input, onChange, children, - perPage, + perPage = 25, filter: permanentFilter, reference, filterToQuery, @@ -139,21 +142,19 @@ export const ReferenceInputController: FunctionComponent = ({ }) => { const translate = useTranslate(); - const { - matchingReferences, + const { pagination, setPagination } = usePaginationState(perPage); + const { sort, setSort } = useSortState(); + const { filter, setFilter } = useFilterState({ + permanentFilter, + filterToQuery, + }); + + const { matchingReferences } = useReferenceSearch({ + reference, + referenceSource, filter, - setFilter, pagination, - setPagination, sort, - setSort, - } = useReferenceSearch({ - reference, - referenceSource, - filterToQuery, - filterValue: input.value, - permanentFilter, - perPage, resource, source, }); diff --git a/packages/ra-core/src/controller/input/useReferenceSearch.spec.tsx b/packages/ra-core/src/controller/input/useReferenceSearch.spec.tsx new file mode 100644 index 00000000000..e650f2088d4 --- /dev/null +++ b/packages/ra-core/src/controller/input/useReferenceSearch.spec.tsx @@ -0,0 +1,54 @@ +import renderHookWithRedux from '../../util/renderHookWithRedux'; +import useReferenceSearch from './useReferenceSearch'; +import { cleanup } from 'react-testing-library'; + +describe('useReferenceSearch', () => { + const defaultProps = { + reference: 'posts', + resource: 'comments', + source: 'post_id', + filter: { q: '' }, + pagination: { + perPage: 25, + page: 1, + }, + sort: { field: 'id', order: 'DESC' }, + referenceSource: undefined, + }; + + afterEach(cleanup); + + it('should fetch matchingReferences on mount', () => { + const { dispatch } = renderHookWithRedux(() => { + return { matchingReferences: useReferenceSearch(defaultProps) }; + }); + + expect(dispatch).toBeCalledTimes(1); + expect(dispatch.mock.calls[0][0].type).toBe( + 'RA/CRUD_GET_MATCHING_ACCUMULATE' + ); + }); + + it('should pass matching references from redux state to its children', () => { + const { childrenProps } = renderHookWithRedux( + () => { + return { matchingReferences: useReferenceSearch(defaultProps) }; + }, + { + admin: { + resources: { + posts: { data: { 1: { id: 1 }, 2: { id: 2 } } }, + }, + references: { + possibleValues: { 'comments@post_id': [2, 1] }, + }, + }, + } + ); + + expect(childrenProps.matchingReferences).toEqual([ + { id: 2 }, + { id: 1 }, + ]); + }); +}); diff --git a/packages/ra-core/src/controller/input/useReferenceSearch.ts b/packages/ra-core/src/controller/input/useReferenceSearch.ts index e2aae65b2f7..75369967ce6 100644 --- a/packages/ra-core/src/controller/input/useReferenceSearch.ts +++ b/packages/ra-core/src/controller/input/useReferenceSearch.ts @@ -3,46 +3,39 @@ import { useEffect } from 'react'; import { useSelector, useDispatch } from 'react-redux'; import { createSelector } from 'reselect'; -import usePaginationState from '../usePaginationState'; -import useSortState from '../useSortState'; -import useFilterState, { Filter } from '../useFilterState'; +import { Filter } from '../useFilterState'; import { crudGetMatchingAccumulate } from '../../actions/accumulateActions'; import { getPossibleReferences, getPossibleReferenceValues, getReferenceResource, } from '../../reducer'; +import { Pagination, Sort } from '../../types'; interface UseReferenceSearchOption { reference: string; referenceSource: (resource: string, source: string) => string; resource: string; source: string; - permanentFilter: Filter; - filterToQuery: (v: string) => Filter; - perPage: number; - filterValue: string; + filter: Filter; + pagination: Pagination; + sort: Sort; } +const defaultReferenceSource = (resource: string, source: string) => + `${resource}@${source}`; + export default ({ reference, - referenceSource, + referenceSource = defaultReferenceSource, resource, source, - permanentFilter, - filterToQuery, - perPage, - filterValue, + filter, + pagination, + sort, }: UseReferenceSearchOption) => { const dispatch = useDispatch(); - const { pagination, setPagination } = usePaginationState(perPage); - const { sort, setSort } = useSortState(); - const { filter, setFilter } = useFilterState({ - permanentFilter, - filterToQuery, - }); - useEffect( () => fetchOptions({ @@ -71,23 +64,15 @@ export default ({ const matchingReferences = useSelector( getMatchingReferences({ referenceSource, - filterValue, + filter, reference, resource, source, }), - [filterValue, referenceSource, reference, source, resource] + [filter, referenceSource, reference, source, resource] ); - return { - matchingReferences, - setFilter, - filter, - pagination, - setPagination, - sort, - setSort, - }; + return matchingReferences; }; const fetchOptions = ({ @@ -117,8 +102,9 @@ const matchingReferencesSelector = createSelector( getPossibleReferenceValues, (_, props) => props.filterValue, ], - (referenceState, possibleValues, inputId) => - getPossibleReferences(referenceState, possibleValues, [inputId]) + (referenceState, possibleValues, inputId) => { + return getPossibleReferences(referenceState, possibleValues, [inputId]); + } ); const getMatchingReferences = props => state => diff --git a/packages/ra-core/src/reducer/admin/references/possibleValues.ts b/packages/ra-core/src/reducer/admin/references/possibleValues.ts index ed518772ab2..d0337006613 100644 --- a/packages/ra-core/src/reducer/admin/references/possibleValues.ts +++ b/packages/ra-core/src/reducer/admin/references/possibleValues.ts @@ -40,8 +40,9 @@ const possibleValuesreducer: Reducer = ( } }; -export const getPossibleReferenceValues = (state, props) => - state[props.referenceSource(props.resource, props.source)]; +export const getPossibleReferenceValues = (state, props) => { + return state[props.referenceSource(props.resource, props.source)]; +}; export const getPossibleReferences = ( referenceState, diff --git a/packages/ra-core/src/util/renderHookWithRedux.tsx b/packages/ra-core/src/util/renderHookWithRedux.tsx new file mode 100644 index 00000000000..9d4008af2c9 --- /dev/null +++ b/packages/ra-core/src/util/renderHookWithRedux.tsx @@ -0,0 +1,19 @@ +import React from 'react'; +import renderWithRedux from './renderWithRedux'; + +const TestHook = ({ children, hookProps }) => { + return children(hookProps()); +}; +export default (hookProps, reduxState?) => { + let childrenProps = null; + const children = props => { + childrenProps = props; + return

child

; + }; + const result = renderWithRedux( + , + reduxState + ); + + return { ...result, childrenProps }; +}; From d60bc81bb4ad46d4399fd51a9ce7eac54ee97266 Mon Sep 17 00:00:00 2001 From: thiery Date: Wed, 19 Jun 2019 10:01:49 +0200 Subject: [PATCH 07/48] rename useReferenceSearch to useMatchingReferences --- .../src/controller/input/ReferenceInputController.tsx | 6 +++--- ...enceSearch.spec.tsx => useMatchingReferences.spec.tsx} | 8 ++++---- .../{useReferenceSearch.ts => useMatchingReferences.ts} | 0 3 files changed, 7 insertions(+), 7 deletions(-) rename packages/ra-core/src/controller/input/{useReferenceSearch.spec.tsx => useMatchingReferences.spec.tsx} (83%) rename packages/ra-core/src/controller/input/{useReferenceSearch.ts => useMatchingReferences.ts} (100%) diff --git a/packages/ra-core/src/controller/input/ReferenceInputController.tsx b/packages/ra-core/src/controller/input/ReferenceInputController.tsx index 237e4ecbb30..41f175a885f 100644 --- a/packages/ra-core/src/controller/input/ReferenceInputController.tsx +++ b/packages/ra-core/src/controller/input/ReferenceInputController.tsx @@ -10,7 +10,7 @@ import { getStatusForInput as getDataStatus } from './referenceDataStatus'; import useTranslate from '../../i18n/useTranslate'; import { Sort, Record, Pagination } from '../../types'; import useReference from '../useReference'; -import useReferenceSearch from './useReferenceSearch'; +import useMatchingReferences from './useMatchingReferences'; import usePaginationState from '../usePaginationState'; import { useSortState } from '..'; import useFilterState from '../useFilterState'; @@ -133,7 +133,7 @@ export const ReferenceInputController: FunctionComponent = ({ onChange, children, perPage = 25, - filter: permanentFilter, + filter: permanentFilter = {}, reference, filterToQuery, referenceSource = defaultReferenceSource, @@ -149,7 +149,7 @@ export const ReferenceInputController: FunctionComponent = ({ filterToQuery, }); - const { matchingReferences } = useReferenceSearch({ + const matchingReferences = useMatchingReferences({ reference, referenceSource, filter, diff --git a/packages/ra-core/src/controller/input/useReferenceSearch.spec.tsx b/packages/ra-core/src/controller/input/useMatchingReferences.spec.tsx similarity index 83% rename from packages/ra-core/src/controller/input/useReferenceSearch.spec.tsx rename to packages/ra-core/src/controller/input/useMatchingReferences.spec.tsx index e650f2088d4..5998b236bd6 100644 --- a/packages/ra-core/src/controller/input/useReferenceSearch.spec.tsx +++ b/packages/ra-core/src/controller/input/useMatchingReferences.spec.tsx @@ -1,8 +1,8 @@ import renderHookWithRedux from '../../util/renderHookWithRedux'; -import useReferenceSearch from './useReferenceSearch'; +import useMatchingReferences from './useMatchingReferences'; import { cleanup } from 'react-testing-library'; -describe('useReferenceSearch', () => { +describe('useMatchingReferences', () => { const defaultProps = { reference: 'posts', resource: 'comments', @@ -20,7 +20,7 @@ describe('useReferenceSearch', () => { it('should fetch matchingReferences on mount', () => { const { dispatch } = renderHookWithRedux(() => { - return { matchingReferences: useReferenceSearch(defaultProps) }; + return { matchingReferences: useMatchingReferences(defaultProps) }; }); expect(dispatch).toBeCalledTimes(1); @@ -32,7 +32,7 @@ describe('useReferenceSearch', () => { it('should pass matching references from redux state to its children', () => { const { childrenProps } = renderHookWithRedux( () => { - return { matchingReferences: useReferenceSearch(defaultProps) }; + return { matchingReferences: useMatchingReferences(defaultProps) }; }, { admin: { diff --git a/packages/ra-core/src/controller/input/useReferenceSearch.ts b/packages/ra-core/src/controller/input/useMatchingReferences.ts similarity index 100% rename from packages/ra-core/src/controller/input/useReferenceSearch.ts rename to packages/ra-core/src/controller/input/useMatchingReferences.ts From cab8d99885a8ea42c71f1c3690fd313fd16ed9a6 Mon Sep 17 00:00:00 2001 From: thiery Date: Wed, 19 Jun 2019 11:19:50 +0200 Subject: [PATCH 08/48] initialize selector on mount for useMatchingReferences and useReference hooks --- .../input/ReferenceInputController.tsx | 2 +- .../input/useMatchingReferences.spec.tsx | 36 +++++++++- .../controller/input/useMatchingReferences.ts | 65 ++++++++++++++----- .../ra-core/src/controller/useReference.ts | 16 +++-- 4 files changed, 91 insertions(+), 28 deletions(-) diff --git a/packages/ra-core/src/controller/input/ReferenceInputController.tsx b/packages/ra-core/src/controller/input/ReferenceInputController.tsx index 41f175a885f..d38c3d9d849 100644 --- a/packages/ra-core/src/controller/input/ReferenceInputController.tsx +++ b/packages/ra-core/src/controller/input/ReferenceInputController.tsx @@ -149,7 +149,7 @@ export const ReferenceInputController: FunctionComponent = ({ filterToQuery, }); - const matchingReferences = useMatchingReferences({ + const { matchingReferences } = useMatchingReferences({ reference, referenceSource, filter, diff --git a/packages/ra-core/src/controller/input/useMatchingReferences.spec.tsx b/packages/ra-core/src/controller/input/useMatchingReferences.spec.tsx index 5998b236bd6..392f301177f 100644 --- a/packages/ra-core/src/controller/input/useMatchingReferences.spec.tsx +++ b/packages/ra-core/src/controller/input/useMatchingReferences.spec.tsx @@ -19,8 +19,8 @@ describe('useMatchingReferences', () => { afterEach(cleanup); it('should fetch matchingReferences on mount', () => { - const { dispatch } = renderHookWithRedux(() => { - return { matchingReferences: useMatchingReferences(defaultProps) }; + const { dispatch, rerender } = renderHookWithRedux(() => { + return useMatchingReferences(defaultProps); }); expect(dispatch).toBeCalledTimes(1); @@ -32,7 +32,7 @@ describe('useMatchingReferences', () => { it('should pass matching references from redux state to its children', () => { const { childrenProps } = renderHookWithRedux( () => { - return { matchingReferences: useMatchingReferences(defaultProps) }; + return useMatchingReferences(defaultProps); }, { admin: { @@ -50,5 +50,35 @@ describe('useMatchingReferences', () => { { id: 2 }, { id: 1 }, ]); + + expect(childrenProps.loading).toBe(false); + expect(childrenProps.error).toBe(null); + }); + + it('should pass an error if an error is in redux state', () => { + const { childrenProps } = renderHookWithRedux( + () => { + return useMatchingReferences(defaultProps); + }, + { + admin: { + resources: { + posts: { data: { 1: { id: 1 }, 2: { id: 2 } } }, + }, + references: { + possibleValues: { + 'comments@post_id': { + error: 'Something bad happened', + }, + }, + }, + }, + } + ); + + expect(childrenProps.matchingReferences).toBe(null); + + expect(childrenProps.loading).toBe(false); + expect(childrenProps.error).toBe('Something bad happened'); }); }); diff --git a/packages/ra-core/src/controller/input/useMatchingReferences.ts b/packages/ra-core/src/controller/input/useMatchingReferences.ts index 75369967ce6..b44b46bc03f 100644 --- a/packages/ra-core/src/controller/input/useMatchingReferences.ts +++ b/packages/ra-core/src/controller/input/useMatchingReferences.ts @@ -1,4 +1,4 @@ -import { useEffect } from 'react'; +import { useEffect, useMemo } from 'react'; // @ts-ignore import { useSelector, useDispatch } from 'react-redux'; import { createSelector } from 'reselect'; @@ -10,9 +10,9 @@ import { getPossibleReferenceValues, getReferenceResource, } from '../../reducer'; -import { Pagination, Sort } from '../../types'; +import { Pagination, Sort, Record } from '../../types'; -interface UseReferenceSearchOption { +interface UseMAtchingReferencesOption { reference: string; referenceSource: (resource: string, source: string) => string; resource: string; @@ -22,6 +22,12 @@ interface UseReferenceSearchOption { sort: Sort; } +interface UseMatchingReferencesProps { + error?: string; + matchingReferences?: Record[]; + loading: boolean; +} + const defaultReferenceSource = (resource: string, source: string) => `${resource}@${source}`; @@ -33,9 +39,11 @@ export default ({ filter, pagination, sort, -}: UseReferenceSearchOption) => { +}: UseMAtchingReferencesOption): UseMatchingReferencesProps => { const dispatch = useDispatch(); + const getMatchingReferences = useMemo(makeMatchingReferencesSelector, []); + useEffect( () => fetchOptions({ @@ -72,7 +80,27 @@ export default ({ [filter, referenceSource, reference, source, resource] ); - return matchingReferences; + if (!matchingReferences) { + return { + loading: true, + error: null, + matchingReferences: null, + }; + } + + if (matchingReferences.error) { + return { + loading: false, + matchingReferences: null, + error: matchingReferences.error, + }; + } + + return { + loading: false, + error: null, + matchingReferences, + }; }; const fetchOptions = ({ @@ -96,16 +124,19 @@ const fetchOptions = ({ ); }; -const matchingReferencesSelector = createSelector( - [ - getReferenceResource, - getPossibleReferenceValues, - (_, props) => props.filterValue, - ], - (referenceState, possibleValues, inputId) => { - return getPossibleReferences(referenceState, possibleValues, [inputId]); - } -); +const makeMatchingReferencesSelector = () => { + const matchingReferencesSelector = createSelector( + [ + getReferenceResource, + getPossibleReferenceValues, + (_, props) => props.filterValue, + ], + (referenceState, possibleValues, inputId) => { + return getPossibleReferences(referenceState, possibleValues, [ + inputId, + ]); + } + ); -const getMatchingReferences = props => state => - matchingReferencesSelector(state, props); + return props => state => matchingReferencesSelector(state, props); +}; diff --git a/packages/ra-core/src/controller/useReference.ts b/packages/ra-core/src/controller/useReference.ts index c7cc36c23da..b74921c0481 100644 --- a/packages/ra-core/src/controller/useReference.ts +++ b/packages/ra-core/src/controller/useReference.ts @@ -1,4 +1,4 @@ -import { useEffect } from 'react'; +import { useEffect, useMemo } from 'react'; // @ts-ignore import { useDispatch, useSelector } from 'react-redux'; @@ -50,6 +50,7 @@ export const useReference = ({ reference, id, }: Option): UseReferenceProps => { + const getReferenceRecord = useMemo(makeSelectedReferenceSelector, []); const referenceRecord = useSelector(getReferenceRecord({ id, reference })); const dispatch = useDispatch(); useEffect(() => { @@ -64,12 +65,13 @@ export const useReference = ({ }; }; -const selectedReferenceSelector = createSelector( - [getReferenceResource, (_, props) => props.id], - (referenceState, id) => referenceState && referenceState.data[id] -); +const makeSelectedReferenceSelector = () => { + const selectedReferenceSelector = createSelector( + [getReferenceResource, (_, props) => props.id], + (referenceState, id) => referenceState && referenceState.data[id] + ); -const getReferenceRecord = props => state => - selectedReferenceSelector(state, props); + return props => state => selectedReferenceSelector(state, props); +}; export default useReference; From d63e24224e771020b1d23c568d0f47e55a3e694a Mon Sep 17 00:00:00 2001 From: thiery Date: Wed, 19 Jun 2019 12:28:45 +0200 Subject: [PATCH 09/48] WIP --- .../input/ReferenceInputController.spec.tsx | 928 +++++++++--------- .../input/useMatchingReferences.spec.tsx | 27 +- .../controller/input/useMatchingReferences.ts | 3 +- .../src/controller/useReference.spec.ts | 64 ++ .../ra-core/src/controller/useReference.ts | 25 +- 5 files changed, 543 insertions(+), 504 deletions(-) create mode 100644 packages/ra-core/src/controller/useReference.spec.ts diff --git a/packages/ra-core/src/controller/input/ReferenceInputController.spec.tsx b/packages/ra-core/src/controller/input/ReferenceInputController.spec.tsx index c5617b602c9..000fec34a84 100644 --- a/packages/ra-core/src/controller/input/ReferenceInputController.spec.tsx +++ b/packages/ra-core/src/controller/input/ReferenceInputController.spec.tsx @@ -1,17 +1,19 @@ import React from 'react'; -import assert from 'assert'; -import { shallow } from 'enzyme'; -import { render } from 'react-testing-library'; -import { UnconnectedReferenceInputController as ReferenceInputController } from './ReferenceInputController'; +import { WrappedFieldInputProps } from 'redux-form'; +import { cleanup } from 'react-testing-library'; +import omit from 'lodash/omit'; + +import renderWithRedux from '../../util/renderWithRedux'; +import ReferenceInputController from './ReferenceInputController'; +import { crudGetMatchingAccumulate } from '../../actions'; +import * as referenceDataStatus from './referenceDataStatus'; describe('', () => { const defaultProps = { basePath: '/comments', children: jest.fn(), - crudGetManyAccumulate: jest.fn(), - crudGetMatchingAccumulate: jest.fn(), meta: {}, - input: { value: undefined }, + input: { value: undefined } as WrappedFieldInputProps, onChange: jest.fn(), reference: 'posts', resource: 'comments', @@ -19,208 +21,167 @@ describe('', () => { translate: x => `*${x}*`, }; - it('should set isLoading to true if the references are being searched and a selected reference does not have data', () => { - const children = jest.fn(); - shallow( - - {children} - - ); - - assert.equal(children.mock.calls[0][0].isLoading, true); - }); - - it('should set isLoading to true if the references are being searched and there is no reference already selected', () => { - const children = jest.fn(); - shallow( - - {children} - - ); - assert.equal(children.mock.calls[0][0].isLoading, true); - }); - - it('should set isLoading to false if the references are being searched but a selected reference have data', () => { - const children = jest.fn(); - shallow( - - {children} - - ); - assert.equal(children.mock.calls[0][0].isLoading, false); - assert.deepEqual(children.mock.calls[0][0].choices, [{ id: 1 }]); - }); - - it('should set isLoading to false if the references were found but a selected reference does not have data', () => { - const children = jest.fn(); - shallow( - - {children} - - ); - assert.equal(children.mock.calls[0][0].isLoading, false); - assert.deepEqual(children.mock.calls[0][0].choices, [{ id: 2 }]); - }); - - it('should set error in case of references fetch error and selected reference does not have data', () => { - const children = jest.fn(); - shallow( + afterEach(cleanup); + + // it('should fetch reference and matchingReferences', () => { + // const children = jest.fn().mockReturnValue(

child

); + // const { dispatch } = renderWithRedux( + // + // {children} + // , + // { + // admin: { + // resources: { posts: { data: { 1: { id: 1 } } } }, + // references: { + // possibleValues: { 'comments@post_id': [2, 1] }, + // }, + // }, + // } + // ); + + // expect( + // omit(children.mock.calls[0][0], [ + // 'onChange', + // 'setPagination', + // 'setFilter', + // 'setSort', + // ]) + // ).toEqual({ + // choices: [{ id: 1 }], + // error: null, + // filter: {}, + // isLoading: false, + // pagination: { page: 1, perPage: 25 }, + // sort: { field: 'id', order: 'DESC' }, + // warning: null, + // }); + + // expect(dispatch).toBeCalledTimes(2); + // expect(dispatch.mock.calls[0][0].type).toBe( + // 'RA/CRUD_GET_MATCHING_ACCUMULATE' + // ); + // expect(dispatch.mock.calls[1][0].type).toBe( + // 'RA/CRUD_GET_MANY_ACCUMULATE' + // ); + // }); + + // it('should set isLoading to true if the references are being searched and there is no reference already selected', () => { + // const children = jest.fn().mockReturnValue(

child

); + // renderWithRedux( + // + // {children} + // , + // { + // admin: { + // resources: { posts: { data: { 1: { id: 1 } } } }, + // references: { + // possibleValues: { 'comments@post_id': null }, + // }, + // }, + // } + // ); + // expect(children.mock.calls[0][0].isLoading).toBe(true); + // }); + + // it('should set isLoading to false if the references are being searched but a selected reference have data', () => { + // const children = jest.fn().mockReturnValue(

child

); + // renderWithRedux( + // + // {children} + // , + // { + // admin: { + // resources: { posts: { data: { 1: { id: 1 } } } }, + // references: { + // possibleValues: { 'comments@post_id': null }, + // }, + // }, + // } + // ); + // expect(children.mock.calls[0][0].isLoading).toBe(false); + // expect(children.mock.calls[0][0].choices).toEqual([{ id: 1 }]); + // }); + + // it('should set isLoading to false if the references were found but a selected reference does not have data', () => { + // const children = jest.fn().mockReturnValue(

child

); + // renderWithRedux( + // + // {children} + // , + // { + // admin: { + // resources: { + // posts: { data: { 1: { id: 1 }, 2: { id: 2 } } }, + // }, + // references: { + // possibleValues: { 'comments@post_id': [2, 1, 3] }, + // }, + // }, + // } + // ); + // expect(children.mock.calls[0][0].isLoading).toBe(false); + // expect(children.mock.calls[0][0].choices).toEqual([ + // { id: 2 }, + // { id: 1 }, + // ]); + // }); + + it('should call getDataStatus with referenceRecord and matchingReferences', () => { + const children = jest.fn().mockReturnValue(

child

); + const spy = jest.spyOn(referenceDataStatus, 'getStatusForInput'); + renderWithRedux( {children} - - ); - assert.equal( - children.mock.calls[0][0].error, - '*ra.input.references.single_missing*' - ); - }); - - it('should set error in case of references fetch error and there is no reference already selected', () => { - const children = jest.fn(); - shallow( - - {children} - - ); - assert.equal(children.mock.calls[0][0].error, '*fetch error*'); - }); - - it('should not set error in case of references fetch error but selected reference have data', () => { - const children = jest.fn(); - shallow( - - {children} - - ); - - assert.equal(children.mock.calls[0][0].error, undefined); - }); - - it('should not set error if the references are empty (but fetched without error) and a selected reference does not have data', () => { - const children = jest.fn(); - shallow( - - {children} - - ); - assert.equal(children.mock.calls[0][0].error, undefined); - }); - - it('should set warning in case of references fetch error and there selected reference with data', () => { - const children = jest.fn(); - shallow( - - {children} - - ); - assert.equal(children.mock.calls[0][0].warning, '*fetch error*'); - }); - - it('should set warning if references were found but not the already selected one', () => { - const children = jest.fn(); - shallow( - - {children} - - ); - assert.equal( - children.mock.calls[0][0].warning, - '*ra.input.references.single_missing*' - ); - }); - - it('should not set warning if all references were found', () => { - const children = jest.fn(); - shallow( - - {children} - + , + { + admin: { + resources: { + posts: { + data: { 5: { id: 5 } }, + }, + }, + references: { + possibleValues: { + 'comments@post_id': [2, 1], + }, + }, + }, + } ); - assert.equal(children.mock.calls[0][0].warning, undefined); + expect(spy.mock.calls[0][0].input).toEqual({ value: 5 }); + expect(spy.mock.calls[0][0].matchingReferences).toEqual([2, 1]); + expect(spy.mock.calls[0][0].referenceRecord).toEqual({ id: 5 }); }); - it('should call crudGetMatchingAccumulate on mount with default fetch values', () => { - const crudGetMatchingAccumulate = jest.fn(); - shallow( - + it('should call useReferenceSearch', () => { + // const crudGetMatchingAccumulate = jest.fn(); + const { dispatch } = renderWithRedux( + ); assert.deepEqual(crudGetMatchingAccumulate.mock.calls[0], [ 'posts', @@ -237,295 +198,280 @@ describe('', () => { ]); }); - it('should allow to customize crudGetMatchingAccumulate arguments with perPage, sort, and filter props', () => { - const crudGetMatchingAccumulate = jest.fn(); - shallow( - - ); - assert.deepEqual(crudGetMatchingAccumulate.mock.calls[0], [ - 'posts', - 'comments@post_id', - { - page: 1, - perPage: 5, - }, - { - field: 'foo', - order: 'ASC', - }, - { - q: 'foo', - }, - ]); - }); - - it('should allow to customize crudGetMatchingAccumulate arguments with perPage, sort, and filter props without loosing original default filter', () => { - const crudGetMatchingAccumulate = jest.fn(); - const wrapper = shallow( - - ); - - wrapper.instance().setFilter('search_me'); - - assert.deepEqual(crudGetMatchingAccumulate.mock.calls[1], [ - 'posts', - 'comments@post_id', - { - page: 1, - perPage: 5, - }, - { - field: 'foo', - order: 'ASC', - }, - { - foo: 'bar', - q: 'search_me', - }, - ]); - }); - - it('should call crudGetMatchingAccumulate when setFilter is called', () => { - const crudGetMatchingAccumulate = jest.fn(); - const wrapper = shallow( - - ); - wrapper.instance().setFilter('bar'); - assert.deepEqual(crudGetMatchingAccumulate.mock.calls[1], [ - 'posts', - 'comments@post_id', - { - page: 1, - perPage: 25, - }, - { - field: 'id', - order: 'DESC', - }, - { - q: 'bar', - }, - ]); - }); - - it('should use custom filterToQuery function prop', () => { - const crudGetMatchingAccumulate = jest.fn(); - const wrapper = shallow( - ({ foo: searchText })} - /> - ); - wrapper.instance().setFilter('bar'); - assert.deepEqual(crudGetMatchingAccumulate.mock.calls[1], [ - 'posts', - 'comments@post_id', - { - page: 1, - perPage: 25, - }, - { - field: 'id', - order: 'DESC', - }, - { - foo: 'bar', - }, - ]); - }); - - it('should call crudGetManyAccumulate on mount if value is set', () => { - const crudGetManyAccumulate = jest.fn(); - shallow( - - ); - assert.deepEqual(crudGetManyAccumulate.mock.calls[0], ['posts', [5]]); - }); - - it('should pass onChange down to child component', () => { - const children = jest.fn(); - const onChange = jest.fn(); - shallow( - - {children} - - ); - assert.deepEqual(children.mock.calls[0][0].onChange, onChange); - }); - - it('should only call crudGetMatchingAccumulate when calling setFilter', () => { - const crudGetMatchingAccumulate = jest.fn(); - const crudGetManyAccumulate = jest.fn(); - const wrapper = shallow( - - ); - assert.equal(crudGetMatchingAccumulate.mock.calls.length, 1); - assert.equal(crudGetManyAccumulate.mock.calls.length, 1); - - wrapper.instance().setFilter('bar'); - assert.equal(crudGetMatchingAccumulate.mock.calls.length, 2); - assert.equal(crudGetManyAccumulate.mock.calls.length, 1); - }); - - it('should only call crudGetMatching when props are changed from outside', () => { - const crudGetMatchingAccumulate = jest.fn(); - const crudGetManyAccumulate = jest.fn(); - const ControllerWrapper = props => ( - - {() => null} - - ); - - const { rerender } = render(); - assert.equal(crudGetMatchingAccumulate.mock.calls.length, 1); - assert.equal(crudGetManyAccumulate.mock.calls.length, 1); - - rerender(); - - assert.equal(crudGetManyAccumulate.mock.calls.length, 1); - assert.deepEqual(crudGetMatchingAccumulate.mock.calls[1], [ - 'posts', - 'comments@post_id', - { page: 1, perPage: 25 }, - { field: 'id', order: 'DESC' }, - { foo: 'bar' }, - ]); - - rerender( - - ); - - assert.equal(crudGetManyAccumulate.mock.calls.length, 1); - assert.deepEqual(crudGetMatchingAccumulate.mock.calls[2], [ - 'posts', - 'comments@post_id', - { page: 1, perPage: 25 }, - { field: 'foo', order: 'ASC' }, - { foo: 'bar' }, - ]); - - rerender( - - ); - - assert.equal(crudGetManyAccumulate.mock.calls.length, 1); - assert.deepEqual(crudGetMatchingAccumulate.mock.calls[3], [ - 'posts', - 'comments@post_id', - { page: 1, perPage: 42 }, - { field: 'foo', order: 'ASC' }, - { foo: 'bar' }, - ]); - }); - - it('should only call crudGetMatchingAccumulate when props are changed from outside', () => { - const crudGetMatchingAccumulate = jest.fn(); - const crudGetManyAccumulate = jest.fn(); - const wrapper = shallow( - - ); - expect(crudGetMatchingAccumulate).toHaveBeenCalledTimes(1); - expect(crudGetManyAccumulate).toHaveBeenCalledTimes(1); - - wrapper.setProps({ filter: { foo: 'bar' } }); - expect(crudGetMatchingAccumulate.mock.calls.length).toBe(2); - expect(crudGetManyAccumulate).toHaveBeenCalledTimes(1); - - wrapper.setProps({ sort: { field: 'foo', order: 'ASC' } }); - expect(crudGetMatchingAccumulate.mock.calls.length).toBe(3); - expect(crudGetManyAccumulate).toHaveBeenCalledTimes(1); - - wrapper.setProps({ perPage: 42 }); - expect(crudGetMatchingAccumulate.mock.calls.length).toBe(4); - expect(crudGetManyAccumulate).toHaveBeenCalledTimes(1); - }); - - it('should call crudGetManyAccumulate when input value changes', () => { - const crudGetManyAccumulate = jest.fn(); - const wrapper = shallow( - - ); - assert.equal(crudGetManyAccumulate.mock.calls.length, 1); - wrapper.setProps({ input: { value: 6 } }); - assert.equal(crudGetManyAccumulate.mock.calls.length, 2); - }); - - it('should call crudGetManyAccumulate and crudGetMatchingAccumulate when record changes', () => { - const crudGetManyAccumulate = jest.fn(); - const crudGetMatchingAccumulate = jest.fn(); - const wrapper = shallow( - - ); - assert.equal(crudGetMatchingAccumulate.mock.calls.length, 1); - assert.equal(crudGetManyAccumulate.mock.calls.length, 1); - wrapper.setProps({ record: { id: 1 } }); - assert.equal(crudGetMatchingAccumulate.mock.calls.length, 2); - assert.equal(crudGetManyAccumulate.mock.calls.length, 2); - }); + // it('should allow to customize crudGetMatchingAccumulate arguments with perPage, sort, and filter props', () => { + // const crudGetMatchingAccumulate = jest.fn(); + // renderWithRedux( + // + // ); + // assert.deepEqual(crudGetMatchingAccumulate.mock.calls[0], [ + // 'posts', + // 'comments@post_id', + // { + // page: 1, + // perPage: 5, + // }, + // { + // field: 'foo', + // order: 'ASC', + // }, + // { + // q: 'foo', + // }, + // ]); + // }); + + // it('should allow to customize crudGetMatchingAccumulate arguments with perPage, sort, and filter props without loosing original default filter', () => { + // const crudGetMatchingAccumulate = jest.fn(); + // const wrapper = renderWithRedux( + // + // ); + + // wrapper.instance().setFilter('search_me'); + + // assert.deepEqual(crudGetMatchingAccumulate.mock.calls[1], [ + // 'posts', + // 'comments@post_id', + // { + // page: 1, + // perPage: 5, + // }, + // { + // field: 'foo', + // order: 'ASC', + // }, + // { + // foo: 'bar', + // q: 'search_me', + // }, + // ]); + // }); + + // it('should call crudGetMatchingAccumulate when setFilter is called', () => { + // const crudGetMatchingAccumulate = jest.fn(); + // const wrapper = renderWithRedux( + // + // ); + // wrapper.instance().setFilter('bar'); + // assert.deepEqual(crudGetMatchingAccumulate.mock.calls[1], [ + // 'posts', + // 'comments@post_id', + // { + // page: 1, + // perPage: 25, + // }, + // { + // field: 'id', + // order: 'DESC', + // }, + // { + // q: 'bar', + // }, + // ]); + // }); + + // it('should use custom filterToQuery function prop', () => { + // const crudGetMatchingAccumulate = jest.fn(); + // const wrapper = renderWithRedux( + // ({ foo: searchText })} + // /> + // ); + // wrapper.instance().setFilter('bar'); + // assert.deepEqual(crudGetMatchingAccumulate.mock.calls[1], [ + // 'posts', + // 'comments@post_id', + // { + // page: 1, + // perPage: 25, + // }, + // { + // field: 'id', + // order: 'DESC', + // }, + // { + // foo: 'bar', + // }, + // ]); + // }); + + // it('should call crudGetManyAccumulate on mount if value is set', () => { + // const crudGetManyAccumulate = jest.fn(); + // renderWithRedux( + // + // ); + // assert.deepEqual(crudGetManyAccumulate.mock.calls[0], ['posts', [5]]); + // }); + + // it('should pass onChange down to child component', () => { + // const children = jest.fn().mockReturnValue(

child

); + // const onChange = jest.fn(); + // renderWithRedux( + // + // {children} + // + // ); + // assert.deepEqual(children.mock.calls[0][0].onChange, onChange); + // }); + + // it('should only call crudGetMatchingAccumulate when calling setFilter', () => { + // const crudGetMatchingAccumulate = jest.fn(); + // const crudGetManyAccumulate = jest.fn(); + // const wrapper = renderWithRedux( + // + // ); + // assert.equal(crudGetMatchingAccumulate.mock.calls.length, 1); + // assert.equal(crudGetManyAccumulate.mock.calls.length, 1); + + // wrapper.instance().setFilter('bar'); + // assert.equal(crudGetMatchingAccumulate.mock.calls.length, 2); + // assert.equal(crudGetManyAccumulate.mock.calls.length, 1); + // }); + + // it('should only call crudGetMatching when props are changed from outside', () => { + // const crudGetMatchingAccumulate = jest.fn(); + // const crudGetManyAccumulate = jest.fn(); + // const ControllerWrapper = props => ( + // + // {() => null} + // + // ); + + // const { rerender } = render(); + // assert.equal(crudGetMatchingAccumulate.mock.calls.length, 1); + // assert.equal(crudGetManyAccumulate.mock.calls.length, 1); + + // rerender(); + + // assert.equal(crudGetManyAccumulate.mock.calls.length, 1); + // assert.deepEqual(crudGetMatchingAccumulate.mock.calls[1], [ + // 'posts', + // 'comments@post_id', + // { page: 1, perPage: 25 }, + // { field: 'id', order: 'DESC' }, + // { foo: 'bar' }, + // ]); + + // rerender( + // + // ); + + // assert.equal(crudGetManyAccumulate.mock.calls.length, 1); + // assert.deepEqual(crudGetMatchingAccumulate.mock.calls[2], [ + // 'posts', + // 'comments@post_id', + // { page: 1, perPage: 25 }, + // { field: 'foo', order: 'ASC' }, + // { foo: 'bar' }, + // ]); + + // rerender( + // + // ); + + // assert.equal(crudGetManyAccumulate.mock.calls.length, 1); + // assert.deepEqual(crudGetMatchingAccumulate.mock.calls[3], [ + // 'posts', + // 'comments@post_id', + // { page: 1, perPage: 42 }, + // { field: 'foo', order: 'ASC' }, + // { foo: 'bar' }, + // ]); + // }); + + // it('should only call crudGetMatchingAccumulate when props are changed from outside', () => { + // const crudGetMatchingAccumulate = jest.fn(); + // const crudGetManyAccumulate = jest.fn(); + // const wrapper = renderWithRedux( + // + // ); + // expect(crudGetMatchingAccumulate).toHaveBeenCalledTimes(1); + // expect(crudGetManyAccumulate).toHaveBeenCalledTimes(1); + + // wrapper.setProps({ filter: { foo: 'bar' } }); + // expect(crudGetMatchingAccumulate.mock.calls.length).toBe(2); + // expect(crudGetManyAccumulate).toHaveBeenCalledTimes(1); + + // wrapper.setProps({ sort: { field: 'foo', order: 'ASC' } }); + // expect(crudGetMatchingAccumulate.mock.calls.length).toBe(3); + // expect(crudGetManyAccumulate).toHaveBeenCalledTimes(1); + + // wrapper.setProps({ perPage: 42 }); + // expect(crudGetMatchingAccumulate.mock.calls.length).toBe(4); + // expect(crudGetManyAccumulate).toHaveBeenCalledTimes(1); + // }); + + // it('should call crudGetManyAccumulate when input value changes', () => { + // const crudGetManyAccumulate = jest.fn(); + // const wrapper = renderWithRedux( + // + // ); + // assert.equal(crudGetManyAccumulate.mock.calls.length, 1); + // wrapper.setProps({ input: { value: 6 } }); + // assert.equal(crudGetManyAccumulate.mock.calls.length, 2); + // }); + + // it('should call crudGetManyAccumulate and crudGetMatchingAccumulate when record changes', () => { + // const crudGetManyAccumulate = jest.fn(); + // const crudGetMatchingAccumulate = jest.fn(); + // const wrapper = renderWithRedux( + // + // ); + // assert.equal(crudGetMatchingAccumulate.mock.calls.length, 1); + // assert.equal(crudGetManyAccumulate.mock.calls.length, 1); + // wrapper.setProps({ record: { id: 1 } }); + // assert.equal(crudGetMatchingAccumulate.mock.calls.length, 2); + // assert.equal(crudGetManyAccumulate.mock.calls.length, 2); + // }); }); diff --git a/packages/ra-core/src/controller/input/useMatchingReferences.spec.tsx b/packages/ra-core/src/controller/input/useMatchingReferences.spec.tsx index 392f301177f..5f0ed42bf67 100644 --- a/packages/ra-core/src/controller/input/useMatchingReferences.spec.tsx +++ b/packages/ra-core/src/controller/input/useMatchingReferences.spec.tsx @@ -19,7 +19,7 @@ describe('useMatchingReferences', () => { afterEach(cleanup); it('should fetch matchingReferences on mount', () => { - const { dispatch, rerender } = renderHookWithRedux(() => { + const { dispatch } = renderHookWithRedux(() => { return useMatchingReferences(defaultProps); }); @@ -81,4 +81,29 @@ describe('useMatchingReferences', () => { expect(childrenProps.loading).toBe(false); expect(childrenProps.error).toBe('Something bad happened'); }); + + it('should pass loading true if no matching reference yet', () => { + const { childrenProps } = renderHookWithRedux( + () => { + return useMatchingReferences(defaultProps); + }, + { + admin: { + resources: { + posts: { data: {} }, + }, + references: { + possibleValues: { + 'comments@post_id': null, + }, + }, + }, + } + ); + + expect(childrenProps.matchingReferences).toBe(null); + + expect(childrenProps.loading).toBe(true); + expect(childrenProps.error).toBe(null); + }); }); diff --git a/packages/ra-core/src/controller/input/useMatchingReferences.ts b/packages/ra-core/src/controller/input/useMatchingReferences.ts index b44b46bc03f..5455579dae7 100644 --- a/packages/ra-core/src/controller/input/useMatchingReferences.ts +++ b/packages/ra-core/src/controller/input/useMatchingReferences.ts @@ -76,8 +76,7 @@ export default ({ reference, resource, source, - }), - [filter, referenceSource, reference, source, resource] + }) ); if (!matchingReferences) { diff --git a/packages/ra-core/src/controller/useReference.spec.ts b/packages/ra-core/src/controller/useReference.spec.ts new file mode 100644 index 00000000000..92ece79a023 --- /dev/null +++ b/packages/ra-core/src/controller/useReference.spec.ts @@ -0,0 +1,64 @@ +import renderHookWithRedux from '../util/renderHookWithRedux'; +import useReference from './useReference'; +import { cleanup } from 'react-testing-library'; + +describe('useReference', () => { + const defaultProps = { + id: 'id', + reference: 'posts', + allowEmpty: false, + }; + + afterEach(cleanup); + + it('should fetch references on mount', () => { + const { dispatch } = renderHookWithRedux(() => { + return useReference(defaultProps); + }); + + expect(dispatch).toBeCalledTimes(1); + expect(dispatch.mock.calls[0][0].type).toBe( + 'RA/CRUD_GET_MANY_ACCUMULATE' + ); + }); + + it('should pass referenceRecord from redux state to its children', () => { + const { childrenProps } = renderHookWithRedux( + () => { + return useReference(defaultProps); + }, + { + admin: { + resources: { + posts: { data: { id: { id: 'id' }, 2: { id: 2 } } }, + }, + }, + } + ); + + expect(childrenProps).toEqual({ + referenceRecord: { id: 'id' }, + isLoading: false, + }); + }); + + it('should pass loading true if no referenceRecord yet', () => { + const { childrenProps } = renderHookWithRedux( + () => { + return useReference(defaultProps); + }, + { + admin: { + resources: { + posts: { data: {} }, + }, + }, + } + ); + + expect(childrenProps).toEqual({ + referenceRecord: undefined, + isLoading: true, + }); + }); +}); diff --git a/packages/ra-core/src/controller/useReference.ts b/packages/ra-core/src/controller/useReference.ts index b74921c0481..cc734c29f2c 100644 --- a/packages/ra-core/src/controller/useReference.ts +++ b/packages/ra-core/src/controller/useReference.ts @@ -5,7 +5,6 @@ import { useDispatch, useSelector } from 'react-redux'; import { crudGetManyAccumulate } from '../actions'; import { Record } from '../types'; import { getReferenceResource } from '../reducer'; -import { createSelector } from 'reselect'; interface Option { id: string; @@ -50,9 +49,18 @@ export const useReference = ({ reference, id, }: Option): UseReferenceProps => { - const getReferenceRecord = useMemo(makeSelectedReferenceSelector, []); - const referenceRecord = useSelector(getReferenceRecord({ id, reference })); const dispatch = useDispatch(); + const getReferenceRecord = useMemo( + () => makeGetReferenceRecord({ id, reference }), + [id, reference] + ); + const referenceRecord = useSelector( + getReferenceRecord, + (newState, latestState) => { + console.log({ newState, latestState }); + return newState === latestState; + } + ); useEffect(() => { if (id !== null && typeof id !== 'undefined') { dispatch(crudGetManyAccumulate(reference, [id])); @@ -65,13 +73,10 @@ export const useReference = ({ }; }; -const makeSelectedReferenceSelector = () => { - const selectedReferenceSelector = createSelector( - [getReferenceResource, (_, props) => props.id], - (referenceState, id) => referenceState && referenceState.data[id] - ); - - return props => state => selectedReferenceSelector(state, props); +const makeGetReferenceRecord = props => state => { + const referenceState = getReferenceResource(state, props); + console.log({ referenceState }); + return referenceState && referenceState.data[props.id]; }; export default useReference; From 72deb2514eb39b935bca12d30536189e92b45fe4 Mon Sep 17 00:00:00 2001 From: thiery Date: Wed, 19 Jun 2019 14:31:06 +0200 Subject: [PATCH 10/48] fix TestContext to not mutate its default store --- packages/ra-core/src/util/TestContext.tsx | 1 + 1 file changed, 1 insertion(+) diff --git a/packages/ra-core/src/util/TestContext.tsx b/packages/ra-core/src/util/TestContext.tsx index 78bb8e91f55..988d9377286 100644 --- a/packages/ra-core/src/util/TestContext.tsx +++ b/packages/ra-core/src/util/TestContext.tsx @@ -55,6 +55,7 @@ class TestContext extends Component { constructor(props) { super(props); const { initialState = {}, enableReducers = false } = props; + this.storeWithDefault = enableReducers ? createAdminStore({ initialState: merge({}, defaultStore, initialState), From 9ae131b7167ac023e139d8d9730b07b08060f000 Mon Sep 17 00:00:00 2001 From: thiery Date: Wed, 19 Jun 2019 15:25:48 +0200 Subject: [PATCH 11/48] add test on useReference and useMatchingReferences --- .../input/useMatchingReferences.spec.tsx | 467 +++++++++++++++++- .../controller/input/useMatchingReferences.ts | 35 +- .../src/controller/useReference.spec.ts | 87 +++- .../ra-core/src/controller/useReference.ts | 10 +- .../ra-core/src/util/renderHookWithRedux.tsx | 6 +- packages/ra-core/src/util/usePrevious.ts | 10 + 6 files changed, 583 insertions(+), 32 deletions(-) create mode 100644 packages/ra-core/src/util/usePrevious.ts diff --git a/packages/ra-core/src/controller/input/useMatchingReferences.spec.tsx b/packages/ra-core/src/controller/input/useMatchingReferences.spec.tsx index 5f0ed42bf67..07736fa5055 100644 --- a/packages/ra-core/src/controller/input/useMatchingReferences.spec.tsx +++ b/packages/ra-core/src/controller/input/useMatchingReferences.spec.tsx @@ -19,7 +19,7 @@ describe('useMatchingReferences', () => { afterEach(cleanup); it('should fetch matchingReferences on mount', () => { - const { dispatch } = renderHookWithRedux(() => { + const { dispatch, rerender } = renderHookWithRedux(() => { return useMatchingReferences(defaultProps); }); @@ -27,6 +27,467 @@ describe('useMatchingReferences', () => { expect(dispatch.mock.calls[0][0].type).toBe( 'RA/CRUD_GET_MATCHING_ACCUMULATE' ); + expect( + JSON.parse(dispatch.mock.calls[0][0].meta.accumulateKey) + ).toEqual({ + resource: 'posts', + relatedTo: 'comments@post_id', + pagination:{ + perPage:25, + page:1 + }, + sort:{ + field: 'id', + order: 'DESC' + }, + filter:{ + q: '' + } + }); + + rerender(() => { + return useMatchingReferences({ + reference: 'posts', + resource: 'comments', + source: 'post_id', + filter: { q: '' }, + pagination: { + perPage: 25, + page: 1, + }, + sort: { field: 'id', order: 'DESC' }, + referenceSource: undefined, + }); // deep but not shallow equal + }); + expect(dispatch).toBeCalledTimes(1); + }); + + it('should fetch matchingReferences when filter change', () => { + const { dispatch, rerender } = renderHookWithRedux(() => { + return useMatchingReferences(defaultProps); + }); + + expect(dispatch).toBeCalledTimes(1); + expect(dispatch.mock.calls[0][0].type).toBe( + 'RA/CRUD_GET_MATCHING_ACCUMULATE' + ); + expect( + JSON.parse(dispatch.mock.calls[0][0].meta.accumulateKey) + ).toEqual({ + resource: 'posts', + relatedTo: 'comments@post_id', + pagination:{ + perPage:25, + page:1 + }, + sort:{ + field: 'id', + order: 'DESC' + }, + filter:{ + q: '' + } + }); + + rerender(() => { + return useMatchingReferences({...defaultProps, filter: { q: 'typing' } }); + }); + expect(dispatch).toBeCalledTimes(2);expect(dispatch.mock.calls[1][0].type).toBe( + 'RA/CRUD_GET_MATCHING_ACCUMULATE' + ); + expect( + JSON.parse(dispatch.mock.calls[1][0].meta.accumulateKey) + ).toEqual({ + resource: 'posts', + relatedTo: 'comments@post_id', + pagination:{ + perPage:25, + page:1 + }, + sort:{ + field: 'id', + order: 'DESC' + }, + filter:{ + q: 'typing' + } + }); + }); + + it('should refetch matchingReferences when reference change', () => { + const { dispatch, rerender } = renderHookWithRedux(() => { + return useMatchingReferences(defaultProps); + }); + + expect(dispatch).toBeCalledTimes(1); + expect(dispatch.mock.calls[0][0].type).toBe( + 'RA/CRUD_GET_MATCHING_ACCUMULATE' + ); + expect( + JSON.parse(dispatch.mock.calls[0][0].meta.accumulateKey) + ).toEqual({ + resource: 'posts', + relatedTo: 'comments@post_id', + pagination:{ + perPage:25, + page:1 + }, + sort:{ + field: 'id', + order: 'DESC' + }, + filter:{ + q: '' + } + }); + + rerender(() => { + return useMatchingReferences({...defaultProps, reference: 'blog_posts' }); + }); + expect(dispatch).toBeCalledTimes(2);expect(dispatch.mock.calls[1][0].type).toBe( + 'RA/CRUD_GET_MATCHING_ACCUMULATE' + ); + expect( + JSON.parse(dispatch.mock.calls[1][0].meta.accumulateKey) + ).toEqual({ + resource: 'blog_posts', + relatedTo: 'comments@post_id', + pagination:{ + perPage:25, + page:1 + }, + sort:{ + field: 'id', + order: 'DESC' + }, + filter:{ + q: '' + } + }); + }); + + it('should refetch matchingReferences when resource change', () => { + const { dispatch, rerender } = renderHookWithRedux(() => { + return useMatchingReferences(defaultProps); + }); + + expect(dispatch).toBeCalledTimes(1); + expect(dispatch.mock.calls[0][0].type).toBe( + 'RA/CRUD_GET_MATCHING_ACCUMULATE' + ); + expect( + JSON.parse(dispatch.mock.calls[0][0].meta.accumulateKey) + ).toEqual({ + resource: 'posts', + relatedTo: 'comments@post_id', + pagination:{ + perPage:25, + page:1 + }, + sort:{ + field: 'id', + order: 'DESC' + }, + filter:{ + q: '' + } + }); + + rerender(() => { + return useMatchingReferences({...defaultProps, resource: 'note' }); + }); + expect(dispatch).toBeCalledTimes(2);expect(dispatch.mock.calls[1][0].type).toBe( + 'RA/CRUD_GET_MATCHING_ACCUMULATE' + ); + expect( + JSON.parse(dispatch.mock.calls[1][0].meta.accumulateKey) + ).toEqual({ + resource: 'posts', + relatedTo: 'note@post_id', + pagination:{ + perPage:25, + page:1 + }, + sort:{ + field: 'id', + order: 'DESC' + }, + filter:{ + q: '' + } + }); + }); + + it('should refetch matchingReferences when source change', () => { + const { dispatch, rerender } = renderHookWithRedux(() => { + return useMatchingReferences(defaultProps); + }); + + expect(dispatch).toBeCalledTimes(1); + expect(dispatch.mock.calls[0][0].type).toBe( + 'RA/CRUD_GET_MATCHING_ACCUMULATE' + ); + expect( + JSON.parse(dispatch.mock.calls[0][0].meta.accumulateKey) + ).toEqual({ + resource: 'posts', + relatedTo: 'comments@post_id', + pagination:{ + perPage:25, + page:1 + }, + sort:{ + field: 'id', + order: 'DESC' + }, + filter:{ + q: '' + } + }); + + rerender(() => { + return useMatchingReferences({...defaultProps, source: 'blog_posts_id' }); + }); + expect(dispatch).toBeCalledTimes(2);expect(dispatch.mock.calls[1][0].type).toBe( + 'RA/CRUD_GET_MATCHING_ACCUMULATE' + ); + expect( + JSON.parse(dispatch.mock.calls[1][0].meta.accumulateKey) + ).toEqual({ + resource: 'posts', + relatedTo: 'comments@blog_posts_id', + pagination:{ + perPage:25, + page:1 + }, + sort:{ + field: 'id', + order: 'DESC' + }, + filter:{ + q: '' + } + }); + }); + + it('should refetch matchingReferences when pagination.page change', () => { + const { dispatch, rerender } = renderHookWithRedux(() => { + return useMatchingReferences(defaultProps); + }); + + expect(dispatch).toBeCalledTimes(1); + expect(dispatch.mock.calls[0][0].type).toBe( + 'RA/CRUD_GET_MATCHING_ACCUMULATE' + ); + expect( + JSON.parse(dispatch.mock.calls[0][0].meta.accumulateKey) + ).toEqual({ + resource: 'posts', + relatedTo: 'comments@post_id', + pagination:{ + perPage:25, + page:1 + }, + sort:{ + field: 'id', + order: 'DESC' + }, + filter:{ + q: '' + } + }); + + rerender(() => { + return useMatchingReferences({...defaultProps, pagination: { + perPage: 25, + page: 2, + } }); + }); + expect(dispatch).toBeCalledTimes(2);expect(dispatch.mock.calls[1][0].type).toBe( + 'RA/CRUD_GET_MATCHING_ACCUMULATE' + ); + expect( + JSON.parse(dispatch.mock.calls[1][0].meta.accumulateKey) + ).toEqual({ + resource: 'posts', + relatedTo: 'comments@post_id', + pagination:{ + perPage:25, + page: 2, + }, + sort:{ + field: 'id', + order: 'DESC' + }, + filter:{ + q: '' + } + }); + }); + + it('should refetch matchingReferences when pagination.pagination change', () => { + const { dispatch, rerender } = renderHookWithRedux(() => { + return useMatchingReferences(defaultProps); + }); + + expect(dispatch).toBeCalledTimes(1); + expect(dispatch.mock.calls[0][0].type).toBe( + 'RA/CRUD_GET_MATCHING_ACCUMULATE' + ); + expect( + JSON.parse(dispatch.mock.calls[0][0].meta.accumulateKey) + ).toEqual({ + resource: 'posts', + relatedTo: 'comments@post_id', + pagination:{ + perPage:25, + page:1 + }, + sort:{ + field: 'id', + order: 'DESC' + }, + filter:{ + q: '' + } + }); + + rerender(() => { + return useMatchingReferences({...defaultProps, pagination: { + perPage: 50, + page: 1, + } }); + }); + expect(dispatch).toBeCalledTimes(2);expect(dispatch.mock.calls[1][0].type).toBe( + 'RA/CRUD_GET_MATCHING_ACCUMULATE' + ); + expect( + JSON.parse(dispatch.mock.calls[1][0].meta.accumulateKey) + ).toEqual({ + resource: 'posts', + relatedTo: 'comments@post_id', + pagination:{ + perPage:50, + page: 1, + }, + sort:{ + field: 'id', + order: 'DESC' + }, + filter:{ + q: '' + } + }); + }); + + it('should refetch matchingReferences when sort.field change', () => { + const { dispatch, rerender } = renderHookWithRedux(() => { + return useMatchingReferences(defaultProps); + }); + + expect(dispatch).toBeCalledTimes(1); + expect(dispatch.mock.calls[0][0].type).toBe( + 'RA/CRUD_GET_MATCHING_ACCUMULATE' + ); + expect( + JSON.parse(dispatch.mock.calls[0][0].meta.accumulateKey) + ).toEqual({ + resource: 'posts', + relatedTo: 'comments@post_id', + pagination:{ + perPage:25, + page:1 + }, + sort:{ + field: 'id', + order: 'DESC' + }, + filter:{ + q: '' + } + }); + + rerender(() => { + return useMatchingReferences({...defaultProps, sort:{ + field: 'uid', + order: 'DESC' + } }); + }); + expect(dispatch).toBeCalledTimes(2);expect(dispatch.mock.calls[1][0].type).toBe( + 'RA/CRUD_GET_MATCHING_ACCUMULATE' + ); + expect( + JSON.parse(dispatch.mock.calls[1][0].meta.accumulateKey) + ).toEqual({ + resource: 'posts', + relatedTo: 'comments@post_id', + pagination:{ + perPage: 25, + page: 1, + }, + sort:{ + field: 'uid', + order: 'DESC' + }, + filter:{ + q: '' + } + }); + }); + + it('should refetch matchingReferences when sort.order change', () => { + const { dispatch, rerender } = renderHookWithRedux(() => { + return useMatchingReferences(defaultProps); + }); + + expect(dispatch).toBeCalledTimes(1); + expect(dispatch.mock.calls[0][0].type).toBe( + 'RA/CRUD_GET_MATCHING_ACCUMULATE' + ); + expect( + JSON.parse(dispatch.mock.calls[0][0].meta.accumulateKey) + ).toEqual({ + resource: 'posts', + relatedTo: 'comments@post_id', + pagination:{ + perPage:25, + page:1 + }, + sort:{ + field: 'id', + order: 'DESC' + }, + filter:{ + q: '' + } + }); + + rerender(() => { + return useMatchingReferences({ ...defaultProps, sort: { + field: 'id', + order: 'ASC' + } }); + }); + expect(dispatch).toBeCalledTimes(2);expect(dispatch.mock.calls[1][0].type).toBe( + 'RA/CRUD_GET_MATCHING_ACCUMULATE' + ); + expect( + JSON.parse(dispatch.mock.calls[1][0].meta.accumulateKey) + ).toEqual({ + resource: 'posts', + relatedTo: 'comments@post_id', + pagination:{ + perPage: 25, + page: 1, + }, + sort:{ + field: 'id', + order: 'ASC' + }, + filter:{ + q: '' + } + }); }); it('should pass matching references from redux state to its children', () => { @@ -93,9 +554,7 @@ describe('useMatchingReferences', () => { posts: { data: {} }, }, references: { - possibleValues: { - 'comments@post_id': null, - }, + possibleValues: {}, }, }, } diff --git a/packages/ra-core/src/controller/input/useMatchingReferences.ts b/packages/ra-core/src/controller/input/useMatchingReferences.ts index 5455579dae7..fdbf1cc9c34 100644 --- a/packages/ra-core/src/controller/input/useMatchingReferences.ts +++ b/packages/ra-core/src/controller/input/useMatchingReferences.ts @@ -2,6 +2,7 @@ import { useEffect, useMemo } from 'react'; // @ts-ignore import { useSelector, useDispatch } from 'react-redux'; import { createSelector } from 'reselect'; +import isEqual from 'lodash.isequal'; import { Filter } from '../useFilterState'; import { crudGetMatchingAccumulate } from '../../actions/accumulateActions'; @@ -11,6 +12,7 @@ import { getReferenceResource, } from '../../reducer'; import { Pagination, Sort, Record } from '../../types'; +import usePrevious from '../../util/usePrevious'; interface UseMAtchingReferencesOption { reference: string; @@ -44,8 +46,25 @@ export default ({ const getMatchingReferences = useMemo(makeMatchingReferencesSelector, []); + const options = { + dispatch, + filter, + reference, + referenceSource, + resource, + source, + pagination, + sort, + }; + + const previousOption = usePrevious(options); + useEffect( - () => + () => { + if (isEqual(options, previousOption)) { + return; + } + fetchOptions({ dispatch, filter, @@ -55,18 +74,8 @@ export default ({ source, pagination, sort, - }), - [ - filter, - reference, - referenceSource, - resource, - source, - pagination.page, - pagination.perPage, - sort.field, - sort.order, - ] + }) + } ); const matchingReferences = useSelector( diff --git a/packages/ra-core/src/controller/useReference.spec.ts b/packages/ra-core/src/controller/useReference.spec.ts index 92ece79a023..d7921707cc8 100644 --- a/packages/ra-core/src/controller/useReference.spec.ts +++ b/packages/ra-core/src/controller/useReference.spec.ts @@ -4,15 +4,15 @@ import { cleanup } from 'react-testing-library'; describe('useReference', () => { const defaultProps = { - id: 'id', + id: '1', reference: 'posts', allowEmpty: false, }; afterEach(cleanup); - it('should fetch references on mount', () => { - const { dispatch } = renderHookWithRedux(() => { + it('should fetch reference on mount', () => { + const { dispatch, rerender } = renderHookWithRedux(() => { return useReference(defaultProps); }); @@ -20,6 +20,61 @@ describe('useReference', () => { expect(dispatch.mock.calls[0][0].type).toBe( 'RA/CRUD_GET_MANY_ACCUMULATE' ); + + rerender(() => { + return useReference(defaultProps); + }); + expect(dispatch).toBeCalledTimes(1); + }); + + it('should refetch reference when id change', () => { + const { dispatch, rerender } = renderHookWithRedux(() => { + return useReference(defaultProps); + }); + + expect(dispatch).toBeCalledTimes(1); + expect(dispatch.mock.calls[0][0].type).toBe( + 'RA/CRUD_GET_MANY_ACCUMULATE' + ); + expect(dispatch.mock.calls[0][0].payload).toEqual( + { ids: ['1'], resource: 'posts' } + ); + rerender(() => { + return useReference({ ...defaultProps, id: '2' }); + }) + + expect(dispatch).toBeCalledTimes(2); + expect(dispatch.mock.calls[1][0].type).toBe( + 'RA/CRUD_GET_MANY_ACCUMULATE' + ); + expect(dispatch.mock.calls[1][0].payload).toEqual( + { ids: ['2'], resource: 'posts' } + ); + }); + + it('should refetch reference when reference prop change', () => { + const { dispatch, rerender } = renderHookWithRedux(() => { + return useReference(defaultProps); + }); + + expect(dispatch).toBeCalledTimes(1); + expect(dispatch.mock.calls[0][0].type).toBe( + 'RA/CRUD_GET_MANY_ACCUMULATE' + ); + expect(dispatch.mock.calls[0][0].payload).toEqual( + { ids: ['1'], resource: 'posts' } + ); + rerender(() => { + return useReference({ ...defaultProps, reference: 'comments' }); + }) + + expect(dispatch).toBeCalledTimes(2); + expect(dispatch.mock.calls[1][0].type).toBe( + 'RA/CRUD_GET_MANY_ACCUMULATE' + ); + expect(dispatch.mock.calls[1][0].payload).toEqual( + { ids: ['1'], resource: 'comments' } + ); }); it('should pass referenceRecord from redux state to its children', () => { @@ -30,19 +85,19 @@ describe('useReference', () => { { admin: { resources: { - posts: { data: { id: { id: 'id' }, 2: { id: 2 } } }, + posts: { data: { 1: { id: 1 }, 2: { id: 2 } } }, }, }, } ); expect(childrenProps).toEqual({ - referenceRecord: { id: 'id' }, + referenceRecord: { id: 1 }, isLoading: false, }); }); - it('should pass loading true if no referenceRecord yet', () => { + it('should set isLoading to true if no referenceRecord yet', () => { const { childrenProps } = renderHookWithRedux( () => { return useReference(defaultProps); @@ -61,4 +116,24 @@ describe('useReference', () => { isLoading: true, }); }); + + it('should set isLoading to false even if no referenceRecord yet when allowEmpty is true', () => { + const { childrenProps } = renderHookWithRedux( + () => { + return useReference({ ...defaultProps, allowEmpty: true }); + }, + { + admin: { + resources: { + posts: { data: {} }, + }, + }, + } + ); + + expect(childrenProps).toEqual({ + referenceRecord: undefined, + isLoading: false, + }); + }); }); diff --git a/packages/ra-core/src/controller/useReference.ts b/packages/ra-core/src/controller/useReference.ts index cc734c29f2c..b181141e89f 100644 --- a/packages/ra-core/src/controller/useReference.ts +++ b/packages/ra-core/src/controller/useReference.ts @@ -54,13 +54,7 @@ export const useReference = ({ () => makeGetReferenceRecord({ id, reference }), [id, reference] ); - const referenceRecord = useSelector( - getReferenceRecord, - (newState, latestState) => { - console.log({ newState, latestState }); - return newState === latestState; - } - ); + const referenceRecord = useSelector(getReferenceRecord); useEffect(() => { if (id !== null && typeof id !== 'undefined') { dispatch(crudGetManyAccumulate(reference, [id])); @@ -75,7 +69,7 @@ export const useReference = ({ const makeGetReferenceRecord = props => state => { const referenceState = getReferenceResource(state, props); - console.log({ referenceState }); + return referenceState && referenceState.data[props.id]; }; diff --git a/packages/ra-core/src/util/renderHookWithRedux.tsx b/packages/ra-core/src/util/renderHookWithRedux.tsx index 9d4008af2c9..a4576b6b42f 100644 --- a/packages/ra-core/src/util/renderHookWithRedux.tsx +++ b/packages/ra-core/src/util/renderHookWithRedux.tsx @@ -15,5 +15,9 @@ export default (hookProps, reduxState?) => { reduxState ); - return { ...result, childrenProps }; + return { ...result, childrenProps, rerender: newHook => { + return result.rerender( + , + ); + }, }; }; diff --git a/packages/ra-core/src/util/usePrevious.ts b/packages/ra-core/src/util/usePrevious.ts new file mode 100644 index 00000000000..d9a08a677c0 --- /dev/null +++ b/packages/ra-core/src/util/usePrevious.ts @@ -0,0 +1,10 @@ +import { useRef, useEffect } from "react"; + +export default value => { + const ref = useRef(); + useEffect(() => { + ref.current = value; + }, [ value]); + + return ref.current; +} \ No newline at end of file From 3e9e879e521c87959881c00a57db1c4fc6390d46 Mon Sep 17 00:00:00 2001 From: thiery Date: Wed, 19 Jun 2019 16:16:16 +0200 Subject: [PATCH 12/48] add test on usePaginationState hook --- .../field/ReferenceManyFieldController.tsx | 2 +- .../input/ReferenceInputController.tsx | 2 +- .../src/controller/usePaginationState.spec.ts | 71 +++++++++++++++++++ .../src/controller/usePaginationState.ts | 46 ++++++++---- packages/ra-core/src/util/renderHook.tsx | 26 +++++++ .../src/field/ReferenceManyField.js | 6 +- 6 files changed, 133 insertions(+), 20 deletions(-) create mode 100644 packages/ra-core/src/controller/usePaginationState.spec.ts create mode 100644 packages/ra-core/src/util/renderHook.tsx diff --git a/packages/ra-core/src/controller/field/ReferenceManyFieldController.tsx b/packages/ra-core/src/controller/field/ReferenceManyFieldController.tsx index b005c914c78..2fa352a697f 100644 --- a/packages/ra-core/src/controller/field/ReferenceManyFieldController.tsx +++ b/packages/ra-core/src/controller/field/ReferenceManyFieldController.tsx @@ -93,7 +93,7 @@ export const ReferenceManyFieldController: FunctionComponent = ({ }) => { const { sort, setSortField } = useSortState(initialSort); const { page, perPage, setPage, setPerPage } = usePaginationState( - initialPerPage + { perPage: initialPerPage } ); const { data, diff --git a/packages/ra-core/src/controller/input/ReferenceInputController.tsx b/packages/ra-core/src/controller/input/ReferenceInputController.tsx index d38c3d9d849..dc8ebe3524c 100644 --- a/packages/ra-core/src/controller/input/ReferenceInputController.tsx +++ b/packages/ra-core/src/controller/input/ReferenceInputController.tsx @@ -142,7 +142,7 @@ export const ReferenceInputController: FunctionComponent = ({ }) => { const translate = useTranslate(); - const { pagination, setPagination } = usePaginationState(perPage); + const { pagination, setPagination } = usePaginationState({ perPage }); const { sort, setSort } = useSortState(); const { filter, setFilter } = useFilterState({ permanentFilter, diff --git a/packages/ra-core/src/controller/usePaginationState.spec.ts b/packages/ra-core/src/controller/usePaginationState.spec.ts new file mode 100644 index 00000000000..b6b6db80178 --- /dev/null +++ b/packages/ra-core/src/controller/usePaginationState.spec.ts @@ -0,0 +1,71 @@ + +import renderHook from '../util/renderHook'; +import usePaginationState from './usePaginationState'; +import { act } from 'react-testing-library'; + +describe('usePaginationState', () => { + it('should initialise pagination state with default', () => { + const { childrenProps } = renderHook(() => usePaginationState()); + expect(childrenProps.pagination).toEqual({ page: 1, perPage: 25 }); + }); + + it('should take given page and perpage to initalise', () => { + const { childrenProps } = renderHook(() => usePaginationState({ perPage: 50, page: 10 })); + expect(childrenProps.pagination).toEqual({ page: 10, perPage: 50 }); + }); + + it('should update perPage if its props update', () => { + const { childrenProps, childrenMock, rerender } = renderHook(() => usePaginationState({ perPage: 50, page: 10 })); + expect(childrenProps.pagination).toEqual({ page: 10, perPage: 50 }); + rerender(() => usePaginationState({ perPage: 100, page: 10 })); + + expect(childrenMock).toBeCalledTimes(3); + + expect(childrenMock.mock.calls[2][0].pagination).toEqual({ + page: 10, + perPage: 100, + }); + }); + + it('should provide setPagination to setPaginationState', () => { + const { childrenProps, childrenMock } = renderHook(() => usePaginationState()); + expect(childrenProps.pagination).toEqual({ page: 1, perPage: 25 }); + + act(() => childrenProps.setPagination({ perPage: 100, page: 20 })); + + expect(childrenMock).toBeCalledTimes(2); + + expect(childrenMock.mock.calls[1][0].pagination).toEqual({ + page: 20, + perPage: 100, + }); + }); + + it('should provide setPage to set only the page', () => { + const { childrenProps, childrenMock } = renderHook(() => usePaginationState()); + expect(childrenProps.pagination).toEqual({ page: 1, perPage: 25 }); + + act(() => childrenProps.setPage(20)); + + expect(childrenMock).toBeCalledTimes(2); + + expect(childrenMock.mock.calls[1][0].pagination).toEqual({ + page: 20, + perPage: 25, + }); + }); + + it('should provide setPerPage to set only perPage', () => { + const { childrenProps, childrenMock } = renderHook(() => usePaginationState()); + expect(childrenProps.pagination).toEqual({ page: 1, perPage: 25 }); + + act(() => childrenProps.setPerPage(100)); + + expect(childrenMock).toBeCalledTimes(2); + + expect(childrenMock.mock.calls[1][0].pagination).toEqual({ + page: 1, + perPage: 100, + }); + }); +}); diff --git a/packages/ra-core/src/controller/usePaginationState.ts b/packages/ra-core/src/controller/usePaginationState.ts index 53e80df70af..72e4ab35c84 100644 --- a/packages/ra-core/src/controller/usePaginationState.ts +++ b/packages/ra-core/src/controller/usePaginationState.ts @@ -1,4 +1,4 @@ -import { useState, useEffect } from 'react'; +import { useState, useEffect, useReducer, useMemo, useRef } from 'react'; import { Pagination } from '../types'; interface PaginationProps { @@ -10,6 +10,18 @@ interface PaginationProps { setPagination: (pagination: Pagination) => void; } +const paginationReducer = (prevState, nextState) => { + return { + ...prevState, + ...nextState, + }; +}; + +const defaultPagination = { + page: 1, + perPage: 25, +}; + /** * set the sort to the given field, swap the order if the field is the same * @name setNumber @@ -36,24 +48,28 @@ interface PaginationProps { * @param {numper} initialPerPage the initial value per page * @returns {PaginationProps} The pagination props */ -export default (initialPerPage: number = 25): PaginationProps => { - const [page, setPage] = useState(1); - const [perPage, setPerPage] = useState(initialPerPage); +export default (initialPagination: { perPage?: number, page?: number } = {}): PaginationProps => { + const [pagination, setPagination] = useReducer(paginationReducer, { + ...defaultPagination, + ...initialPagination, + }); + const isFirstRender = useRef(true); - const setPagination = (pagination: Pagination) => { - setPage(pagination.page); - setPerPage(pagination.perPage); - }; + const setPerPage = useMemo(() => perPage => setPagination({ perPage }), []); + const setPage = useMemo(() => page => setPagination({ page }), []); - useEffect(() => setPerPage(initialPerPage), [initialPerPage]); + useEffect(() => { + if (isFirstRender.current) { + isFirstRender.current = false; + return; + } + setPerPage(initialPagination.perPage || 25); + }, [initialPagination.perPage || 25]); return { - page, - perPage, - pagination: { - page, - perPage, - }, + page: pagination.page, + perPage: pagination.perPage, + pagination, setPage, setPerPage, setPagination, diff --git a/packages/ra-core/src/util/renderHook.tsx b/packages/ra-core/src/util/renderHook.tsx new file mode 100644 index 00000000000..5b50c1ea254 --- /dev/null +++ b/packages/ra-core/src/util/renderHook.tsx @@ -0,0 +1,26 @@ +import React from 'react'; +import { render } from 'react-testing-library'; + +const TestHook = ({ children, hookProps }) => { + return children(hookProps()); +}; +export default (hookProps) => { + let childrenProps = null; + const children = props => { + childrenProps = props; + return

child

; + }; + const childrenMock = jest.fn().mockImplementation(children); + const result = render( + , + ); + + return { + ...result, + childrenProps, + childrenMock, + rerender: (newHook) => { + result.rerender() + } + }; +}; diff --git a/packages/ra-ui-materialui/src/field/ReferenceManyField.js b/packages/ra-ui-materialui/src/field/ReferenceManyField.js index 54b852edf1f..e9a48e32d8e 100644 --- a/packages/ra-ui-materialui/src/field/ReferenceManyField.js +++ b/packages/ra-ui-materialui/src/field/ReferenceManyField.js @@ -124,9 +124,9 @@ export const ReferenceManyField = props => { ); } const { sort, setSort } = useSortState(initialSort); - const { page, perPage, setPage, setPerPage } = usePaginationState( - initialPerPage - ); + const { page, perPage, setPage, setPerPage } = usePaginationState({ + page: initialPerPage, + }); const useReferenceManyProps = useReferenceMany({ resource, From 96ad01f0f13c463dc0a584c7cba8f778a089d4f0 Mon Sep 17 00:00:00 2001 From: thiery Date: Wed, 19 Jun 2019 17:58:25 +0200 Subject: [PATCH 13/48] move hooks helper in util and use UsePrevious and useDeepCompareEffect from it --- .../controller/input/useMatchingReferences.ts | 25 +++++++++++-------- packages/ra-core/src/util/usePrevious.ts | 10 -------- 2 files changed, 14 insertions(+), 21 deletions(-) delete mode 100644 packages/ra-core/src/util/usePrevious.ts diff --git a/packages/ra-core/src/controller/input/useMatchingReferences.ts b/packages/ra-core/src/controller/input/useMatchingReferences.ts index fdbf1cc9c34..031e3c4c781 100644 --- a/packages/ra-core/src/controller/input/useMatchingReferences.ts +++ b/packages/ra-core/src/controller/input/useMatchingReferences.ts @@ -1,8 +1,7 @@ -import { useEffect, useMemo } from 'react'; +import { useMemo } from 'react'; // @ts-ignore import { useSelector, useDispatch } from 'react-redux'; import { createSelector } from 'reselect'; -import isEqual from 'lodash.isequal'; import { Filter } from '../useFilterState'; import { crudGetMatchingAccumulate } from '../../actions/accumulateActions'; @@ -12,7 +11,7 @@ import { getReferenceResource, } from '../../reducer'; import { Pagination, Sort, Record } from '../../types'; -import usePrevious from '../../util/usePrevious'; +import { useDeepCompareEffect } from '../../util/hooks'; interface UseMAtchingReferencesOption { reference: string; @@ -57,14 +56,8 @@ export default ({ sort, }; - const previousOption = usePrevious(options); - - useEffect( + useDeepCompareEffect( () => { - if (isEqual(options, previousOption)) { - return; - } - fetchOptions({ dispatch, filter, @@ -75,7 +68,17 @@ export default ({ pagination, sort, }) - } + }, + [ + dispatch, + filter, + reference, + referenceSource, + resource, + source, + pagination, + sort, + ] ); const matchingReferences = useSelector( diff --git a/packages/ra-core/src/util/usePrevious.ts b/packages/ra-core/src/util/usePrevious.ts deleted file mode 100644 index d9a08a677c0..00000000000 --- a/packages/ra-core/src/util/usePrevious.ts +++ /dev/null @@ -1,10 +0,0 @@ -import { useRef, useEffect } from "react"; - -export default value => { - const ref = useRef(); - useEffect(() => { - ref.current = value; - }, [ value]); - - return ref.current; -} \ No newline at end of file From a55ca2f69bb9e55156ed88cb2315de6afde023a6 Mon Sep 17 00:00:00 2001 From: thiery Date: Wed, 19 Jun 2019 18:42:02 +0200 Subject: [PATCH 14/48] add test on useSortState --- .../src/controller/useSortState.spec.ts | 69 +++++++++++++++++++ .../ra-core/src/controller/useSortState.ts | 8 ++- 2 files changed, 74 insertions(+), 3 deletions(-) create mode 100644 packages/ra-core/src/controller/useSortState.spec.ts diff --git a/packages/ra-core/src/controller/useSortState.spec.ts b/packages/ra-core/src/controller/useSortState.spec.ts new file mode 100644 index 00000000000..6508a35942e --- /dev/null +++ b/packages/ra-core/src/controller/useSortState.spec.ts @@ -0,0 +1,69 @@ +import renderHook from '../util/renderHook'; +import useSortState, { defaultSort } from './useSortState'; +import { act } from 'react-testing-library'; + +describe('useSortState', () => { + it('should initialize sortState with default sort', () => { + const { childrenProps } = renderHook(() => useSortState()); + + expect(childrenProps.sort).toEqual(defaultSort); + }); + + it('should initialize sortState with given sort', () => { + const { childrenProps } = renderHook(() => useSortState({ + field: 'name', + order: 'ASC', + })); + + expect(childrenProps.sort).toEqual({ field: 'name', order: 'ASC' }); + }); + + it('should provide setSort method to change the whole sort', () => { + const { childrenProps, childrenMock } = renderHook( + () => useSortState({ field: 'id', order: 'DESC' }) + ); + + expect(childrenProps.sort).toEqual({ field: 'id', order: 'DESC' }); + + act(() => childrenProps.setSort({ field: 'name', order: 'ASC' })); + expect(childrenMock.mock.calls[1][0].sort).toEqual({ + field: 'name', + order: 'ASC' + }); + }); + + describe('should provide setSortField method that', () => { + it('should just change the order if receiving the current field', () => { + const { childrenProps, childrenMock } = renderHook( + () => useSortState({ field: 'id', order: 'DESC' }) + ); + + expect(childrenProps.sort).toEqual({ field: 'id', order: 'DESC' }); + + act(() => childrenProps.setSortField('id')); + expect(childrenMock.mock.calls[1][0].sort).toEqual({ + field: 'id', + order: 'ASC' + }); + }); + + it('should change the field and set the order to DESC if receiving another field', () => { + const { childrenProps, childrenMock } = renderHook( + () => useSortState({ field: 'id', order: 'ASC' }) + ); + + expect(childrenProps.sort).toEqual({ field: 'id', order: 'ASC' }); + + act(() => childrenProps.setSortField('name')); + expect(childrenMock.mock.calls[1][0].sort).toEqual({ + field: 'name', + order: 'DESC' + }); + act(() => childrenProps.setSortField('id')); + expect(childrenMock.mock.calls[2][0].sort).toEqual({ + field: 'id', + order: 'DESC' + }); + }); + }) +}); diff --git a/packages/ra-core/src/controller/useSortState.ts b/packages/ra-core/src/controller/useSortState.ts index 3b634b5a667..5af2db59e4b 100644 --- a/packages/ra-core/src/controller/useSortState.ts +++ b/packages/ra-core/src/controller/useSortState.ts @@ -17,12 +17,14 @@ const sortReducer = (state: Sort, field: string | Sort): Sort => { return field; } const order = - state.field === field && state.order === SORT_ASC + state.field === field ? state.order === SORT_ASC ? SORT_DESC - : SORT_ASC; + : SORT_ASC : SORT_ASC; return { field, order }; }; +export const defaultSort = { field: 'id', order: 'DESC' }; + /** * set the sort to the given field, swap the order if the field is the same * @name setSort @@ -52,7 +54,7 @@ const sortReducer = (state: Sort, field: string | Sort): Sort => { * @returns {SortProps} The sort props */ export default ( - initialSort: Sort = { field: 'id', order: 'DESC' } + initialSort: Sort = defaultSort ): SortProps => { const [sort, dispatch] = useReducer(sortReducer, initialSort); const isFirstRender = useRef(true); From f152f8aec4566e30ceadc712c9478d28e4d4396a Mon Sep 17 00:00:00 2001 From: thiery Date: Tue, 25 Jun 2019 18:56:02 +0200 Subject: [PATCH 15/48] add test on useFilterState --- .../src/controller/useFilterState.spec.ts | 73 +++++++++++++++++++ .../ra-core/src/controller/useFilterState.ts | 11 ++- 2 files changed, 80 insertions(+), 4 deletions(-) create mode 100644 packages/ra-core/src/controller/useFilterState.spec.ts diff --git a/packages/ra-core/src/controller/useFilterState.spec.ts b/packages/ra-core/src/controller/useFilterState.spec.ts new file mode 100644 index 00000000000..93968e9a08c --- /dev/null +++ b/packages/ra-core/src/controller/useFilterState.spec.ts @@ -0,0 +1,73 @@ +import renderHook from '../util/renderHook'; +import useFilterState from './useFilterState'; +import { act } from 'react-testing-library'; + +describe('useFilterState', () => { + it('should initialize filterState with default filter', () => { + const { childrenProps } = renderHook(() => useFilterState({})); + + expect(childrenProps.filter).toEqual({ q: '' }); + }); + + it('should initialize filterState with permanent filter', () => { + const { childrenProps } = renderHook(() => + useFilterState({ permanentFilter: { type: 'thisOne' } }) + ); + + expect(childrenProps.filter).toEqual({ q: '', type: 'thisOne' }); + }); + + it('should initialize using filterToQuery if provided', () => { + const { childrenProps } = renderHook(() => + useFilterState({ filterToQuery: v => ({ search: v }) }) + ); + + expect(childrenProps.filter).toEqual({ search: '' }); + }); + + it('should provide setFilter to update filter value after given debounceTime', async () => { + const { childrenProps, childrenMock } = renderHook(() => + useFilterState({ debounceTime: 10 }) + ); + + expect(childrenProps.filter).toEqual({ q: '' }); + + act(() => childrenProps.setFilter('needle in a haystack')); + + expect(childrenMock).toBeCalledTimes(1); + await new Promise(resolve => setTimeout(resolve, 10)); + + expect(childrenMock).toBeCalledTimes(2); + + expect(childrenMock.mock.calls[1][0].filter).toEqual({ + q: 'needle in a haystack', + }); + }); + + it('should provide setFilter to update filter value after given debounceTime preserving permanentFilter and filterToQuery', async () => { + const { childrenProps, childrenMock } = renderHook(() => + useFilterState({ + debounceTime: 10, + permanentFilter: { type: 'thisOne' }, + filterToQuery: v => ({ search: v }), + }) + ); + + act(() => childrenProps.setFilter('needle in a haystack')); + + expect(childrenMock).toBeCalledTimes(1); + await new Promise(resolve => setTimeout(resolve, 10)); + + expect(childrenMock).toBeCalledTimes(2); + + expect(childrenMock.mock.calls[0][0].filter).toEqual({ + type: 'thisOne', + search: '', + }); + + expect(childrenMock.mock.calls[1][0].filter).toEqual({ + type: 'thisOne', + search: 'needle in a haystack', + }); + }); +}); diff --git a/packages/ra-core/src/controller/useFilterState.ts b/packages/ra-core/src/controller/useFilterState.ts index 984028a020c..03953914243 100644 --- a/packages/ra-core/src/controller/useFilterState.ts +++ b/packages/ra-core/src/controller/useFilterState.ts @@ -6,8 +6,8 @@ export interface Filter { } interface UseFilterStateOptions { - filterToQuery: (v: string) => Filter; - permanentFilter: Filter; + filterToQuery?: (v: string) => Filter; + permanentFilter?: Filter; debounceTime?: number; } @@ -18,10 +18,13 @@ interface UseFilterStateProps { export default ({ filterToQuery = v => ({ q: v }), - permanentFilter, + permanentFilter = {}, debounceTime = 500, }: UseFilterStateOptions): UseFilterStateProps => { - const [filter, setFilterValue] = useState(permanentFilter); + const [filter, setFilterValue] = useState({ + ...permanentFilter, + ...filterToQuery(''), + }); const setFilter = debounce( value => From 970ebe87c6dd07ac1c64937bb3af3ae09ac99b62 Mon Sep 17 00:00:00 2001 From: thiery Date: Sat, 29 Jun 2019 21:21:10 +0200 Subject: [PATCH 16/48] update ReferenceInputCOntroller test --- .../input/ReferenceInputController.spec.tsx | 464 ++---------------- 1 file changed, 29 insertions(+), 435 deletions(-) diff --git a/packages/ra-core/src/controller/input/ReferenceInputController.spec.tsx b/packages/ra-core/src/controller/input/ReferenceInputController.spec.tsx index 000fec34a84..f0827a1a9b2 100644 --- a/packages/ra-core/src/controller/input/ReferenceInputController.spec.tsx +++ b/packages/ra-core/src/controller/input/ReferenceInputController.spec.tsx @@ -5,8 +5,6 @@ import omit from 'lodash/omit'; import renderWithRedux from '../../util/renderWithRedux'; import ReferenceInputController from './ReferenceInputController'; -import { crudGetMatchingAccumulate } from '../../actions'; -import * as referenceDataStatus from './referenceDataStatus'; describe('', () => { const defaultProps = { @@ -23,455 +21,51 @@ describe('', () => { afterEach(cleanup); - // it('should fetch reference and matchingReferences', () => { - // const children = jest.fn().mockReturnValue(

child

); - // const { dispatch } = renderWithRedux( - // - // {children} - // , - // { - // admin: { - // resources: { posts: { data: { 1: { id: 1 } } } }, - // references: { - // possibleValues: { 'comments@post_id': [2, 1] }, - // }, - // }, - // } - // ); - - // expect( - // omit(children.mock.calls[0][0], [ - // 'onChange', - // 'setPagination', - // 'setFilter', - // 'setSort', - // ]) - // ).toEqual({ - // choices: [{ id: 1 }], - // error: null, - // filter: {}, - // isLoading: false, - // pagination: { page: 1, perPage: 25 }, - // sort: { field: 'id', order: 'DESC' }, - // warning: null, - // }); - - // expect(dispatch).toBeCalledTimes(2); - // expect(dispatch.mock.calls[0][0].type).toBe( - // 'RA/CRUD_GET_MATCHING_ACCUMULATE' - // ); - // expect(dispatch.mock.calls[1][0].type).toBe( - // 'RA/CRUD_GET_MANY_ACCUMULATE' - // ); - // }); - - // it('should set isLoading to true if the references are being searched and there is no reference already selected', () => { - // const children = jest.fn().mockReturnValue(

child

); - // renderWithRedux( - // - // {children} - // , - // { - // admin: { - // resources: { posts: { data: { 1: { id: 1 } } } }, - // references: { - // possibleValues: { 'comments@post_id': null }, - // }, - // }, - // } - // ); - // expect(children.mock.calls[0][0].isLoading).toBe(true); - // }); - - // it('should set isLoading to false if the references are being searched but a selected reference have data', () => { - // const children = jest.fn().mockReturnValue(

child

); - // renderWithRedux( - // - // {children} - // , - // { - // admin: { - // resources: { posts: { data: { 1: { id: 1 } } } }, - // references: { - // possibleValues: { 'comments@post_id': null }, - // }, - // }, - // } - // ); - // expect(children.mock.calls[0][0].isLoading).toBe(false); - // expect(children.mock.calls[0][0].choices).toEqual([{ id: 1 }]); - // }); - - // it('should set isLoading to false if the references were found but a selected reference does not have data', () => { - // const children = jest.fn().mockReturnValue(

child

); - // renderWithRedux( - // - // {children} - // , - // { - // admin: { - // resources: { - // posts: { data: { 1: { id: 1 }, 2: { id: 2 } } }, - // }, - // references: { - // possibleValues: { 'comments@post_id': [2, 1, 3] }, - // }, - // }, - // } - // ); - // expect(children.mock.calls[0][0].isLoading).toBe(false); - // expect(children.mock.calls[0][0].choices).toEqual([ - // { id: 2 }, - // { id: 1 }, - // ]); - // }); - - it('should call getDataStatus with referenceRecord and matchingReferences', () => { + it('should fetch reference matchingReferences, and provice filter pagination and sort', () => { const children = jest.fn().mockReturnValue(

child

); - const spy = jest.spyOn(referenceDataStatus, 'getStatusForInput'); - renderWithRedux( + const { dispatch } = renderWithRedux( {children} , { admin: { - resources: { - posts: { - data: { 5: { id: 5 } }, - }, - }, + resources: { posts: { data: { 1: { id: 1 } } } }, references: { - possibleValues: { - 'comments@post_id': [2, 1], - }, + possibleValues: { 'comments@post_id': [2, 1] }, }, }, } ); - expect(spy.mock.calls[0][0].input).toEqual({ value: 5 }); - expect(spy.mock.calls[0][0].matchingReferences).toEqual([2, 1]); - expect(spy.mock.calls[0][0].referenceRecord).toEqual({ id: 5 }); - }); - it('should call useReferenceSearch', () => { - // const crudGetMatchingAccumulate = jest.fn(); - const { dispatch } = renderWithRedux( - + expect( + omit(children.mock.calls[0][0], [ + 'onChange', + 'setPagination', + 'setFilter', + 'setSort', + ]) + ).toEqual({ + choices: [{ id: 1 }], + error: null, + filter: { q: '' }, + isLoading: false, + pagination: { page: 1, perPage: 25 }, + sort: { field: 'id', order: 'DESC' }, + warning: null, + }); + + expect(dispatch).toBeCalledTimes(2); + expect(dispatch.mock.calls[0][0].type).toBe( + 'RA/CRUD_GET_MATCHING_ACCUMULATE' + ); + expect(dispatch.mock.calls[1][0].type).toBe( + 'RA/CRUD_GET_MANY_ACCUMULATE' ); - assert.deepEqual(crudGetMatchingAccumulate.mock.calls[0], [ - 'posts', - 'comments@post_id', - { - page: 1, - perPage: 25, - }, - { - field: 'id', - order: 'DESC', - }, - {}, - ]); }); - - // it('should allow to customize crudGetMatchingAccumulate arguments with perPage, sort, and filter props', () => { - // const crudGetMatchingAccumulate = jest.fn(); - // renderWithRedux( - // - // ); - // assert.deepEqual(crudGetMatchingAccumulate.mock.calls[0], [ - // 'posts', - // 'comments@post_id', - // { - // page: 1, - // perPage: 5, - // }, - // { - // field: 'foo', - // order: 'ASC', - // }, - // { - // q: 'foo', - // }, - // ]); - // }); - - // it('should allow to customize crudGetMatchingAccumulate arguments with perPage, sort, and filter props without loosing original default filter', () => { - // const crudGetMatchingAccumulate = jest.fn(); - // const wrapper = renderWithRedux( - // - // ); - - // wrapper.instance().setFilter('search_me'); - - // assert.deepEqual(crudGetMatchingAccumulate.mock.calls[1], [ - // 'posts', - // 'comments@post_id', - // { - // page: 1, - // perPage: 5, - // }, - // { - // field: 'foo', - // order: 'ASC', - // }, - // { - // foo: 'bar', - // q: 'search_me', - // }, - // ]); - // }); - - // it('should call crudGetMatchingAccumulate when setFilter is called', () => { - // const crudGetMatchingAccumulate = jest.fn(); - // const wrapper = renderWithRedux( - // - // ); - // wrapper.instance().setFilter('bar'); - // assert.deepEqual(crudGetMatchingAccumulate.mock.calls[1], [ - // 'posts', - // 'comments@post_id', - // { - // page: 1, - // perPage: 25, - // }, - // { - // field: 'id', - // order: 'DESC', - // }, - // { - // q: 'bar', - // }, - // ]); - // }); - - // it('should use custom filterToQuery function prop', () => { - // const crudGetMatchingAccumulate = jest.fn(); - // const wrapper = renderWithRedux( - // ({ foo: searchText })} - // /> - // ); - // wrapper.instance().setFilter('bar'); - // assert.deepEqual(crudGetMatchingAccumulate.mock.calls[1], [ - // 'posts', - // 'comments@post_id', - // { - // page: 1, - // perPage: 25, - // }, - // { - // field: 'id', - // order: 'DESC', - // }, - // { - // foo: 'bar', - // }, - // ]); - // }); - - // it('should call crudGetManyAccumulate on mount if value is set', () => { - // const crudGetManyAccumulate = jest.fn(); - // renderWithRedux( - // - // ); - // assert.deepEqual(crudGetManyAccumulate.mock.calls[0], ['posts', [5]]); - // }); - - // it('should pass onChange down to child component', () => { - // const children = jest.fn().mockReturnValue(

child

); - // const onChange = jest.fn(); - // renderWithRedux( - // - // {children} - // - // ); - // assert.deepEqual(children.mock.calls[0][0].onChange, onChange); - // }); - - // it('should only call crudGetMatchingAccumulate when calling setFilter', () => { - // const crudGetMatchingAccumulate = jest.fn(); - // const crudGetManyAccumulate = jest.fn(); - // const wrapper = renderWithRedux( - // - // ); - // assert.equal(crudGetMatchingAccumulate.mock.calls.length, 1); - // assert.equal(crudGetManyAccumulate.mock.calls.length, 1); - - // wrapper.instance().setFilter('bar'); - // assert.equal(crudGetMatchingAccumulate.mock.calls.length, 2); - // assert.equal(crudGetManyAccumulate.mock.calls.length, 1); - // }); - - // it('should only call crudGetMatching when props are changed from outside', () => { - // const crudGetMatchingAccumulate = jest.fn(); - // const crudGetManyAccumulate = jest.fn(); - // const ControllerWrapper = props => ( - // - // {() => null} - // - // ); - - // const { rerender } = render(); - // assert.equal(crudGetMatchingAccumulate.mock.calls.length, 1); - // assert.equal(crudGetManyAccumulate.mock.calls.length, 1); - - // rerender(); - - // assert.equal(crudGetManyAccumulate.mock.calls.length, 1); - // assert.deepEqual(crudGetMatchingAccumulate.mock.calls[1], [ - // 'posts', - // 'comments@post_id', - // { page: 1, perPage: 25 }, - // { field: 'id', order: 'DESC' }, - // { foo: 'bar' }, - // ]); - - // rerender( - // - // ); - - // assert.equal(crudGetManyAccumulate.mock.calls.length, 1); - // assert.deepEqual(crudGetMatchingAccumulate.mock.calls[2], [ - // 'posts', - // 'comments@post_id', - // { page: 1, perPage: 25 }, - // { field: 'foo', order: 'ASC' }, - // { foo: 'bar' }, - // ]); - - // rerender( - // - // ); - - // assert.equal(crudGetManyAccumulate.mock.calls.length, 1); - // assert.deepEqual(crudGetMatchingAccumulate.mock.calls[3], [ - // 'posts', - // 'comments@post_id', - // { page: 1, perPage: 42 }, - // { field: 'foo', order: 'ASC' }, - // { foo: 'bar' }, - // ]); - // }); - - // it('should only call crudGetMatchingAccumulate when props are changed from outside', () => { - // const crudGetMatchingAccumulate = jest.fn(); - // const crudGetManyAccumulate = jest.fn(); - // const wrapper = renderWithRedux( - // - // ); - // expect(crudGetMatchingAccumulate).toHaveBeenCalledTimes(1); - // expect(crudGetManyAccumulate).toHaveBeenCalledTimes(1); - - // wrapper.setProps({ filter: { foo: 'bar' } }); - // expect(crudGetMatchingAccumulate.mock.calls.length).toBe(2); - // expect(crudGetManyAccumulate).toHaveBeenCalledTimes(1); - - // wrapper.setProps({ sort: { field: 'foo', order: 'ASC' } }); - // expect(crudGetMatchingAccumulate.mock.calls.length).toBe(3); - // expect(crudGetManyAccumulate).toHaveBeenCalledTimes(1); - - // wrapper.setProps({ perPage: 42 }); - // expect(crudGetMatchingAccumulate.mock.calls.length).toBe(4); - // expect(crudGetManyAccumulate).toHaveBeenCalledTimes(1); - // }); - - // it('should call crudGetManyAccumulate when input value changes', () => { - // const crudGetManyAccumulate = jest.fn(); - // const wrapper = renderWithRedux( - // - // ); - // assert.equal(crudGetManyAccumulate.mock.calls.length, 1); - // wrapper.setProps({ input: { value: 6 } }); - // assert.equal(crudGetManyAccumulate.mock.calls.length, 2); - // }); - - // it('should call crudGetManyAccumulate and crudGetMatchingAccumulate when record changes', () => { - // const crudGetManyAccumulate = jest.fn(); - // const crudGetMatchingAccumulate = jest.fn(); - // const wrapper = renderWithRedux( - // - // ); - // assert.equal(crudGetMatchingAccumulate.mock.calls.length, 1); - // assert.equal(crudGetManyAccumulate.mock.calls.length, 1); - // wrapper.setProps({ record: { id: 1 } }); - // assert.equal(crudGetMatchingAccumulate.mock.calls.length, 2); - // assert.equal(crudGetManyAccumulate.mock.calls.length, 2); - // }); }); From 9513bd8ff72f558568223a1759ea7f768f287925 Mon Sep 17 00:00:00 2001 From: thiery Date: Thu, 4 Jul 2019 13:43:45 +0200 Subject: [PATCH 17/48] fix test --- .../field/ReferenceManyFieldController.tsx | 8 +-- .../src/controller/useSortState.spec.ts | 50 ++++++++++--------- 2 files changed, 31 insertions(+), 27 deletions(-) diff --git a/packages/ra-core/src/controller/field/ReferenceManyFieldController.tsx b/packages/ra-core/src/controller/field/ReferenceManyFieldController.tsx index 2fa352a697f..25db5f5fd56 100644 --- a/packages/ra-core/src/controller/field/ReferenceManyFieldController.tsx +++ b/packages/ra-core/src/controller/field/ReferenceManyFieldController.tsx @@ -33,6 +33,8 @@ interface Props { total?: number; } +const defaultPerPage = 25; + /** * Render related records to the current one. * @@ -92,9 +94,9 @@ export const ReferenceManyFieldController: FunctionComponent = ({ children, }) => { const { sort, setSortField } = useSortState(initialSort); - const { page, perPage, setPage, setPerPage } = usePaginationState( - { perPage: initialPerPage } - ); + const { page, perPage, setPage, setPerPage } = usePaginationState({ + perPage: initialPerPage || defaultPerPage, + }); const { data, ids, diff --git a/packages/ra-core/src/controller/useSortState.spec.ts b/packages/ra-core/src/controller/useSortState.spec.ts index 6508a35942e..df6d13e4aa4 100644 --- a/packages/ra-core/src/controller/useSortState.spec.ts +++ b/packages/ra-core/src/controller/useSortState.spec.ts @@ -10,60 +10,62 @@ describe('useSortState', () => { }); it('should initialize sortState with given sort', () => { - const { childrenProps } = renderHook(() => useSortState({ - field: 'name', - order: 'ASC', - })); + const { childrenProps } = renderHook(() => + useSortState({ + field: 'name', + order: 'ASC', + }) + ); expect(childrenProps.sort).toEqual({ field: 'name', order: 'ASC' }); }); it('should provide setSort method to change the whole sort', () => { - const { childrenProps, childrenMock } = renderHook( - () => useSortState({ field: 'id', order: 'DESC' }) + const { childrenProps, childrenMock } = renderHook(() => + useSortState({ field: 'id', order: 'DESC' }) ); expect(childrenProps.sort).toEqual({ field: 'id', order: 'DESC' }); act(() => childrenProps.setSort({ field: 'name', order: 'ASC' })); expect(childrenMock.mock.calls[1][0].sort).toEqual({ - field: 'name', - order: 'ASC' + field: 'name', + order: 'ASC', }); }); describe('should provide setSortField method that', () => { it('should just change the order if receiving the current field', () => { - const { childrenProps, childrenMock } = renderHook( - () => useSortState({ field: 'id', order: 'DESC' }) + const { childrenProps, childrenMock } = renderHook(() => + useSortState({ field: 'id', order: 'DESC' }) ); - + expect(childrenProps.sort).toEqual({ field: 'id', order: 'DESC' }); - + act(() => childrenProps.setSortField('id')); expect(childrenMock.mock.calls[1][0].sort).toEqual({ - field: 'id', - order: 'ASC' + field: 'id', + order: 'ASC', }); }); - - it('should change the field and set the order to DESC if receiving another field', () => { - const { childrenProps, childrenMock } = renderHook( - () => useSortState({ field: 'id', order: 'ASC' }) + + it('should change the field and set the order to ASC if receiving another field', () => { + const { childrenProps, childrenMock } = renderHook(() => + useSortState({ field: 'id', order: 'ASC' }) ); - + expect(childrenProps.sort).toEqual({ field: 'id', order: 'ASC' }); - + act(() => childrenProps.setSortField('name')); expect(childrenMock.mock.calls[1][0].sort).toEqual({ - field: 'name', - order: 'DESC' + field: 'name', + order: 'ASC', }); act(() => childrenProps.setSortField('id')); expect(childrenMock.mock.calls[2][0].sort).toEqual({ field: 'id', - order: 'DESC' + order: 'ASC', }); }); - }) + }); }); From 64e2d244faf48cb1b696de45a689679c6fc1cd47 Mon Sep 17 00:00:00 2001 From: thiery Date: Thu, 4 Jul 2019 20:08:09 +0200 Subject: [PATCH 18/48] prettier --- .../field/ReferenceFieldController.tsx | 5 +- .../input/useMatchingReferences.spec.tsx | 345 ++++++++++-------- .../controller/input/useMatchingReferences.ts | 31 +- .../src/controller/usePaginationState.spec.ts | 23 +- .../src/controller/usePaginationState.ts | 6 +- .../src/controller/useReference.spec.ts | 32 +- .../ra-core/src/controller/useReference.ts | 4 +- .../ra-core/src/controller/useSortState.ts | 12 +- packages/ra-core/src/util/TestContext.tsx | 2 +- packages/ra-core/src/util/renderHook.tsx | 20 +- .../ra-core/src/util/renderHookWithRedux.tsx | 14 +- .../src/field/ReferenceField.js | 4 +- 12 files changed, 275 insertions(+), 223 deletions(-) diff --git a/packages/ra-core/src/controller/field/ReferenceFieldController.tsx b/packages/ra-core/src/controller/field/ReferenceFieldController.tsx index 2a67fccefc6..e294b63c55d 100644 --- a/packages/ra-core/src/controller/field/ReferenceFieldController.tsx +++ b/packages/ra-core/src/controller/field/ReferenceFieldController.tsx @@ -1,7 +1,10 @@ import { FunctionComponent, ReactNode, ReactElement } from 'react'; import { Record } from '../../types'; -import useReferenceField, { UseReferenceProps, LinkToFunctionType } from './useReferenceField'; +import useReferenceField, { + UseReferenceProps, + LinkToFunctionType, +} from './useReferenceField'; interface Props { allowEmpty?: boolean; diff --git a/packages/ra-core/src/controller/input/useMatchingReferences.spec.tsx b/packages/ra-core/src/controller/input/useMatchingReferences.spec.tsx index 07736fa5055..dce66d549ae 100644 --- a/packages/ra-core/src/controller/input/useMatchingReferences.spec.tsx +++ b/packages/ra-core/src/controller/input/useMatchingReferences.spec.tsx @@ -32,17 +32,17 @@ describe('useMatchingReferences', () => { ).toEqual({ resource: 'posts', relatedTo: 'comments@post_id', - pagination:{ - perPage:25, - page:1 + pagination: { + perPage: 25, + page: 1, }, - sort:{ + sort: { field: 'id', - order: 'DESC' + order: 'DESC', + }, + filter: { + q: '', }, - filter:{ - q: '' - } }); rerender(() => { @@ -76,23 +76,27 @@ describe('useMatchingReferences', () => { ).toEqual({ resource: 'posts', relatedTo: 'comments@post_id', - pagination:{ - perPage:25, - page:1 + pagination: { + perPage: 25, + page: 1, }, - sort:{ + sort: { field: 'id', - order: 'DESC' + order: 'DESC', + }, + filter: { + q: '', }, - filter:{ - q: '' - } }); rerender(() => { - return useMatchingReferences({...defaultProps, filter: { q: 'typing' } }); + return useMatchingReferences({ + ...defaultProps, + filter: { q: 'typing' }, + }); }); - expect(dispatch).toBeCalledTimes(2);expect(dispatch.mock.calls[1][0].type).toBe( + expect(dispatch).toBeCalledTimes(2); + expect(dispatch.mock.calls[1][0].type).toBe( 'RA/CRUD_GET_MATCHING_ACCUMULATE' ); expect( @@ -100,17 +104,17 @@ describe('useMatchingReferences', () => { ).toEqual({ resource: 'posts', relatedTo: 'comments@post_id', - pagination:{ - perPage:25, - page:1 + pagination: { + perPage: 25, + page: 1, }, - sort:{ + sort: { field: 'id', - order: 'DESC' + order: 'DESC', + }, + filter: { + q: 'typing', }, - filter:{ - q: 'typing' - } }); }); @@ -128,23 +132,27 @@ describe('useMatchingReferences', () => { ).toEqual({ resource: 'posts', relatedTo: 'comments@post_id', - pagination:{ - perPage:25, - page:1 + pagination: { + perPage: 25, + page: 1, }, - sort:{ + sort: { field: 'id', - order: 'DESC' + order: 'DESC', + }, + filter: { + q: '', }, - filter:{ - q: '' - } }); rerender(() => { - return useMatchingReferences({...defaultProps, reference: 'blog_posts' }); + return useMatchingReferences({ + ...defaultProps, + reference: 'blog_posts', + }); }); - expect(dispatch).toBeCalledTimes(2);expect(dispatch.mock.calls[1][0].type).toBe( + expect(dispatch).toBeCalledTimes(2); + expect(dispatch.mock.calls[1][0].type).toBe( 'RA/CRUD_GET_MATCHING_ACCUMULATE' ); expect( @@ -152,17 +160,17 @@ describe('useMatchingReferences', () => { ).toEqual({ resource: 'blog_posts', relatedTo: 'comments@post_id', - pagination:{ - perPage:25, - page:1 + pagination: { + perPage: 25, + page: 1, }, - sort:{ + sort: { field: 'id', - order: 'DESC' + order: 'DESC', + }, + filter: { + q: '', }, - filter:{ - q: '' - } }); }); @@ -180,23 +188,24 @@ describe('useMatchingReferences', () => { ).toEqual({ resource: 'posts', relatedTo: 'comments@post_id', - pagination:{ - perPage:25, - page:1 + pagination: { + perPage: 25, + page: 1, }, - sort:{ + sort: { field: 'id', - order: 'DESC' + order: 'DESC', + }, + filter: { + q: '', }, - filter:{ - q: '' - } }); rerender(() => { - return useMatchingReferences({...defaultProps, resource: 'note' }); + return useMatchingReferences({ ...defaultProps, resource: 'note' }); }); - expect(dispatch).toBeCalledTimes(2);expect(dispatch.mock.calls[1][0].type).toBe( + expect(dispatch).toBeCalledTimes(2); + expect(dispatch.mock.calls[1][0].type).toBe( 'RA/CRUD_GET_MATCHING_ACCUMULATE' ); expect( @@ -204,17 +213,17 @@ describe('useMatchingReferences', () => { ).toEqual({ resource: 'posts', relatedTo: 'note@post_id', - pagination:{ - perPage:25, - page:1 + pagination: { + perPage: 25, + page: 1, }, - sort:{ + sort: { field: 'id', - order: 'DESC' + order: 'DESC', + }, + filter: { + q: '', }, - filter:{ - q: '' - } }); }); @@ -232,23 +241,27 @@ describe('useMatchingReferences', () => { ).toEqual({ resource: 'posts', relatedTo: 'comments@post_id', - pagination:{ - perPage:25, - page:1 + pagination: { + perPage: 25, + page: 1, }, - sort:{ + sort: { field: 'id', - order: 'DESC' + order: 'DESC', + }, + filter: { + q: '', }, - filter:{ - q: '' - } }); rerender(() => { - return useMatchingReferences({...defaultProps, source: 'blog_posts_id' }); + return useMatchingReferences({ + ...defaultProps, + source: 'blog_posts_id', + }); }); - expect(dispatch).toBeCalledTimes(2);expect(dispatch.mock.calls[1][0].type).toBe( + expect(dispatch).toBeCalledTimes(2); + expect(dispatch.mock.calls[1][0].type).toBe( 'RA/CRUD_GET_MATCHING_ACCUMULATE' ); expect( @@ -256,17 +269,17 @@ describe('useMatchingReferences', () => { ).toEqual({ resource: 'posts', relatedTo: 'comments@blog_posts_id', - pagination:{ - perPage:25, - page:1 + pagination: { + perPage: 25, + page: 1, }, - sort:{ + sort: { field: 'id', - order: 'DESC' + order: 'DESC', + }, + filter: { + q: '', }, - filter:{ - q: '' - } }); }); @@ -284,26 +297,30 @@ describe('useMatchingReferences', () => { ).toEqual({ resource: 'posts', relatedTo: 'comments@post_id', - pagination:{ - perPage:25, - page:1 + pagination: { + perPage: 25, + page: 1, }, - sort:{ + sort: { field: 'id', - order: 'DESC' + order: 'DESC', + }, + filter: { + q: '', }, - filter:{ - q: '' - } }); rerender(() => { - return useMatchingReferences({...defaultProps, pagination: { - perPage: 25, - page: 2, - } }); + return useMatchingReferences({ + ...defaultProps, + pagination: { + perPage: 25, + page: 2, + }, + }); }); - expect(dispatch).toBeCalledTimes(2);expect(dispatch.mock.calls[1][0].type).toBe( + expect(dispatch).toBeCalledTimes(2); + expect(dispatch.mock.calls[1][0].type).toBe( 'RA/CRUD_GET_MATCHING_ACCUMULATE' ); expect( @@ -311,17 +328,17 @@ describe('useMatchingReferences', () => { ).toEqual({ resource: 'posts', relatedTo: 'comments@post_id', - pagination:{ - perPage:25, + pagination: { + perPage: 25, page: 2, }, - sort:{ + sort: { field: 'id', - order: 'DESC' + order: 'DESC', + }, + filter: { + q: '', }, - filter:{ - q: '' - } }); }); @@ -339,26 +356,30 @@ describe('useMatchingReferences', () => { ).toEqual({ resource: 'posts', relatedTo: 'comments@post_id', - pagination:{ - perPage:25, - page:1 + pagination: { + perPage: 25, + page: 1, }, - sort:{ + sort: { field: 'id', - order: 'DESC' + order: 'DESC', + }, + filter: { + q: '', }, - filter:{ - q: '' - } }); rerender(() => { - return useMatchingReferences({...defaultProps, pagination: { - perPage: 50, - page: 1, - } }); + return useMatchingReferences({ + ...defaultProps, + pagination: { + perPage: 50, + page: 1, + }, + }); }); - expect(dispatch).toBeCalledTimes(2);expect(dispatch.mock.calls[1][0].type).toBe( + expect(dispatch).toBeCalledTimes(2); + expect(dispatch.mock.calls[1][0].type).toBe( 'RA/CRUD_GET_MATCHING_ACCUMULATE' ); expect( @@ -366,17 +387,17 @@ describe('useMatchingReferences', () => { ).toEqual({ resource: 'posts', relatedTo: 'comments@post_id', - pagination:{ - perPage:50, + pagination: { + perPage: 50, page: 1, }, - sort:{ + sort: { field: 'id', - order: 'DESC' + order: 'DESC', + }, + filter: { + q: '', }, - filter:{ - q: '' - } }); }); @@ -394,26 +415,30 @@ describe('useMatchingReferences', () => { ).toEqual({ resource: 'posts', relatedTo: 'comments@post_id', - pagination:{ - perPage:25, - page:1 + pagination: { + perPage: 25, + page: 1, }, - sort:{ + sort: { field: 'id', - order: 'DESC' + order: 'DESC', + }, + filter: { + q: '', }, - filter:{ - q: '' - } }); rerender(() => { - return useMatchingReferences({...defaultProps, sort:{ - field: 'uid', - order: 'DESC' - } }); + return useMatchingReferences({ + ...defaultProps, + sort: { + field: 'uid', + order: 'DESC', + }, + }); }); - expect(dispatch).toBeCalledTimes(2);expect(dispatch.mock.calls[1][0].type).toBe( + expect(dispatch).toBeCalledTimes(2); + expect(dispatch.mock.calls[1][0].type).toBe( 'RA/CRUD_GET_MATCHING_ACCUMULATE' ); expect( @@ -421,17 +446,17 @@ describe('useMatchingReferences', () => { ).toEqual({ resource: 'posts', relatedTo: 'comments@post_id', - pagination:{ + pagination: { perPage: 25, page: 1, }, - sort:{ + sort: { field: 'uid', - order: 'DESC' + order: 'DESC', + }, + filter: { + q: '', }, - filter:{ - q: '' - } }); }); @@ -449,26 +474,30 @@ describe('useMatchingReferences', () => { ).toEqual({ resource: 'posts', relatedTo: 'comments@post_id', - pagination:{ - perPage:25, - page:1 + pagination: { + perPage: 25, + page: 1, }, - sort:{ + sort: { field: 'id', - order: 'DESC' + order: 'DESC', + }, + filter: { + q: '', }, - filter:{ - q: '' - } }); rerender(() => { - return useMatchingReferences({ ...defaultProps, sort: { - field: 'id', - order: 'ASC' - } }); + return useMatchingReferences({ + ...defaultProps, + sort: { + field: 'id', + order: 'ASC', + }, + }); }); - expect(dispatch).toBeCalledTimes(2);expect(dispatch.mock.calls[1][0].type).toBe( + expect(dispatch).toBeCalledTimes(2); + expect(dispatch.mock.calls[1][0].type).toBe( 'RA/CRUD_GET_MATCHING_ACCUMULATE' ); expect( @@ -476,17 +505,17 @@ describe('useMatchingReferences', () => { ).toEqual({ resource: 'posts', relatedTo: 'comments@post_id', - pagination:{ + pagination: { perPage: 25, page: 1, }, - sort:{ + sort: { field: 'id', - order: 'ASC' + order: 'ASC', + }, + filter: { + q: '', }, - filter:{ - q: '' - } }); }); diff --git a/packages/ra-core/src/controller/input/useMatchingReferences.ts b/packages/ra-core/src/controller/input/useMatchingReferences.ts index 031e3c4c781..2d0762e540e 100644 --- a/packages/ra-core/src/controller/input/useMatchingReferences.ts +++ b/packages/ra-core/src/controller/input/useMatchingReferences.ts @@ -11,7 +11,7 @@ import { getReferenceResource, } from '../../reducer'; import { Pagination, Sort, Record } from '../../types'; -import { useDeepCompareEffect } from '../../util/hooks'; +import { useDeepCompareEffect } from '../../util/hooks'; interface UseMAtchingReferencesOption { reference: string; @@ -56,20 +56,8 @@ export default ({ sort, }; - useDeepCompareEffect( - () => { - fetchOptions({ - dispatch, - filter, - reference, - referenceSource, - resource, - source, - pagination, - sort, - }) - }, - [ + useDeepCompareEffect(() => { + fetchOptions({ dispatch, filter, reference, @@ -78,8 +66,17 @@ export default ({ source, pagination, sort, - ] - ); + }); + }, [ + dispatch, + filter, + reference, + referenceSource, + resource, + source, + pagination, + sort, + ]); const matchingReferences = useSelector( getMatchingReferences({ diff --git a/packages/ra-core/src/controller/usePaginationState.spec.ts b/packages/ra-core/src/controller/usePaginationState.spec.ts index b6b6db80178..f8cf36a9156 100644 --- a/packages/ra-core/src/controller/usePaginationState.spec.ts +++ b/packages/ra-core/src/controller/usePaginationState.spec.ts @@ -1,4 +1,3 @@ - import renderHook from '../util/renderHook'; import usePaginationState from './usePaginationState'; import { act } from 'react-testing-library'; @@ -10,14 +9,18 @@ describe('usePaginationState', () => { }); it('should take given page and perpage to initalise', () => { - const { childrenProps } = renderHook(() => usePaginationState({ perPage: 50, page: 10 })); + const { childrenProps } = renderHook(() => + usePaginationState({ perPage: 50, page: 10 }) + ); expect(childrenProps.pagination).toEqual({ page: 10, perPage: 50 }); }); it('should update perPage if its props update', () => { - const { childrenProps, childrenMock, rerender } = renderHook(() => usePaginationState({ perPage: 50, page: 10 })); + const { childrenProps, childrenMock, rerender } = renderHook(() => + usePaginationState({ perPage: 50, page: 10 }) + ); expect(childrenProps.pagination).toEqual({ page: 10, perPage: 50 }); - rerender(() => usePaginationState({ perPage: 100, page: 10 })); + rerender(() => usePaginationState({ perPage: 100, page: 10 })); expect(childrenMock).toBeCalledTimes(3); @@ -28,7 +31,9 @@ describe('usePaginationState', () => { }); it('should provide setPagination to setPaginationState', () => { - const { childrenProps, childrenMock } = renderHook(() => usePaginationState()); + const { childrenProps, childrenMock } = renderHook(() => + usePaginationState() + ); expect(childrenProps.pagination).toEqual({ page: 1, perPage: 25 }); act(() => childrenProps.setPagination({ perPage: 100, page: 20 })); @@ -42,7 +47,9 @@ describe('usePaginationState', () => { }); it('should provide setPage to set only the page', () => { - const { childrenProps, childrenMock } = renderHook(() => usePaginationState()); + const { childrenProps, childrenMock } = renderHook(() => + usePaginationState() + ); expect(childrenProps.pagination).toEqual({ page: 1, perPage: 25 }); act(() => childrenProps.setPage(20)); @@ -56,7 +63,9 @@ describe('usePaginationState', () => { }); it('should provide setPerPage to set only perPage', () => { - const { childrenProps, childrenMock } = renderHook(() => usePaginationState()); + const { childrenProps, childrenMock } = renderHook(() => + usePaginationState() + ); expect(childrenProps.pagination).toEqual({ page: 1, perPage: 25 }); act(() => childrenProps.setPerPage(100)); diff --git a/packages/ra-core/src/controller/usePaginationState.ts b/packages/ra-core/src/controller/usePaginationState.ts index 72e4ab35c84..925cf46b0b4 100644 --- a/packages/ra-core/src/controller/usePaginationState.ts +++ b/packages/ra-core/src/controller/usePaginationState.ts @@ -48,7 +48,9 @@ const defaultPagination = { * @param {numper} initialPerPage the initial value per page * @returns {PaginationProps} The pagination props */ -export default (initialPagination: { perPage?: number, page?: number } = {}): PaginationProps => { +export default ( + initialPagination: { perPage?: number; page?: number } = {} +): PaginationProps => { const [pagination, setPagination] = useReducer(paginationReducer, { ...defaultPagination, ...initialPagination, @@ -64,7 +66,7 @@ export default (initialPagination: { perPage?: number, page?: number } = {}): Pa return; } setPerPage(initialPagination.perPage || 25); - }, [initialPagination.perPage || 25]); + }, [initialPagination.perPage, setPerPage]); return { page: pagination.page, diff --git a/packages/ra-core/src/controller/useReference.spec.ts b/packages/ra-core/src/controller/useReference.spec.ts index d7921707cc8..b7e7dbffb49 100644 --- a/packages/ra-core/src/controller/useReference.spec.ts +++ b/packages/ra-core/src/controller/useReference.spec.ts @@ -36,20 +36,22 @@ describe('useReference', () => { expect(dispatch.mock.calls[0][0].type).toBe( 'RA/CRUD_GET_MANY_ACCUMULATE' ); - expect(dispatch.mock.calls[0][0].payload).toEqual( - { ids: ['1'], resource: 'posts' } - ); + expect(dispatch.mock.calls[0][0].payload).toEqual({ + ids: ['1'], + resource: 'posts', + }); rerender(() => { return useReference({ ...defaultProps, id: '2' }); - }) + }); expect(dispatch).toBeCalledTimes(2); expect(dispatch.mock.calls[1][0].type).toBe( 'RA/CRUD_GET_MANY_ACCUMULATE' ); - expect(dispatch.mock.calls[1][0].payload).toEqual( - { ids: ['2'], resource: 'posts' } - ); + expect(dispatch.mock.calls[1][0].payload).toEqual({ + ids: ['2'], + resource: 'posts', + }); }); it('should refetch reference when reference prop change', () => { @@ -61,20 +63,22 @@ describe('useReference', () => { expect(dispatch.mock.calls[0][0].type).toBe( 'RA/CRUD_GET_MANY_ACCUMULATE' ); - expect(dispatch.mock.calls[0][0].payload).toEqual( - { ids: ['1'], resource: 'posts' } - ); + expect(dispatch.mock.calls[0][0].payload).toEqual({ + ids: ['1'], + resource: 'posts', + }); rerender(() => { return useReference({ ...defaultProps, reference: 'comments' }); - }) + }); expect(dispatch).toBeCalledTimes(2); expect(dispatch.mock.calls[1][0].type).toBe( 'RA/CRUD_GET_MANY_ACCUMULATE' ); - expect(dispatch.mock.calls[1][0].payload).toEqual( - { ids: ['1'], resource: 'comments' } - ); + expect(dispatch.mock.calls[1][0].payload).toEqual({ + ids: ['1'], + resource: 'comments', + }); }); it('should pass referenceRecord from redux state to its children', () => { diff --git a/packages/ra-core/src/controller/useReference.ts b/packages/ra-core/src/controller/useReference.ts index b181141e89f..997fb5ae3bf 100644 --- a/packages/ra-core/src/controller/useReference.ts +++ b/packages/ra-core/src/controller/useReference.ts @@ -59,7 +59,7 @@ export const useReference = ({ if (id !== null && typeof id !== 'undefined') { dispatch(crudGetManyAccumulate(reference, [id])); } - }, [id, reference]); + }, [dispatch, id, reference]); return { isLoading: !referenceRecord && !allowEmpty, @@ -69,7 +69,7 @@ export const useReference = ({ const makeGetReferenceRecord = props => state => { const referenceState = getReferenceResource(state, props); - + return referenceState && referenceState.data[props.id]; }; diff --git a/packages/ra-core/src/controller/useSortState.ts b/packages/ra-core/src/controller/useSortState.ts index 5af2db59e4b..42dd9bd37ce 100644 --- a/packages/ra-core/src/controller/useSortState.ts +++ b/packages/ra-core/src/controller/useSortState.ts @@ -17,9 +17,11 @@ const sortReducer = (state: Sort, field: string | Sort): Sort => { return field; } const order = - state.field === field ? state.order === SORT_ASC - ? SORT_DESC - : SORT_ASC : SORT_ASC; + state.field === field + ? state.order === SORT_ASC + ? SORT_DESC + : SORT_ASC + : SORT_ASC; return { field, order }; }; @@ -53,9 +55,7 @@ export const defaultSort = { field: 'id', order: 'DESC' }; * @param {string} initialSort.reference The linked resource name * @returns {SortProps} The sort props */ -export default ( - initialSort: Sort = defaultSort -): SortProps => { +export default (initialSort: Sort = defaultSort): SortProps => { const [sort, dispatch] = useReducer(sortReducer, initialSort); const isFirstRender = useRef(true); useEffect(() => { diff --git a/packages/ra-core/src/util/TestContext.tsx b/packages/ra-core/src/util/TestContext.tsx index 988d9377286..7772028850c 100644 --- a/packages/ra-core/src/util/TestContext.tsx +++ b/packages/ra-core/src/util/TestContext.tsx @@ -55,7 +55,7 @@ class TestContext extends Component { constructor(props) { super(props); const { initialState = {}, enableReducers = false } = props; - + this.storeWithDefault = enableReducers ? createAdminStore({ initialState: merge({}, defaultStore, initialState), diff --git a/packages/ra-core/src/util/renderHook.tsx b/packages/ra-core/src/util/renderHook.tsx index 5b50c1ea254..2212328586a 100644 --- a/packages/ra-core/src/util/renderHook.tsx +++ b/packages/ra-core/src/util/renderHook.tsx @@ -4,7 +4,7 @@ import { render } from 'react-testing-library'; const TestHook = ({ children, hookProps }) => { return children(hookProps()); }; -export default (hookProps) => { +export default hookProps => { let childrenProps = null; const children = props => { childrenProps = props; @@ -12,15 +12,17 @@ export default (hookProps) => { }; const childrenMock = jest.fn().mockImplementation(children); const result = render( - , + ); - return { - ...result, - childrenProps, - childrenMock, - rerender: (newHook) => { - result.rerender() - } + return { + ...result, + childrenProps, + childrenMock, + rerender: newHook => { + result.rerender( + + ); + }, }; }; diff --git a/packages/ra-core/src/util/renderHookWithRedux.tsx b/packages/ra-core/src/util/renderHookWithRedux.tsx index a4576b6b42f..55c33c19f42 100644 --- a/packages/ra-core/src/util/renderHookWithRedux.tsx +++ b/packages/ra-core/src/util/renderHookWithRedux.tsx @@ -15,9 +15,13 @@ export default (hookProps, reduxState?) => { reduxState ); - return { ...result, childrenProps, rerender: newHook => { - return result.rerender( - , - ); - }, }; + return { + ...result, + childrenProps, + rerender: newHook => { + return result.rerender( + + ); + }, + }; }; diff --git a/packages/ra-ui-materialui/src/field/ReferenceField.js b/packages/ra-ui-materialui/src/field/ReferenceField.js index a1be5da0d66..def391fcf2b 100644 --- a/packages/ra-ui-materialui/src/field/ReferenceField.js +++ b/packages/ra-ui-materialui/src/field/ReferenceField.js @@ -137,7 +137,9 @@ const ReferenceField = ({ children, ...props }) => { throw new Error(' only accepts a single child'); } - const { isLoading, referenceRecord, resourceLinkPath } = useReferenceField(props); + const { isLoading, referenceRecord, resourceLinkPath } = useReferenceField( + props + ); return ( Date: Tue, 9 Jul 2019 09:22:29 +0200 Subject: [PATCH 19/48] Update packages/ra-core/src/controller/useFilterState.spec.ts Co-Authored-By: Francois Zaninotto --- packages/ra-core/src/controller/useFilterState.spec.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/ra-core/src/controller/useFilterState.spec.ts b/packages/ra-core/src/controller/useFilterState.spec.ts index 93968e9a08c..c46f5b0aef3 100644 --- a/packages/ra-core/src/controller/useFilterState.spec.ts +++ b/packages/ra-core/src/controller/useFilterState.spec.ts @@ -25,7 +25,7 @@ describe('useFilterState', () => { expect(childrenProps.filter).toEqual({ search: '' }); }); - it('should provide setFilter to update filter value after given debounceTime', async () => { + it('should return a setFilter function to update the filter value after a given debounceTime', async () => { const { childrenProps, childrenMock } = renderHook(() => useFilterState({ debounceTime: 10 }) ); From aaeb45dd31cbf53fd19c793e59c164242eb13993 Mon Sep 17 00:00:00 2001 From: ThieryMichel Date: Tue, 9 Jul 2019 09:22:37 +0200 Subject: [PATCH 20/48] Update packages/ra-core/src/controller/input/useMatchingReferences.ts Co-Authored-By: Francois Zaninotto --- packages/ra-core/src/controller/input/useMatchingReferences.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/ra-core/src/controller/input/useMatchingReferences.ts b/packages/ra-core/src/controller/input/useMatchingReferences.ts index 2d0762e540e..65cf111ea67 100644 --- a/packages/ra-core/src/controller/input/useMatchingReferences.ts +++ b/packages/ra-core/src/controller/input/useMatchingReferences.ts @@ -40,7 +40,7 @@ export default ({ filter, pagination, sort, -}: UseMAtchingReferencesOption): UseMatchingReferencesProps => { +}: UseMatchingReferencesOption): UseMatchingReferencesProps => { const dispatch = useDispatch(); const getMatchingReferences = useMemo(makeMatchingReferencesSelector, []); From f933e9d45349c6d6b10b617ce5892b8ea27e1374 Mon Sep 17 00:00:00 2001 From: ThieryMichel Date: Tue, 9 Jul 2019 09:22:44 +0200 Subject: [PATCH 21/48] Update packages/ra-core/src/controller/input/useMatchingReferences.ts Co-Authored-By: Francois Zaninotto --- packages/ra-core/src/controller/input/useMatchingReferences.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/ra-core/src/controller/input/useMatchingReferences.ts b/packages/ra-core/src/controller/input/useMatchingReferences.ts index 65cf111ea67..2c29ded2cac 100644 --- a/packages/ra-core/src/controller/input/useMatchingReferences.ts +++ b/packages/ra-core/src/controller/input/useMatchingReferences.ts @@ -13,7 +13,7 @@ import { import { Pagination, Sort, Record } from '../../types'; import { useDeepCompareEffect } from '../../util/hooks'; -interface UseMAtchingReferencesOption { +interface UseMatchingReferencesOption { reference: string; referenceSource: (resource: string, source: string) => string; resource: string; From bf5dc6c97e303e6c4e86cf17ffab3cbd758b6232 Mon Sep 17 00:00:00 2001 From: ThieryMichel Date: Tue, 9 Jul 2019 09:22:58 +0200 Subject: [PATCH 22/48] Update packages/ra-core/src/controller/useReference.spec.ts Co-Authored-By: Francois Zaninotto --- packages/ra-core/src/controller/useReference.spec.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/ra-core/src/controller/useReference.spec.ts b/packages/ra-core/src/controller/useReference.spec.ts index b7e7dbffb49..19ecb10b764 100644 --- a/packages/ra-core/src/controller/useReference.spec.ts +++ b/packages/ra-core/src/controller/useReference.spec.ts @@ -27,7 +27,7 @@ describe('useReference', () => { expect(dispatch).toBeCalledTimes(1); }); - it('should refetch reference when id change', () => { + it('should refetch reference when id changes', () => { const { dispatch, rerender } = renderHookWithRedux(() => { return useReference(defaultProps); }); From 0bb73bef2b24e4b43d62e7fdd966f60b058fc619 Mon Sep 17 00:00:00 2001 From: ThieryMichel Date: Tue, 9 Jul 2019 09:23:13 +0200 Subject: [PATCH 23/48] Update packages/ra-core/src/controller/useReference.spec.ts Co-Authored-By: Francois Zaninotto --- packages/ra-core/src/controller/useReference.spec.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/ra-core/src/controller/useReference.spec.ts b/packages/ra-core/src/controller/useReference.spec.ts index 19ecb10b764..1f64e136eae 100644 --- a/packages/ra-core/src/controller/useReference.spec.ts +++ b/packages/ra-core/src/controller/useReference.spec.ts @@ -54,7 +54,7 @@ describe('useReference', () => { }); }); - it('should refetch reference when reference prop change', () => { + it('should refetch reference when reference prop changes', () => { const { dispatch, rerender } = renderHookWithRedux(() => { return useReference(defaultProps); }); From cd6b801f5f706d317ab0c83a82f4f4df9b20a641 Mon Sep 17 00:00:00 2001 From: ThieryMichel Date: Tue, 9 Jul 2019 09:23:35 +0200 Subject: [PATCH 24/48] Update packages/ra-core/src/controller/input/useMatchingReferences.spec.tsx Co-Authored-By: Francois Zaninotto --- .../ra-core/src/controller/input/useMatchingReferences.spec.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/ra-core/src/controller/input/useMatchingReferences.spec.tsx b/packages/ra-core/src/controller/input/useMatchingReferences.spec.tsx index dce66d549ae..b52aa720105 100644 --- a/packages/ra-core/src/controller/input/useMatchingReferences.spec.tsx +++ b/packages/ra-core/src/controller/input/useMatchingReferences.spec.tsx @@ -62,7 +62,7 @@ describe('useMatchingReferences', () => { expect(dispatch).toBeCalledTimes(1); }); - it('should fetch matchingReferences when filter change', () => { + it('should fetch matchingReferences when the filter prop changes', () => { const { dispatch, rerender } = renderHookWithRedux(() => { return useMatchingReferences(defaultProps); }); From 65b38c61b229230c76758cd9a363383369e8d643 Mon Sep 17 00:00:00 2001 From: ThieryMichel Date: Tue, 9 Jul 2019 09:23:57 +0200 Subject: [PATCH 25/48] Update packages/ra-core/src/controller/input/useMatchingReferences.spec.tsx Co-Authored-By: Francois Zaninotto --- .../ra-core/src/controller/input/useMatchingReferences.spec.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/ra-core/src/controller/input/useMatchingReferences.spec.tsx b/packages/ra-core/src/controller/input/useMatchingReferences.spec.tsx index b52aa720105..4a712ca271c 100644 --- a/packages/ra-core/src/controller/input/useMatchingReferences.spec.tsx +++ b/packages/ra-core/src/controller/input/useMatchingReferences.spec.tsx @@ -118,7 +118,7 @@ describe('useMatchingReferences', () => { }); }); - it('should refetch matchingReferences when reference change', () => { + it('should refetch matchingReferences when the reference prop changes', () => { const { dispatch, rerender } = renderHookWithRedux(() => { return useMatchingReferences(defaultProps); }); From a6209f3b80ea6f8d74b544cae6ebc106b0fd50d7 Mon Sep 17 00:00:00 2001 From: ThieryMichel Date: Tue, 9 Jul 2019 09:24:18 +0200 Subject: [PATCH 26/48] Update packages/ra-core/src/controller/input/useMatchingReferences.spec.tsx Co-Authored-By: Francois Zaninotto --- .../ra-core/src/controller/input/useMatchingReferences.spec.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/ra-core/src/controller/input/useMatchingReferences.spec.tsx b/packages/ra-core/src/controller/input/useMatchingReferences.spec.tsx index 4a712ca271c..490c53f01ad 100644 --- a/packages/ra-core/src/controller/input/useMatchingReferences.spec.tsx +++ b/packages/ra-core/src/controller/input/useMatchingReferences.spec.tsx @@ -174,7 +174,7 @@ describe('useMatchingReferences', () => { }); }); - it('should refetch matchingReferences when resource change', () => { + it('should refetch matchingReferences when the resource prop changes', () => { const { dispatch, rerender } = renderHookWithRedux(() => { return useMatchingReferences(defaultProps); }); From 2d76d3958d2e3b00a8c654a6e9a774442b6cd52d Mon Sep 17 00:00:00 2001 From: ThieryMichel Date: Tue, 9 Jul 2019 09:24:36 +0200 Subject: [PATCH 27/48] Update packages/ra-core/src/controller/input/useMatchingReferences.spec.tsx Co-Authored-By: Francois Zaninotto --- .../ra-core/src/controller/input/useMatchingReferences.spec.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/ra-core/src/controller/input/useMatchingReferences.spec.tsx b/packages/ra-core/src/controller/input/useMatchingReferences.spec.tsx index 490c53f01ad..a12d817981a 100644 --- a/packages/ra-core/src/controller/input/useMatchingReferences.spec.tsx +++ b/packages/ra-core/src/controller/input/useMatchingReferences.spec.tsx @@ -227,7 +227,7 @@ describe('useMatchingReferences', () => { }); }); - it('should refetch matchingReferences when source change', () => { + it('should refetch matchingReferences when the source prop changes', () => { const { dispatch, rerender } = renderHookWithRedux(() => { return useMatchingReferences(defaultProps); }); From 0d3f70f3f6611943a747a74d2e7211d0345bb249 Mon Sep 17 00:00:00 2001 From: ThieryMichel Date: Tue, 9 Jul 2019 09:24:53 +0200 Subject: [PATCH 28/48] Update packages/ra-core/src/controller/input/useMatchingReferences.spec.tsx Co-Authored-By: Francois Zaninotto --- .../ra-core/src/controller/input/useMatchingReferences.spec.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/ra-core/src/controller/input/useMatchingReferences.spec.tsx b/packages/ra-core/src/controller/input/useMatchingReferences.spec.tsx index a12d817981a..7db4d00c433 100644 --- a/packages/ra-core/src/controller/input/useMatchingReferences.spec.tsx +++ b/packages/ra-core/src/controller/input/useMatchingReferences.spec.tsx @@ -283,7 +283,7 @@ describe('useMatchingReferences', () => { }); }); - it('should refetch matchingReferences when pagination.page change', () => { + it('should refetch matchingReferences when the pagination.page prop changes', () => { const { dispatch, rerender } = renderHookWithRedux(() => { return useMatchingReferences(defaultProps); }); From 6586b7fa6870b68df91c9aeaad7a05172f14ce68 Mon Sep 17 00:00:00 2001 From: ThieryMichel Date: Tue, 9 Jul 2019 09:25:10 +0200 Subject: [PATCH 29/48] Update packages/ra-core/src/controller/input/useMatchingReferences.spec.tsx Co-Authored-By: Francois Zaninotto --- .../ra-core/src/controller/input/useMatchingReferences.spec.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/ra-core/src/controller/input/useMatchingReferences.spec.tsx b/packages/ra-core/src/controller/input/useMatchingReferences.spec.tsx index 7db4d00c433..117f841c58f 100644 --- a/packages/ra-core/src/controller/input/useMatchingReferences.spec.tsx +++ b/packages/ra-core/src/controller/input/useMatchingReferences.spec.tsx @@ -342,7 +342,7 @@ describe('useMatchingReferences', () => { }); }); - it('should refetch matchingReferences when pagination.pagination change', () => { + it('should refetch matchingReferences when the pagination.pagination prop changes', () => { const { dispatch, rerender } = renderHookWithRedux(() => { return useMatchingReferences(defaultProps); }); From 3ce3cdf8a3a6af9158a03f09730b42cc8e968378 Mon Sep 17 00:00:00 2001 From: ThieryMichel Date: Tue, 9 Jul 2019 09:25:32 +0200 Subject: [PATCH 30/48] Update packages/ra-core/src/controller/input/useMatchingReferences.spec.tsx Co-Authored-By: Francois Zaninotto --- .../ra-core/src/controller/input/useMatchingReferences.spec.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/ra-core/src/controller/input/useMatchingReferences.spec.tsx b/packages/ra-core/src/controller/input/useMatchingReferences.spec.tsx index 117f841c58f..71f47fa3c92 100644 --- a/packages/ra-core/src/controller/input/useMatchingReferences.spec.tsx +++ b/packages/ra-core/src/controller/input/useMatchingReferences.spec.tsx @@ -401,7 +401,7 @@ describe('useMatchingReferences', () => { }); }); - it('should refetch matchingReferences when sort.field change', () => { + it('should refetch matchingReferences when the sort.field prop changes', () => { const { dispatch, rerender } = renderHookWithRedux(() => { return useMatchingReferences(defaultProps); }); From 44e8f9db793bcc22af6166d26e5999ceaad6f229 Mon Sep 17 00:00:00 2001 From: ThieryMichel Date: Tue, 9 Jul 2019 09:25:57 +0200 Subject: [PATCH 31/48] Update packages/ra-core/src/controller/input/useMatchingReferences.spec.tsx Co-Authored-By: Francois Zaninotto --- .../ra-core/src/controller/input/useMatchingReferences.spec.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/ra-core/src/controller/input/useMatchingReferences.spec.tsx b/packages/ra-core/src/controller/input/useMatchingReferences.spec.tsx index 71f47fa3c92..dcd51753a3d 100644 --- a/packages/ra-core/src/controller/input/useMatchingReferences.spec.tsx +++ b/packages/ra-core/src/controller/input/useMatchingReferences.spec.tsx @@ -460,7 +460,7 @@ describe('useMatchingReferences', () => { }); }); - it('should refetch matchingReferences when sort.order change', () => { + it('should refetch matchingReferences when the sort.order prop changes', () => { const { dispatch, rerender } = renderHookWithRedux(() => { return useMatchingReferences(defaultProps); }); From c01c08d5d75b32d496336acfa63bb694072cd9cd Mon Sep 17 00:00:00 2001 From: ThieryMichel Date: Tue, 9 Jul 2019 09:26:28 +0200 Subject: [PATCH 32/48] Update packages/ra-core/src/controller/input/ReferenceInputController.spec.tsx Co-Authored-By: Francois Zaninotto --- .../src/controller/input/ReferenceInputController.spec.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/ra-core/src/controller/input/ReferenceInputController.spec.tsx b/packages/ra-core/src/controller/input/ReferenceInputController.spec.tsx index f0827a1a9b2..5c3254471a3 100644 --- a/packages/ra-core/src/controller/input/ReferenceInputController.spec.tsx +++ b/packages/ra-core/src/controller/input/ReferenceInputController.spec.tsx @@ -21,7 +21,7 @@ describe('', () => { afterEach(cleanup); - it('should fetch reference matchingReferences, and provice filter pagination and sort', () => { + it('should fetch reference matchingReferences, and provide filter pagination and sort', () => { const children = jest.fn().mockReturnValue(

child

); const { dispatch } = renderWithRedux( Date: Tue, 9 Jul 2019 09:27:24 +0200 Subject: [PATCH 33/48] Update packages/ra-core/src/controller/usePaginationState.spec.ts Co-Authored-By: Francois Zaninotto --- packages/ra-core/src/controller/usePaginationState.spec.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/ra-core/src/controller/usePaginationState.spec.ts b/packages/ra-core/src/controller/usePaginationState.spec.ts index f8cf36a9156..32666d91e79 100644 --- a/packages/ra-core/src/controller/usePaginationState.spec.ts +++ b/packages/ra-core/src/controller/usePaginationState.spec.ts @@ -8,7 +8,7 @@ describe('usePaginationState', () => { expect(childrenProps.pagination).toEqual({ page: 1, perPage: 25 }); }); - it('should take given page and perpage to initalise', () => { + it('should take given page and perPage props to initalize', () => { const { childrenProps } = renderHook(() => usePaginationState({ perPage: 50, page: 10 }) ); From 753a7ff18aa709b8e992d3c9c56e0193625d20ec Mon Sep 17 00:00:00 2001 From: thiery Date: Tue, 9 Jul 2019 18:55:00 +0200 Subject: [PATCH 34/48] code review --- .../field/ReferenceFieldController.spec.tsx | 30 +++++++----- .../field/ReferenceFieldController.tsx | 2 +- .../src/controller/field/useReferenceField.ts | 13 +++-- .../input/useMatchingReferences.spec.tsx | 27 +++++------ .../controller/input/useMatchingReferences.ts | 40 ++++------------ .../src/controller/useFilterState.spec.ts | 22 ++++----- .../ra-core/src/controller/useFilterState.ts | 42 +++++++++++++++++ .../src/controller/usePaginationState.spec.ts | 40 ++++++++-------- .../src/controller/usePaginationState.ts | 1 - .../src/controller/useReference.spec.ts | 47 ++++++++++++++----- .../ra-core/src/controller/useReference.ts | 33 ++++++++----- .../src/controller/useSortState.spec.ts | 28 +++++------ packages/ra-core/src/util/renderHook.tsx | 18 ++++--- .../ra-core/src/util/renderHookWithRedux.tsx | 16 +++---- 14 files changed, 207 insertions(+), 152 deletions(-) diff --git a/packages/ra-core/src/controller/field/ReferenceFieldController.spec.tsx b/packages/ra-core/src/controller/field/ReferenceFieldController.spec.tsx index 375dbc8716b..b5afb10a772 100644 --- a/packages/ra-core/src/controller/field/ReferenceFieldController.spec.tsx +++ b/packages/ra-core/src/controller/field/ReferenceFieldController.spec.tsx @@ -20,6 +20,7 @@ describe('', () => { record={{ id: 1, postId: 123 }} source="postId" reference="posts" + resource="comments" basePath="" />, defaultState @@ -35,6 +36,7 @@ describe('', () => { record={{ id: 1, postId: null }} source="postId" reference="posts" + resource="comments" basePath="" />, defaultState @@ -51,7 +53,6 @@ describe('', () => { reference="posts" resource="comments" basePath="/comments" - crudGetManyAccumulate={crudGetManyAccumulate} > {children} , @@ -59,7 +60,8 @@ describe('', () => { ); expect(children).toBeCalledWith({ - isLoading: false, + loading: false, + loaded: true, referenceRecord: { id: 123, title: 'foo' }, resourceLinkPath: '/posts/123', }); @@ -89,7 +91,8 @@ describe('', () => { ); expect(children).toBeCalledWith({ - isLoading: false, + loading: false, + loaded: true, referenceRecord: { id: 123, title: 'foo' }, resourceLinkPath: '/prefix/posts/123', }); @@ -119,7 +122,8 @@ describe('', () => { ); expect(children).toBeCalledWith({ - isLoading: false, + loading: false, + loaded: true, referenceRecord: { id: 123, title: 'foo' }, resourceLinkPath: '/edit/123', }); @@ -134,7 +138,6 @@ describe('', () => { reference="show" resource="edit" basePath="/edit" - crudGetManyAccumulate={crudGetManyAccumulate} > {children} , @@ -150,7 +153,8 @@ describe('', () => { ); expect(children).toBeCalledWith({ - isLoading: false, + loading: false, + loaded: true, referenceRecord: { id: 123, title: 'foo' }, resourceLinkPath: '/show/123', }); @@ -173,7 +177,8 @@ describe('', () => { ); expect(children).toBeCalledWith({ - isLoading: false, + loading: false, + loaded: true, referenceRecord: { id: 123, title: 'foo' }, resourceLinkPath: '/posts/123/show', }); @@ -204,7 +209,8 @@ describe('', () => { ); expect(children).toBeCalledWith({ - isLoading: false, + loading: false, + loaded: true, referenceRecord: { id: 123, title: 'foo' }, resourceLinkPath: '/edit/123/show', }); @@ -219,7 +225,6 @@ describe('', () => { reference="show" resource="edit" basePath="/edit" - crudGetManyAccumulate={crudGetManyAccumulate} link="show" > {children} @@ -236,7 +241,8 @@ describe('', () => { ); expect(children).toBeCalledWith({ - isLoading: false, + loading: false, + loaded: true, referenceRecord: { id: 123, title: 'foo' }, resourceLinkPath: '/show/123/show', }); @@ -249,6 +255,7 @@ describe('', () => { record={{ id: 1, postId: 123 }} source="postId" reference="posts" + resource="comments" basePath="/foo" link={false} > @@ -258,7 +265,8 @@ describe('', () => { ); expect(children).toBeCalledWith({ - isLoading: false, + loading: false, + loaded: true, referenceRecord: { id: 123, title: 'foo' }, resourceLinkPath: false, }); diff --git a/packages/ra-core/src/controller/field/ReferenceFieldController.tsx b/packages/ra-core/src/controller/field/ReferenceFieldController.tsx index e294b63c55d..2116e6719ef 100644 --- a/packages/ra-core/src/controller/field/ReferenceFieldController.tsx +++ b/packages/ra-core/src/controller/field/ReferenceFieldController.tsx @@ -14,7 +14,7 @@ interface Props { reference: string; resource: string; source: string; - link: string | boolean | LinkToFunctionType; + link?: string | boolean | LinkToFunctionType; } /** diff --git a/packages/ra-core/src/controller/field/useReferenceField.ts b/packages/ra-core/src/controller/field/useReferenceField.ts index 0585cd11b40..bde907dfc73 100644 --- a/packages/ra-core/src/controller/field/useReferenceField.ts +++ b/packages/ra-core/src/controller/field/useReferenceField.ts @@ -15,12 +15,13 @@ interface Option { source: string; reference: string; resource: string; - link: LinkToType; + link?: LinkToType; linkType?: LinkToType; // deprecated, use link instead } export interface UseReferenceProps { - isLoading: boolean; + loading: boolean; + loaded: boolean; referenceRecord: Record; resourceLinkPath: string | false; } @@ -28,7 +29,8 @@ export interface UseReferenceProps { /** * @typedef ReferenceProps * @type {Object} - * @property {boolean} isLoading: boolean indicating if the reference has loaded + * @property {boolean} loading: boolean indicating if the reference is loading + * @property {boolean} loaded: boolean indicating if the reference has loaded * @property {Object} referenceRecord: the referenced record. * @property {string | false} resourceLinkPath link to the page of the related record (depends on link) (false is no link) */ @@ -72,7 +74,7 @@ export const useReferenceField = ({ source, }: Option): UseReferenceProps => { const sourceId = get(record, source); - const { referenceRecord, isLoading } = useReference({ + const { referenceRecord, loading, loaded } = useReference({ id: sourceId, reference, allowEmpty, @@ -97,7 +99,8 @@ export const useReferenceField = ({ ); return { - isLoading, + loading, + loaded, referenceRecord, resourceLinkPath, }; diff --git a/packages/ra-core/src/controller/input/useMatchingReferences.spec.tsx b/packages/ra-core/src/controller/input/useMatchingReferences.spec.tsx index dcd51753a3d..dbb0a9a012c 100644 --- a/packages/ra-core/src/controller/input/useMatchingReferences.spec.tsx +++ b/packages/ra-core/src/controller/input/useMatchingReferences.spec.tsx @@ -520,7 +520,7 @@ describe('useMatchingReferences', () => { }); it('should pass matching references from redux state to its children', () => { - const { childrenProps } = renderHookWithRedux( + const { hookValue } = renderHookWithRedux( () => { return useMatchingReferences(defaultProps); }, @@ -536,17 +536,14 @@ describe('useMatchingReferences', () => { } ); - expect(childrenProps.matchingReferences).toEqual([ - { id: 2 }, - { id: 1 }, - ]); + expect(hookValue.matchingReferences).toEqual([{ id: 2 }, { id: 1 }]); - expect(childrenProps.loading).toBe(false); - expect(childrenProps.error).toBe(null); + expect(hookValue.loading).toBe(false); + expect(hookValue.error).toBe(null); }); it('should pass an error if an error is in redux state', () => { - const { childrenProps } = renderHookWithRedux( + const { hookValue } = renderHookWithRedux( () => { return useMatchingReferences(defaultProps); }, @@ -566,14 +563,14 @@ describe('useMatchingReferences', () => { } ); - expect(childrenProps.matchingReferences).toBe(null); + expect(hookValue.matchingReferences).toBe(null); - expect(childrenProps.loading).toBe(false); - expect(childrenProps.error).toBe('Something bad happened'); + expect(hookValue.loading).toBe(false); + expect(hookValue.error).toBe('Something bad happened'); }); it('should pass loading true if no matching reference yet', () => { - const { childrenProps } = renderHookWithRedux( + const { hookValue } = renderHookWithRedux( () => { return useMatchingReferences(defaultProps); }, @@ -589,9 +586,9 @@ describe('useMatchingReferences', () => { } ); - expect(childrenProps.matchingReferences).toBe(null); + expect(hookValue.matchingReferences).toBe(null); - expect(childrenProps.loading).toBe(true); - expect(childrenProps.error).toBe(null); + expect(hookValue.loading).toBe(true); + expect(hookValue.error).toBe(null); }); }); diff --git a/packages/ra-core/src/controller/input/useMatchingReferences.ts b/packages/ra-core/src/controller/input/useMatchingReferences.ts index 2c29ded2cac..049685f043e 100644 --- a/packages/ra-core/src/controller/input/useMatchingReferences.ts +++ b/packages/ra-core/src/controller/input/useMatchingReferences.ts @@ -57,16 +57,15 @@ export default ({ }; useDeepCompareEffect(() => { - fetchOptions({ - dispatch, - filter, - reference, - referenceSource, - resource, - source, - pagination, - sort, - }); + dispatch( + crudGetMatchingAccumulate( + reference, + referenceSource(resource, source), + pagination, + sort, + filter + ) + ); }, [ dispatch, filter, @@ -111,27 +110,6 @@ export default ({ }; }; -const fetchOptions = ({ - dispatch, - filter, - reference, - referenceSource, - resource, - source, - pagination, - sort, -}) => { - dispatch( - crudGetMatchingAccumulate( - reference, - referenceSource(resource, source), - pagination, - sort, - filter - ) - ); -}; - const makeMatchingReferencesSelector = () => { const matchingReferencesSelector = createSelector( [ diff --git a/packages/ra-core/src/controller/useFilterState.spec.ts b/packages/ra-core/src/controller/useFilterState.spec.ts index c46f5b0aef3..f82056a611d 100644 --- a/packages/ra-core/src/controller/useFilterState.spec.ts +++ b/packages/ra-core/src/controller/useFilterState.spec.ts @@ -4,35 +4,35 @@ import { act } from 'react-testing-library'; describe('useFilterState', () => { it('should initialize filterState with default filter', () => { - const { childrenProps } = renderHook(() => useFilterState({})); + const { hookValue } = renderHook(() => useFilterState({})); - expect(childrenProps.filter).toEqual({ q: '' }); + expect(hookValue.filter).toEqual({ q: '' }); }); it('should initialize filterState with permanent filter', () => { - const { childrenProps } = renderHook(() => + const { hookValue } = renderHook(() => useFilterState({ permanentFilter: { type: 'thisOne' } }) ); - expect(childrenProps.filter).toEqual({ q: '', type: 'thisOne' }); + expect(hookValue.filter).toEqual({ q: '', type: 'thisOne' }); }); it('should initialize using filterToQuery if provided', () => { - const { childrenProps } = renderHook(() => + const { hookValue } = renderHook(() => useFilterState({ filterToQuery: v => ({ search: v }) }) ); - expect(childrenProps.filter).toEqual({ search: '' }); + expect(hookValue.filter).toEqual({ search: '' }); }); it('should return a setFilter function to update the filter value after a given debounceTime', async () => { - const { childrenProps, childrenMock } = renderHook(() => + const { hookValue, childrenMock } = renderHook(() => useFilterState({ debounceTime: 10 }) ); - expect(childrenProps.filter).toEqual({ q: '' }); + expect(hookValue.filter).toEqual({ q: '' }); - act(() => childrenProps.setFilter('needle in a haystack')); + act(() => hookValue.setFilter('needle in a haystack')); expect(childrenMock).toBeCalledTimes(1); await new Promise(resolve => setTimeout(resolve, 10)); @@ -45,7 +45,7 @@ describe('useFilterState', () => { }); it('should provide setFilter to update filter value after given debounceTime preserving permanentFilter and filterToQuery', async () => { - const { childrenProps, childrenMock } = renderHook(() => + const { hookValue, childrenMock } = renderHook(() => useFilterState({ debounceTime: 10, permanentFilter: { type: 'thisOne' }, @@ -53,7 +53,7 @@ describe('useFilterState', () => { }) ); - act(() => childrenProps.setFilter('needle in a haystack')); + act(() => hookValue.setFilter('needle in a haystack')); expect(childrenMock).toBeCalledTimes(1); await new Promise(resolve => setTimeout(resolve, 10)); diff --git a/packages/ra-core/src/controller/useFilterState.ts b/packages/ra-core/src/controller/useFilterState.ts index 03953914243..e07c8ac9b0e 100644 --- a/packages/ra-core/src/controller/useFilterState.ts +++ b/packages/ra-core/src/controller/useFilterState.ts @@ -16,6 +16,48 @@ interface UseFilterStateProps { setFilter: (v: string) => void; } +/** + * @name setFilter + * @function + * @param {string} the value + */ + +/** + * @typedef FilterProps + * @type {Object} + * @property {Object} filter: The filter object. + * @property {setFilter} setFilter: Update the filter with the given string + */ + +/** + * Hooks to provide filter state and setFilter which update the query part of the filter + * + * @example + * + * const { filter, setFilter } = useFilter({ + * filterToQuery: v => ({ query: v }), + * permanentFilter: { foo: 'bar' }, + * debounceTime: 500, + * }); + * // filter inital value: + * { + * query: '', + * foo: 'bar' + * } + * // after updating filter + * setFilter('needle'); + * { + * query: 'needle', + * foo: 'bar' + * } + * + * @param {Object} option + * @param {Function} option.filterToQuery function to convert the filter string to a filter object + * @param {Object} option.permanentFilter permanent filter to be merged with the filter string default to {} + * @param {number} option.debounceTime Time between filter update allow to debounce the search + * + * @returns {FilterProps} The filter props + */ export default ({ filterToQuery = v => ({ q: v }), permanentFilter = {}, diff --git a/packages/ra-core/src/controller/usePaginationState.spec.ts b/packages/ra-core/src/controller/usePaginationState.spec.ts index 32666d91e79..1571a3708c4 100644 --- a/packages/ra-core/src/controller/usePaginationState.spec.ts +++ b/packages/ra-core/src/controller/usePaginationState.spec.ts @@ -3,23 +3,23 @@ import usePaginationState from './usePaginationState'; import { act } from 'react-testing-library'; describe('usePaginationState', () => { - it('should initialise pagination state with default', () => { - const { childrenProps } = renderHook(() => usePaginationState()); - expect(childrenProps.pagination).toEqual({ page: 1, perPage: 25 }); + it('should initialize pagination state with default', () => { + const { hookValue } = renderHook(() => usePaginationState()); + expect(hookValue.pagination).toEqual({ page: 1, perPage: 25 }); }); it('should take given page and perPage props to initalize', () => { - const { childrenProps } = renderHook(() => + const { hookValue } = renderHook(() => usePaginationState({ perPage: 50, page: 10 }) ); - expect(childrenProps.pagination).toEqual({ page: 10, perPage: 50 }); + expect(hookValue.pagination).toEqual({ page: 10, perPage: 50 }); }); - it('should update perPage if its props update', () => { - const { childrenProps, childrenMock, rerender } = renderHook(() => + it('should update perPage state when the perPage props update', () => { + const { hookValue, childrenMock, rerender } = renderHook(() => usePaginationState({ perPage: 50, page: 10 }) ); - expect(childrenProps.pagination).toEqual({ page: 10, perPage: 50 }); + expect(hookValue.pagination).toEqual({ page: 10, perPage: 50 }); rerender(() => usePaginationState({ perPage: 100, page: 10 })); expect(childrenMock).toBeCalledTimes(3); @@ -30,13 +30,13 @@ describe('usePaginationState', () => { }); }); - it('should provide setPagination to setPaginationState', () => { - const { childrenProps, childrenMock } = renderHook(() => + it('should provide a setPagination function to update the pagination state (page + perPage)', () => { + const { hookValue, childrenMock } = renderHook(() => usePaginationState() ); - expect(childrenProps.pagination).toEqual({ page: 1, perPage: 25 }); + expect(hookValue.pagination).toEqual({ page: 1, perPage: 25 }); - act(() => childrenProps.setPagination({ perPage: 100, page: 20 })); + act(() => hookValue.setPagination({ perPage: 100, page: 20 })); expect(childrenMock).toBeCalledTimes(2); @@ -46,13 +46,13 @@ describe('usePaginationState', () => { }); }); - it('should provide setPage to set only the page', () => { - const { childrenProps, childrenMock } = renderHook(() => + it('should provide setPage function to update the page state', () => { + const { hookValue, childrenMock } = renderHook(() => usePaginationState() ); - expect(childrenProps.pagination).toEqual({ page: 1, perPage: 25 }); + expect(hookValue.pagination).toEqual({ page: 1, perPage: 25 }); - act(() => childrenProps.setPage(20)); + act(() => hookValue.setPage(20)); expect(childrenMock).toBeCalledTimes(2); @@ -62,13 +62,13 @@ describe('usePaginationState', () => { }); }); - it('should provide setPerPage to set only perPage', () => { - const { childrenProps, childrenMock } = renderHook(() => + it('should provide a setPerPage function to update the perPage state', () => { + const { hookValue, childrenMock } = renderHook(() => usePaginationState() ); - expect(childrenProps.pagination).toEqual({ page: 1, perPage: 25 }); + expect(hookValue.pagination).toEqual({ page: 1, perPage: 25 }); - act(() => childrenProps.setPerPage(100)); + act(() => hookValue.setPerPage(100)); expect(childrenMock).toBeCalledTimes(2); diff --git a/packages/ra-core/src/controller/usePaginationState.ts b/packages/ra-core/src/controller/usePaginationState.ts index 925cf46b0b4..e3a42735f53 100644 --- a/packages/ra-core/src/controller/usePaginationState.ts +++ b/packages/ra-core/src/controller/usePaginationState.ts @@ -23,7 +23,6 @@ const defaultPagination = { }; /** - * set the sort to the given field, swap the order if the field is the same * @name setNumber * @function * @param {number} state the state value diff --git a/packages/ra-core/src/controller/useReference.spec.ts b/packages/ra-core/src/controller/useReference.spec.ts index 1f64e136eae..7cac71e696e 100644 --- a/packages/ra-core/src/controller/useReference.spec.ts +++ b/packages/ra-core/src/controller/useReference.spec.ts @@ -81,8 +81,28 @@ describe('useReference', () => { }); }); - it('should pass referenceRecord from redux state to its children', () => { - const { childrenProps } = renderHookWithRedux( + it('it should not refetch reference when allowEmpty change', () => { + const { dispatch, rerender } = renderHookWithRedux(() => { + return useReference(defaultProps); + }); + + expect(dispatch).toBeCalledTimes(1); + expect(dispatch.mock.calls[0][0].type).toBe( + 'RA/CRUD_GET_MANY_ACCUMULATE' + ); + expect(dispatch.mock.calls[0][0].payload).toEqual({ + ids: ['1'], + resource: 'posts', + }); + rerender(() => { + return useReference({ ...defaultProps, allowEmpty: true }); + }); + + expect(dispatch).toBeCalledTimes(1); + }); + + it('should retrieve referenceRecord from redux state', () => { + const { hookValue } = renderHookWithRedux( () => { return useReference(defaultProps); }, @@ -95,14 +115,15 @@ describe('useReference', () => { } ); - expect(childrenProps).toEqual({ + expect(hookValue).toEqual({ referenceRecord: { id: 1 }, - isLoading: false, + loading: false, + loaded: true, }); }); - it('should set isLoading to true if no referenceRecord yet', () => { - const { childrenProps } = renderHookWithRedux( + it('should set loading to true if no referenceRecord yet', () => { + const { hookValue } = renderHookWithRedux( () => { return useReference(defaultProps); }, @@ -115,14 +136,15 @@ describe('useReference', () => { } ); - expect(childrenProps).toEqual({ + expect(hookValue).toEqual({ referenceRecord: undefined, - isLoading: true, + loading: true, + loaded: false, }); }); - it('should set isLoading to false even if no referenceRecord yet when allowEmpty is true', () => { - const { childrenProps } = renderHookWithRedux( + it('should set loading to false even if no referenceRecord yet when allowEmpty is true', () => { + const { hookValue } = renderHookWithRedux( () => { return useReference({ ...defaultProps, allowEmpty: true }); }, @@ -135,9 +157,10 @@ describe('useReference', () => { } ); - expect(childrenProps).toEqual({ + expect(hookValue).toEqual({ referenceRecord: undefined, - isLoading: false, + loading: false, + loaded: true, }); }); }); diff --git a/packages/ra-core/src/controller/useReference.ts b/packages/ra-core/src/controller/useReference.ts index 997fb5ae3bf..61ed7da670a 100644 --- a/packages/ra-core/src/controller/useReference.ts +++ b/packages/ra-core/src/controller/useReference.ts @@ -1,4 +1,4 @@ -import { useEffect, useMemo } from 'react'; +import { useEffect, useCallback } from 'react'; // @ts-ignore import { useDispatch, useSelector } from 'react-redux'; @@ -13,14 +13,16 @@ interface Option { } export interface UseReferenceProps { - isLoading: boolean; + loading: boolean; + loaded: boolean; referenceRecord: Record; } /** * @typedef ReferenceProps * @type {Object} - * @property {boolean} isLoading: boolean indicating if the reference has loaded + * @property {boolean} loading: boolean indicating if the reference is loading + * @property {boolean} loaded: boolean indicating if the reference has loaded * @property {Object} referenceRecord: the referenced record. */ @@ -32,7 +34,7 @@ export interface UseReferenceProps { * * @example * - * const { isLoading, referenceRecord } = useReference({ + * const { loading, loaded, referenceRecord } = useReference({ * id: 7, * reference: 'users', * }); @@ -50,27 +52,32 @@ export const useReference = ({ id, }: Option): UseReferenceProps => { const dispatch = useDispatch(); - const getReferenceRecord = useMemo( - () => makeGetReferenceRecord({ id, reference }), - [id, reference] - ); - const referenceRecord = useSelector(getReferenceRecord); useEffect(() => { if (id !== null && typeof id !== 'undefined') { dispatch(crudGetManyAccumulate(reference, [id])); } }, [dispatch, id, reference]); + const referenceRecord = useReferenceSelector({ reference, id }); + return { - isLoading: !referenceRecord && !allowEmpty, + loading: !referenceRecord && !allowEmpty, + loaded: !!referenceRecord || allowEmpty, referenceRecord, }; }; -const makeGetReferenceRecord = props => state => { - const referenceState = getReferenceResource(state, props); +const useReferenceSelector = ({ id, reference }) => { + const getReferenceRecord = useCallback( + state => { + const referenceState = getReferenceResource(state, { reference }); + + return referenceState && referenceState.data[id]; + }, + [id, reference] + ); - return referenceState && referenceState.data[props.id]; + return useSelector(getReferenceRecord); }; export default useReference; diff --git a/packages/ra-core/src/controller/useSortState.spec.ts b/packages/ra-core/src/controller/useSortState.spec.ts index df6d13e4aa4..fd7ef836375 100644 --- a/packages/ra-core/src/controller/useSortState.spec.ts +++ b/packages/ra-core/src/controller/useSortState.spec.ts @@ -4,30 +4,30 @@ import { act } from 'react-testing-library'; describe('useSortState', () => { it('should initialize sortState with default sort', () => { - const { childrenProps } = renderHook(() => useSortState()); + const { hookValue } = renderHook(() => useSortState()); - expect(childrenProps.sort).toEqual(defaultSort); + expect(hookValue.sort).toEqual(defaultSort); }); it('should initialize sortState with given sort', () => { - const { childrenProps } = renderHook(() => + const { hookValue } = renderHook(() => useSortState({ field: 'name', order: 'ASC', }) ); - expect(childrenProps.sort).toEqual({ field: 'name', order: 'ASC' }); + expect(hookValue.sort).toEqual({ field: 'name', order: 'ASC' }); }); it('should provide setSort method to change the whole sort', () => { - const { childrenProps, childrenMock } = renderHook(() => + const { hookValue, childrenMock } = renderHook(() => useSortState({ field: 'id', order: 'DESC' }) ); - expect(childrenProps.sort).toEqual({ field: 'id', order: 'DESC' }); + expect(hookValue.sort).toEqual({ field: 'id', order: 'DESC' }); - act(() => childrenProps.setSort({ field: 'name', order: 'ASC' })); + act(() => hookValue.setSort({ field: 'name', order: 'ASC' })); expect(childrenMock.mock.calls[1][0].sort).toEqual({ field: 'name', order: 'ASC', @@ -36,13 +36,13 @@ describe('useSortState', () => { describe('should provide setSortField method that', () => { it('should just change the order if receiving the current field', () => { - const { childrenProps, childrenMock } = renderHook(() => + const { hookValue, childrenMock } = renderHook(() => useSortState({ field: 'id', order: 'DESC' }) ); - expect(childrenProps.sort).toEqual({ field: 'id', order: 'DESC' }); + expect(hookValue.sort).toEqual({ field: 'id', order: 'DESC' }); - act(() => childrenProps.setSortField('id')); + act(() => hookValue.setSortField('id')); expect(childrenMock.mock.calls[1][0].sort).toEqual({ field: 'id', order: 'ASC', @@ -50,18 +50,18 @@ describe('useSortState', () => { }); it('should change the field and set the order to ASC if receiving another field', () => { - const { childrenProps, childrenMock } = renderHook(() => + const { hookValue, childrenMock } = renderHook(() => useSortState({ field: 'id', order: 'ASC' }) ); - expect(childrenProps.sort).toEqual({ field: 'id', order: 'ASC' }); + expect(hookValue.sort).toEqual({ field: 'id', order: 'ASC' }); - act(() => childrenProps.setSortField('name')); + act(() => hookValue.setSortField('name')); expect(childrenMock.mock.calls[1][0].sort).toEqual({ field: 'name', order: 'ASC', }); - act(() => childrenProps.setSortField('id')); + act(() => hookValue.setSortField('id')); expect(childrenMock.mock.calls[2][0].sort).toEqual({ field: 'id', order: 'ASC', diff --git a/packages/ra-core/src/util/renderHook.tsx b/packages/ra-core/src/util/renderHook.tsx index 2212328586a..e6026b3bbd2 100644 --- a/packages/ra-core/src/util/renderHook.tsx +++ b/packages/ra-core/src/util/renderHook.tsx @@ -1,27 +1,25 @@ import React from 'react'; import { render } from 'react-testing-library'; -const TestHook = ({ children, hookProps }) => { - return children(hookProps()); +const TestHook = ({ children, hook }) => { + return children(hook()); }; -export default hookProps => { - let childrenProps = null; +export default hook => { + let hookValue = null; const children = props => { - childrenProps = props; + hookValue = props; return

child

; }; const childrenMock = jest.fn().mockImplementation(children); - const result = render( - - ); + const result = render(); return { ...result, - childrenProps, + hookValue, childrenMock, rerender: newHook => { result.rerender( - + ); }, }; diff --git a/packages/ra-core/src/util/renderHookWithRedux.tsx b/packages/ra-core/src/util/renderHookWithRedux.tsx index 55c33c19f42..1185998851d 100644 --- a/packages/ra-core/src/util/renderHookWithRedux.tsx +++ b/packages/ra-core/src/util/renderHookWithRedux.tsx @@ -1,26 +1,26 @@ import React from 'react'; import renderWithRedux from './renderWithRedux'; -const TestHook = ({ children, hookProps }) => { - return children(hookProps()); +const TestHook = ({ children, hook }) => { + return children(hook()); }; -export default (hookProps, reduxState?) => { - let childrenProps = null; +export default (hook, reduxState?) => { + let hookValue = null; const children = props => { - childrenProps = props; + hookValue = props; return

child

; }; const result = renderWithRedux( - , + , reduxState ); return { ...result, - childrenProps, + hookValue, rerender: newHook => { return result.rerender( - + ); }, }; From ffd69d5ac5ce7093071f2aaeae810ce7a2bcf9cd Mon Sep 17 00:00:00 2001 From: thiery Date: Wed, 10 Jul 2019 11:07:23 +0200 Subject: [PATCH 35/48] merge renderHookWIthRedux with renderHook --- .../input/useMatchingReferences.spec.tsx | 29 ++++++++------- packages/ra-core/src/util/renderHook.tsx | 35 ++++++++++++++++--- .../ra-core/src/util/renderHookWithRedux.tsx | 27 -------------- packages/ra-core/src/util/renderWithRedux.tsx | 9 +++-- 4 files changed, 54 insertions(+), 46 deletions(-) delete mode 100644 packages/ra-core/src/util/renderHookWithRedux.tsx diff --git a/packages/ra-core/src/controller/input/useMatchingReferences.spec.tsx b/packages/ra-core/src/controller/input/useMatchingReferences.spec.tsx index dbb0a9a012c..f160ea8b3a9 100644 --- a/packages/ra-core/src/controller/input/useMatchingReferences.spec.tsx +++ b/packages/ra-core/src/controller/input/useMatchingReferences.spec.tsx @@ -1,4 +1,4 @@ -import renderHookWithRedux from '../../util/renderHookWithRedux'; +import renderHook from '../../util/renderHook'; import useMatchingReferences from './useMatchingReferences'; import { cleanup } from 'react-testing-library'; @@ -19,7 +19,7 @@ describe('useMatchingReferences', () => { afterEach(cleanup); it('should fetch matchingReferences on mount', () => { - const { dispatch, rerender } = renderHookWithRedux(() => { + const { dispatch, rerender } = renderHook(() => { return useMatchingReferences(defaultProps); }); @@ -63,7 +63,7 @@ describe('useMatchingReferences', () => { }); it('should fetch matchingReferences when the filter prop changes', () => { - const { dispatch, rerender } = renderHookWithRedux(() => { + const { dispatch, rerender } = renderHook(() => { return useMatchingReferences(defaultProps); }); @@ -119,7 +119,7 @@ describe('useMatchingReferences', () => { }); it('should refetch matchingReferences when the reference prop changes', () => { - const { dispatch, rerender } = renderHookWithRedux(() => { + const { dispatch, rerender } = renderHook(() => { return useMatchingReferences(defaultProps); }); @@ -175,7 +175,7 @@ describe('useMatchingReferences', () => { }); it('should refetch matchingReferences when the resource prop changes', () => { - const { dispatch, rerender } = renderHookWithRedux(() => { + const { dispatch, rerender } = renderHook(() => { return useMatchingReferences(defaultProps); }); @@ -228,7 +228,7 @@ describe('useMatchingReferences', () => { }); it('should refetch matchingReferences when the source prop changes', () => { - const { dispatch, rerender } = renderHookWithRedux(() => { + const { dispatch, rerender } = renderHook(() => { return useMatchingReferences(defaultProps); }); @@ -284,7 +284,7 @@ describe('useMatchingReferences', () => { }); it('should refetch matchingReferences when the pagination.page prop changes', () => { - const { dispatch, rerender } = renderHookWithRedux(() => { + const { dispatch, rerender } = renderHook(() => { return useMatchingReferences(defaultProps); }); @@ -343,7 +343,7 @@ describe('useMatchingReferences', () => { }); it('should refetch matchingReferences when the pagination.pagination prop changes', () => { - const { dispatch, rerender } = renderHookWithRedux(() => { + const { dispatch, rerender } = renderHook(() => { return useMatchingReferences(defaultProps); }); @@ -402,7 +402,7 @@ describe('useMatchingReferences', () => { }); it('should refetch matchingReferences when the sort.field prop changes', () => { - const { dispatch, rerender } = renderHookWithRedux(() => { + const { dispatch, rerender } = renderHook(() => { return useMatchingReferences(defaultProps); }); @@ -461,7 +461,7 @@ describe('useMatchingReferences', () => { }); it('should refetch matchingReferences when the sort.order prop changes', () => { - const { dispatch, rerender } = renderHookWithRedux(() => { + const { dispatch, rerender } = renderHook(() => { return useMatchingReferences(defaultProps); }); @@ -520,10 +520,11 @@ describe('useMatchingReferences', () => { }); it('should pass matching references from redux state to its children', () => { - const { hookValue } = renderHookWithRedux( + const { hookValue } = renderHook( () => { return useMatchingReferences(defaultProps); }, + true, { admin: { resources: { @@ -543,10 +544,11 @@ describe('useMatchingReferences', () => { }); it('should pass an error if an error is in redux state', () => { - const { hookValue } = renderHookWithRedux( + const { hookValue } = renderHook( () => { return useMatchingReferences(defaultProps); }, + true, { admin: { resources: { @@ -570,10 +572,11 @@ describe('useMatchingReferences', () => { }); it('should pass loading true if no matching reference yet', () => { - const { hookValue } = renderHookWithRedux( + const { hookValue } = renderHook( () => { return useMatchingReferences(defaultProps); }, + true, { admin: { resources: { diff --git a/packages/ra-core/src/util/renderHook.tsx b/packages/ra-core/src/util/renderHook.tsx index e6026b3bbd2..f239fe4c37e 100644 --- a/packages/ra-core/src/util/renderHook.tsx +++ b/packages/ra-core/src/util/renderHook.tsx @@ -1,17 +1,42 @@ import React from 'react'; -import { render } from 'react-testing-library'; +import { render, RenderResult } from 'react-testing-library'; + +import renderWithRedux, { RenderWithReduxResult } from './renderWithRedux'; const TestHook = ({ children, hook }) => { return children(hook()); }; -export default hook => { + +interface RenderHookResult extends RenderResult { + hookValue: any; + childrenMock: jest.Mock; + rerender: (f: any) => any; +} +interface RenderHookWithReduxResult extends RenderWithReduxResult { + hookValue: any; + childrenMock: jest.Mock; + rerender: (f: any) => any; +} + +function renderHook( + hook: Function, + withRedux?: true, + reduxState?: {} +): RenderHookWithReduxResult; +function renderHook(hook: Function, withRedux: false): RenderHookResult; +function renderHook(hook, withRedux = true, reduxState?) { let hookValue = null; const children = props => { hookValue = props; return

child

; }; const childrenMock = jest.fn().mockImplementation(children); - const result = render(); + const result = withRedux + ? renderWithRedux( + , + reduxState + ) + : render(); return { ...result, @@ -23,4 +48,6 @@ export default hook => { ); }, }; -}; +} + +export default renderHook; diff --git a/packages/ra-core/src/util/renderHookWithRedux.tsx b/packages/ra-core/src/util/renderHookWithRedux.tsx deleted file mode 100644 index 1185998851d..00000000000 --- a/packages/ra-core/src/util/renderHookWithRedux.tsx +++ /dev/null @@ -1,27 +0,0 @@ -import React from 'react'; -import renderWithRedux from './renderWithRedux'; - -const TestHook = ({ children, hook }) => { - return children(hook()); -}; -export default (hook, reduxState?) => { - let hookValue = null; - const children = props => { - hookValue = props; - return

child

; - }; - const result = renderWithRedux( - , - reduxState - ); - - return { - ...result, - hookValue, - rerender: newHook => { - return result.rerender( - - ); - }, - }; -}; diff --git a/packages/ra-core/src/util/renderWithRedux.tsx b/packages/ra-core/src/util/renderWithRedux.tsx index b62582c5979..0ebe561a145 100644 --- a/packages/ra-core/src/util/renderWithRedux.tsx +++ b/packages/ra-core/src/util/renderWithRedux.tsx @@ -1,8 +1,13 @@ import React from 'react'; -import { render } from 'react-testing-library'; +import { render, RenderResult } from 'react-testing-library'; import TestContext from './TestContext'; +export interface RenderWithReduxResult extends RenderResult { + dispatch: jest.Mock; + reduxStore: any; +} + /** * render with react-testing library adding redux context for unit test. * @example @@ -18,7 +23,7 @@ import TestContext from './TestContext'; * dispatch: spy on the redux stroe dispatch method * reduxStore: the redux store used by the tested component */ -export default (component, initialState = {}) => { +export default (component, initialState = {}): RenderWithReduxResult => { let dispatch; let reduxStore; const renderResult = render( From d4653df1d6061be99d69bc1da6446dea13706eff Mon Sep 17 00:00:00 2001 From: thiery Date: Wed, 10 Jul 2019 11:29:22 +0200 Subject: [PATCH 36/48] add jsdoc on renderHook --- packages/ra-core/src/util/renderHook.tsx | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/packages/ra-core/src/util/renderHook.tsx b/packages/ra-core/src/util/renderHook.tsx index f239fe4c37e..11f31561005 100644 --- a/packages/ra-core/src/util/renderHook.tsx +++ b/packages/ra-core/src/util/renderHook.tsx @@ -18,6 +18,15 @@ interface RenderHookWithReduxResult extends RenderWithReduxResult { rerender: (f: any) => any; } +/** + * render given hook using react-testing-library and return hook value + * @param hook the hook to render + * @param withRedux should we provide a redux context default to true + * @param reduxState optional initial state for redux context + * + * @returns {RenderHookResult} + * @returns {RenderHookWithReduxResult} + */ function renderHook( hook: Function, withRedux?: true, From 11b06838e55bf4d527c5bc7acfb9413bd8196902 Mon Sep 17 00:00:00 2001 From: thiery Date: Wed, 10 Jul 2019 11:34:25 +0200 Subject: [PATCH 37/48] update useReference and useReferenceField hook doc --- packages/ra-core/src/controller/field/useReferenceField.ts | 4 ++-- packages/ra-core/src/controller/useReference.ts | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/packages/ra-core/src/controller/field/useReferenceField.ts b/packages/ra-core/src/controller/field/useReferenceField.ts index bde907dfc73..5e470f96252 100644 --- a/packages/ra-core/src/controller/field/useReferenceField.ts +++ b/packages/ra-core/src/controller/field/useReferenceField.ts @@ -36,14 +36,14 @@ export interface UseReferenceProps { */ /** - * Fetch reference record, and return it when avaliable + * Fetch reference record, and return it when available also provide link toward the referenced resource * * The reference prop sould be the name of one of the components * added as child. * * @example * - * const { isLoading, referenceRecord, resourceLinkPath } = useReference({ + * const { loading, loaded, referenceRecord, resourceLinkPath } = useReference({ * source: 'userId', * reference: 'users', * record: { diff --git a/packages/ra-core/src/controller/useReference.ts b/packages/ra-core/src/controller/useReference.ts index 61ed7da670a..fca03a7147b 100644 --- a/packages/ra-core/src/controller/useReference.ts +++ b/packages/ra-core/src/controller/useReference.ts @@ -27,7 +27,7 @@ export interface UseReferenceProps { */ /** - * Fetch reference record, and return it when avaliable + * Fetch reference record, and return it when available * * The reference prop sould be the name of one of the components * added as child. From 79b11f655f35ebb4564f6ea6180227f811526844 Mon Sep 17 00:00:00 2001 From: thiery Date: Wed, 10 Jul 2019 12:15:53 +0200 Subject: [PATCH 38/48] add useReferenceInput hook --- docs/Inputs.md | 2 +- .../input/ReferenceInputController.tsx | 67 ++------- .../src/controller/input/useReferenceInput.ts | 132 ++++++++++++++++++ 3 files changed, 148 insertions(+), 53 deletions(-) create mode 100644 packages/ra-core/src/controller/input/useReferenceInput.ts diff --git a/docs/Inputs.md b/docs/Inputs.md index b71e9074d6f..98a15bd3f4f 100644 --- a/docs/Inputs.md +++ b/docs/Inputs.md @@ -922,7 +922,7 @@ The child component may further filter results (that's the case, for instance, f The child component receives the following props from ``: -- `isLoading`: whether the request for possible values is loading or not +- `loading`: whether the request for possible values is loading or not - `filter`: the current filter of the request for possible values. Defaults to `{}`. - `pagination`: the current pagination of the request for possible values. Defaults to `{ page: 1, perPage: 25 }`. - `sort`: the current sorting of the request for possible values. Defaults to `{ field: 'id', order: 'DESC' }`. diff --git a/packages/ra-core/src/controller/input/ReferenceInputController.tsx b/packages/ra-core/src/controller/input/ReferenceInputController.tsx index dc8ebe3524c..38015e69e5a 100644 --- a/packages/ra-core/src/controller/input/ReferenceInputController.tsx +++ b/packages/ra-core/src/controller/input/ReferenceInputController.tsx @@ -6,14 +6,9 @@ import { } from 'react'; import { WrappedFieldInputProps } from 'redux-form'; -import { getStatusForInput as getDataStatus } from './referenceDataStatus'; -import useTranslate from '../../i18n/useTranslate'; import { Sort, Record, Pagination } from '../../types'; -import useReference from '../useReference'; -import useMatchingReferences from './useMatchingReferences'; -import usePaginationState from '../usePaginationState'; -import { useSortState } from '..'; -import useFilterState from '../useFilterState'; +import useReferenceInput from './useReferenceInput'; +import { filter } from 'async'; const defaultReferenceSource = (resource: string, source: string) => `${resource}@${source}`; @@ -22,7 +17,7 @@ interface ChildrenFuncParams { choices: Record[]; error?: string; filter?: any; - isLoading: boolean; + loading: boolean; onChange: (value: any) => void; pagination: Pagination; setFilter: (filter: string) => void; @@ -130,7 +125,6 @@ interface Props { */ export const ReferenceInputController: FunctionComponent = ({ input, - onChange, children, perPage = 25, filter: permanentFilter = {}, @@ -139,51 +133,20 @@ export const ReferenceInputController: FunctionComponent = ({ referenceSource = defaultReferenceSource, resource, source, + ...props }) => { - const translate = useTranslate(); - - const { pagination, setPagination } = usePaginationState({ perPage }); - const { sort, setSort } = useSortState(); - const { filter, setFilter } = useFilterState({ - permanentFilter, - filterToQuery, - }); - - const { matchingReferences } = useMatchingReferences({ - reference, - referenceSource, - filter, - pagination, - sort, - resource, - source, - }); - - const { referenceRecord } = useReference({ - id: input.value, - reference, - allowEmpty: true, - }); - - const dataStatus = getDataStatus({ - input, - matchingReferences, - referenceRecord, - translate, - }); - return children({ - choices: dataStatus.choices, - error: dataStatus.error, - isLoading: dataStatus.waiting, - onChange, - setFilter, - filter, - pagination, - setPagination, - sort, - setSort, - warning: dataStatus.warning, + ...useReferenceInput({ + input, + perPage, + permanentFilter: filter, + reference, + filterToQuery, + referenceSource, + resource, + source, + }), + ...props, }) as ReactElement; }; diff --git a/packages/ra-core/src/controller/input/useReferenceInput.ts b/packages/ra-core/src/controller/input/useReferenceInput.ts new file mode 100644 index 00000000000..a284d1c14fe --- /dev/null +++ b/packages/ra-core/src/controller/input/useReferenceInput.ts @@ -0,0 +1,132 @@ +import { WrappedFieldInputProps } from 'redux-form'; + +import { getStatusForInput as getDataStatus } from './referenceDataStatus'; +import useTranslate from '../../i18n/useTranslate'; +import { Sort, Record, Pagination } from '../../types'; +import useReference from '../useReference'; +import useMatchingReferences from './useMatchingReferences'; +import usePaginationState from '../usePaginationState'; +import { useSortState } from '..'; +import useFilterState from '../useFilterState'; + +const defaultReferenceSource = (resource: string, source: string) => + `${resource}@${source}`; + +interface ReferenceInputValue { + choices: Record[]; + error?: string; + loading: boolean; + pagination: Pagination; + setFilter: (filter: string) => void; + setPagination: (pagination: Pagination) => void; + setSort: (sort: Sort) => void; + sort: Sort; + warning?: string; +} + +interface Option { + allowEmpty?: boolean; + permanentFilter?: any; + filterToQuery?: (filter: string) => any; + input?: WrappedFieldInputProps; + perPage?: number; + record?: Record; + reference: string; + referenceSource?: typeof defaultReferenceSource; + resource: string; + sort?: Sort; + source: string; +} + +/** + * A hook for choosing a reference record. Useful for foreign keys. + * + * This hook fetches the possible values in the reference resource + * (using the `CRUD_GET_MATCHING` REST method), it returns the possible choices + * as the `choices` attribute. + * + * @example + * const { + * choices, // the available reference resource + * } = useReferenceInput({ + * input, // the input props + * resource: 'comments', + * reference: 'posts', + * source: 'post_id', + * }); + * + * The hook alos allow to filter results. it returns a `setFilter` + * function. It uses the value to create a filter + * for the query - by default { q: [searchText] }. You can customize the mapping + * searchText => searchQuery by setting a custom `filterToQuery` function option + * You can also add a permanentFilter to further filter the result: + * + * @example + * const { + * choices, // the available reference resource + * setFilter, + * } = useReferenceInput({ + * input, // the input props + * resource: 'comments', + * reference: 'posts', + * source: 'post_id', + * permanentFilter: { + * author: 'john' + * }, + * filterToQuery: searchText => ({ title: searchText }) + * }); + */ +export default ({ + input, + perPage = 25, + permanentFilter = {}, + reference, + filterToQuery, + referenceSource = defaultReferenceSource, + resource, + source, +}: Option): ReferenceInputValue => { + const translate = useTranslate(); + + const { pagination, setPagination } = usePaginationState({ perPage }); + const { sort, setSort } = useSortState(); + const { filter, setFilter } = useFilterState({ + permanentFilter, + filterToQuery, + }); + + const { matchingReferences } = useMatchingReferences({ + reference, + referenceSource, + filter, + pagination, + sort, + resource, + source, + }); + + const { referenceRecord } = useReference({ + id: input.value, + reference, + allowEmpty: true, + }); + + const dataStatus = getDataStatus({ + input, + matchingReferences, + referenceRecord, + translate, + }); + + return { + choices: dataStatus.choices, + error: dataStatus.error, + loading: dataStatus.waiting, + setFilter, + pagination, + setPagination, + sort, + setSort, + warning: dataStatus.warning, + }; +}; From 7a1fd189d0716af3a9941ce85337d58280a620a3 Mon Sep 17 00:00:00 2001 From: thiery Date: Wed, 10 Jul 2019 13:27:20 +0200 Subject: [PATCH 39/48] code review --- .../input/ReferenceInputController.spec.tsx | 7 +++---- .../input/ReferenceInputController.tsx | 2 +- ....tsx => useGetMatchingReferences.spec.tsx} | 2 +- ...erences.ts => useGetMatchingReferences.ts} | 0 .../src/controller/input/useReferenceInput.ts | 6 ++++-- .../src/controller/useReference.spec.ts | 19 +++++++++++-------- 6 files changed, 20 insertions(+), 16 deletions(-) rename packages/ra-core/src/controller/input/{useMatchingReferences.spec.tsx => useGetMatchingReferences.spec.tsx} (99%) rename packages/ra-core/src/controller/input/{useMatchingReferences.ts => useGetMatchingReferences.ts} (100%) diff --git a/packages/ra-core/src/controller/input/ReferenceInputController.spec.tsx b/packages/ra-core/src/controller/input/ReferenceInputController.spec.tsx index 5c3254471a3..8ddea66de50 100644 --- a/packages/ra-core/src/controller/input/ReferenceInputController.spec.tsx +++ b/packages/ra-core/src/controller/input/ReferenceInputController.spec.tsx @@ -10,13 +10,11 @@ describe('', () => { const defaultProps = { basePath: '/comments', children: jest.fn(), - meta: {}, input: { value: undefined } as WrappedFieldInputProps, onChange: jest.fn(), reference: 'posts', resource: 'comments', source: 'post_id', - translate: x => `*${x}*`, }; afterEach(cleanup); @@ -28,7 +26,7 @@ describe('', () => { {...{ ...defaultProps, input: { value: 1 } as WrappedFieldInputProps, - isLoading: true, + loading: true, }} > {children} @@ -54,10 +52,11 @@ describe('', () => { choices: [{ id: 1 }], error: null, filter: { q: '' }, - isLoading: false, + loading: false, pagination: { page: 1, perPage: 25 }, sort: { field: 'id', order: 'DESC' }, warning: null, + basePath: '/comments', }); expect(dispatch).toBeCalledTimes(2); diff --git a/packages/ra-core/src/controller/input/ReferenceInputController.tsx b/packages/ra-core/src/controller/input/ReferenceInputController.tsx index 38015e69e5a..6aeb8f3d24b 100644 --- a/packages/ra-core/src/controller/input/ReferenceInputController.tsx +++ b/packages/ra-core/src/controller/input/ReferenceInputController.tsx @@ -136,6 +136,7 @@ export const ReferenceInputController: FunctionComponent = ({ ...props }) => { return children({ + ...props, ...useReferenceInput({ input, perPage, @@ -146,7 +147,6 @@ export const ReferenceInputController: FunctionComponent = ({ resource, source, }), - ...props, }) as ReactElement; }; diff --git a/packages/ra-core/src/controller/input/useMatchingReferences.spec.tsx b/packages/ra-core/src/controller/input/useGetMatchingReferences.spec.tsx similarity index 99% rename from packages/ra-core/src/controller/input/useMatchingReferences.spec.tsx rename to packages/ra-core/src/controller/input/useGetMatchingReferences.spec.tsx index f160ea8b3a9..42671423c1f 100644 --- a/packages/ra-core/src/controller/input/useMatchingReferences.spec.tsx +++ b/packages/ra-core/src/controller/input/useGetMatchingReferences.spec.tsx @@ -1,5 +1,5 @@ import renderHook from '../../util/renderHook'; -import useMatchingReferences from './useMatchingReferences'; +import useMatchingReferences from './useGetMatchingReferences'; import { cleanup } from 'react-testing-library'; describe('useMatchingReferences', () => { diff --git a/packages/ra-core/src/controller/input/useMatchingReferences.ts b/packages/ra-core/src/controller/input/useGetMatchingReferences.ts similarity index 100% rename from packages/ra-core/src/controller/input/useMatchingReferences.ts rename to packages/ra-core/src/controller/input/useGetMatchingReferences.ts diff --git a/packages/ra-core/src/controller/input/useReferenceInput.ts b/packages/ra-core/src/controller/input/useReferenceInput.ts index a284d1c14fe..ab28eead512 100644 --- a/packages/ra-core/src/controller/input/useReferenceInput.ts +++ b/packages/ra-core/src/controller/input/useReferenceInput.ts @@ -4,7 +4,7 @@ import { getStatusForInput as getDataStatus } from './referenceDataStatus'; import useTranslate from '../../i18n/useTranslate'; import { Sort, Record, Pagination } from '../../types'; import useReference from '../useReference'; -import useMatchingReferences from './useMatchingReferences'; +import useGetMatchingReferences from './useGetMatchingReferences'; import usePaginationState from '../usePaginationState'; import { useSortState } from '..'; import useFilterState from '../useFilterState'; @@ -18,6 +18,7 @@ interface ReferenceInputValue { loading: boolean; pagination: Pagination; setFilter: (filter: string) => void; + filter: any; setPagination: (pagination: Pagination) => void; setSort: (sort: Sort) => void; sort: Sort; @@ -95,7 +96,7 @@ export default ({ filterToQuery, }); - const { matchingReferences } = useMatchingReferences({ + const { matchingReferences } = useGetMatchingReferences({ reference, referenceSource, filter, @@ -122,6 +123,7 @@ export default ({ choices: dataStatus.choices, error: dataStatus.error, loading: dataStatus.waiting, + filter, setFilter, pagination, setPagination, diff --git a/packages/ra-core/src/controller/useReference.spec.ts b/packages/ra-core/src/controller/useReference.spec.ts index 7cac71e696e..1cb90334353 100644 --- a/packages/ra-core/src/controller/useReference.spec.ts +++ b/packages/ra-core/src/controller/useReference.spec.ts @@ -1,4 +1,4 @@ -import renderHookWithRedux from '../util/renderHookWithRedux'; +import renderHook from '../util/renderHook'; import useReference from './useReference'; import { cleanup } from 'react-testing-library'; @@ -12,7 +12,7 @@ describe('useReference', () => { afterEach(cleanup); it('should fetch reference on mount', () => { - const { dispatch, rerender } = renderHookWithRedux(() => { + const { dispatch, rerender } = renderHook(() => { return useReference(defaultProps); }); @@ -28,7 +28,7 @@ describe('useReference', () => { }); it('should refetch reference when id changes', () => { - const { dispatch, rerender } = renderHookWithRedux(() => { + const { dispatch, rerender } = renderHook(() => { return useReference(defaultProps); }); @@ -55,7 +55,7 @@ describe('useReference', () => { }); it('should refetch reference when reference prop changes', () => { - const { dispatch, rerender } = renderHookWithRedux(() => { + const { dispatch, rerender } = renderHook(() => { return useReference(defaultProps); }); @@ -82,7 +82,7 @@ describe('useReference', () => { }); it('it should not refetch reference when allowEmpty change', () => { - const { dispatch, rerender } = renderHookWithRedux(() => { + const { dispatch, rerender } = renderHook(() => { return useReference(defaultProps); }); @@ -102,10 +102,11 @@ describe('useReference', () => { }); it('should retrieve referenceRecord from redux state', () => { - const { hookValue } = renderHookWithRedux( + const { hookValue } = renderHook( () => { return useReference(defaultProps); }, + true, { admin: { resources: { @@ -123,10 +124,11 @@ describe('useReference', () => { }); it('should set loading to true if no referenceRecord yet', () => { - const { hookValue } = renderHookWithRedux( + const { hookValue } = renderHook( () => { return useReference(defaultProps); }, + true, { admin: { resources: { @@ -144,10 +146,11 @@ describe('useReference', () => { }); it('should set loading to false even if no referenceRecord yet when allowEmpty is true', () => { - const { hookValue } = renderHookWithRedux( + const { hookValue } = renderHook( () => { return useReference({ ...defaultProps, allowEmpty: true }); }, + true, { admin: { resources: { From 7f33c2fb6690d77c43637d9db79dca5cbfe92f49 Mon Sep 17 00:00:00 2001 From: thiery Date: Wed, 10 Jul 2019 13:50:41 +0200 Subject: [PATCH 40/48] have sort reducer use action --- .../ra-core/src/controller/useSortState.ts | 53 ++++++++++++++----- 1 file changed, 40 insertions(+), 13 deletions(-) diff --git a/packages/ra-core/src/controller/useSortState.ts b/packages/ra-core/src/controller/useSortState.ts index 42dd9bd37ce..3c5b9f7fcd5 100644 --- a/packages/ra-core/src/controller/useSortState.ts +++ b/packages/ra-core/src/controller/useSortState.ts @@ -8,21 +8,44 @@ import { Sort } from '../types'; interface SortProps { setSortField: (field: string) => void; + setSortOrder: (order: string) => void; setSort: (sort: Sort) => void; sort: Sort; } -const sortReducer = (state: Sort, field: string | Sort): Sort => { - if (typeof field !== 'string') { - return field; +interface Action { + type: 'SET_SORT' | 'SET_SORT_FIELD' | 'SET_SORT_ORDER'; + payload: { + sort?: Sort; + field?: string; + order?: string; + }; +} + +const sortReducer = (state: Sort, action: Action): Sort => { + switch (action.type) { + case 'SET_SORT': + return action.payload.sort; + case 'SET_SORT_FIELD': { + const { field } = action.payload; + const order = + state.field === field + ? state.order === SORT_ASC + ? SORT_DESC + : SORT_ASC + : SORT_ASC; + return { field, order }; + } + case 'SET_SORT_ORDER': { + const { order } = action.payload; + return { + ...state, + order, + }; + } + default: + return state; } - const order = - state.field === field - ? state.order === SORT_ASC - ? SORT_DESC - : SORT_ASC - : SORT_ASC; - return { field, order }; }; export const defaultSort = { field: 'id', order: 'DESC' }; @@ -63,12 +86,16 @@ export default (initialSort: Sort = defaultSort): SortProps => { isFirstRender.current = false; return; } - dispatch(initialSort); + dispatch({ type: 'SET_SORT', payload: { sort: initialSort } }); }, [initialSort.field, initialSort.order]); // eslint-disable-line react-hooks/exhaustive-deps return { - setSort: (sortValue: Sort) => dispatch(sortValue), - setSortField: (field: string) => dispatch(field), + setSort: (sort: Sort) => + dispatch({ type: 'SET_SORT', payload: { sort } }), + setSortField: (field: string) => + dispatch({ type: 'SET_SORT_FIELD', payload: { field } }), + setSortOrder: (order: string) => + dispatch({ type: 'SET_SORT_ORDER', payload: { order } }), sort, }; }; From 1d69a5ca2f3a0c92f4b426a29bc678ab3861607d Mon Sep 17 00:00:00 2001 From: thiery Date: Wed, 10 Jul 2019 13:59:35 +0200 Subject: [PATCH 41/48] add setSort usage in jsdoc --- .../ra-core/src/controller/useSortState.ts | 34 ++++++++++++++++--- 1 file changed, 29 insertions(+), 5 deletions(-) diff --git a/packages/ra-core/src/controller/useSortState.ts b/packages/ra-core/src/controller/useSortState.ts index 3c5b9f7fcd5..aca320d09fb 100644 --- a/packages/ra-core/src/controller/useSortState.ts +++ b/packages/ra-core/src/controller/useSortState.ts @@ -51,10 +51,24 @@ const sortReducer = (state: Sort, action: Action): Sort => { export const defaultSort = { field: 'id', order: 'DESC' }; /** - * set the sort to the given field, swap the order if the field is the same + * set the sort { field, order } * @name setSort * @function - * @param {string} field the name of the field to sort + * @param {Sort} sort the sort object + */ + +/** + * set the sort field, swap the order if the field is the same + * @name setSortField + * @function + * @param {string} field the sort field + */ + +/** + * set the sort order + * @name setSortOrder + * @function + * @param {string} order the sort order eiather ASC or DESC */ /** @@ -64,6 +78,8 @@ export const defaultSort = { field: 'id', order: 'DESC' }; * @property {String} sort.field: the sort object. * @property {'ASC' | 'DESC'} sort.order: the sort object. * @property {setSort} setSort + * @property {setSortField} setSortField + * @property {setSortOrder} setSortOrder */ /** @@ -71,11 +87,19 @@ export const defaultSort = { field: 'id', order: 'DESC' }; * * @example * - * const { sort, setSort } = useSort({ field: 'name',order: 'ASC' }); + * const { sort, setSort, setSortField, setSortOrder } = useSort({ + * field: 'name', + * order: 'ASC', + * }); + * + * setSort({ field: 'name', order: 'ASC' }); + * // is the same as + * setSortField('name'); + * setSortOrder('ASC'); * * @param {Object} initialSort - * @param {string} initialSort.resource The current resource name - * @param {string} initialSort.reference The linked resource name + * @param {string} initialSort.field The initial sort field + * @param {string} initialSort.order The initial sort order * @returns {SortProps} The sort props */ export default (initialSort: Sort = defaultSort): SortProps => { From 0a3535dfbf23a201695e7ad324b9357e05957593 Mon Sep 17 00:00:00 2001 From: thiery Date: Wed, 10 Jul 2019 15:16:32 +0200 Subject: [PATCH 42/48] replace useReferenceField by getResourceLinkPath without useReference --- .../field/ReferenceFieldController.tsx | 18 ++++++++----- ...ferenceField.ts => getResourceLinkPath.ts} | 27 +++---------------- .../ra-core/src/controller/field/index.ts | 4 +-- packages/ra-core/src/controller/index.ts | 2 ++ .../src/field/ReferenceField.js | 7 +++-- 5 files changed, 23 insertions(+), 35 deletions(-) rename packages/ra-core/src/controller/field/{useReferenceField.ts => getResourceLinkPath.ts} (83%) diff --git a/packages/ra-core/src/controller/field/ReferenceFieldController.tsx b/packages/ra-core/src/controller/field/ReferenceFieldController.tsx index 2116e6719ef..4be7bee2f11 100644 --- a/packages/ra-core/src/controller/field/ReferenceFieldController.tsx +++ b/packages/ra-core/src/controller/field/ReferenceFieldController.tsx @@ -1,15 +1,18 @@ import { FunctionComponent, ReactNode, ReactElement } from 'react'; import { Record } from '../../types'; -import useReferenceField, { - UseReferenceProps, - LinkToFunctionType, -} from './useReferenceField'; +import getResourceLinkPath, { LinkToFunctionType } from './getResourceLinkPath'; +import useReference, { UseReferenceProps } from '../useReference'; + +interface childrenParams extends UseReferenceProps { + resourceLinkPath: string | false; +} interface Props { + id: string; allowEmpty?: boolean; basePath: string; - children: (params: UseReferenceProps) => ReactNode; + children: (params: childrenParams) => ReactNode; record?: Record; reference: string; resource: string; @@ -50,7 +53,10 @@ export const ReferenceFieldController: FunctionComponent = ({ children, ...props }) => { - return children(useReferenceField(props)) as ReactElement; + return children({ + ...useReference(props), + resourceLinkPath: getResourceLinkPath(props), + }) as ReactElement; }; export default ReferenceFieldController; diff --git a/packages/ra-core/src/controller/field/useReferenceField.ts b/packages/ra-core/src/controller/field/getResourceLinkPath.ts similarity index 83% rename from packages/ra-core/src/controller/field/useReferenceField.ts rename to packages/ra-core/src/controller/field/getResourceLinkPath.ts index 5e470f96252..82c23463d10 100644 --- a/packages/ra-core/src/controller/field/useReferenceField.ts +++ b/packages/ra-core/src/controller/field/getResourceLinkPath.ts @@ -2,7 +2,6 @@ import get from 'lodash/get'; import { linkToRecord } from '../../util'; import { Record } from '../../types'; -import useReference from '../useReference'; export type LinkToFunctionType = (record: Record, reference: string) => string; @@ -19,13 +18,6 @@ interface Option { linkType?: LinkToType; // deprecated, use link instead } -export interface UseReferenceProps { - loading: boolean; - loaded: boolean; - referenceRecord: Record; - resourceLinkPath: string | false; -} - /** * @typedef ReferenceProps * @type {Object} @@ -63,8 +55,7 @@ export interface UseReferenceProps { * * @returns {ReferenceProps} The reference props */ -export const useReferenceField = ({ - allowEmpty = false, +export const getResourceLinkPath = ({ basePath, link = 'edit', linkType, @@ -72,13 +63,8 @@ export const useReferenceField = ({ record = { id: '' }, resource, source, -}: Option): UseReferenceProps => { +}: Option): string | false => { const sourceId = get(record, source); - const { referenceRecord, loading, loaded } = useReference({ - id: sourceId, - reference, - allowEmpty, - }); const rootPath = basePath.replace(resource, reference); // Backward compatibility: keep linkType but with warning const getResourceLinkPath = (linkTo: LinkToType) => @@ -98,12 +84,7 @@ export const useReferenceField = ({ linkType !== undefined ? linkType : link ); - return { - loading, - loaded, - referenceRecord, - resourceLinkPath, - }; + return resourceLinkPath; }; -export default useReferenceField; +export default getResourceLinkPath; diff --git a/packages/ra-core/src/controller/field/index.ts b/packages/ra-core/src/controller/field/index.ts index 05ba1ad6aa2..341d655ad65 100644 --- a/packages/ra-core/src/controller/field/index.ts +++ b/packages/ra-core/src/controller/field/index.ts @@ -1,7 +1,7 @@ import ReferenceArrayFieldController from './ReferenceArrayFieldController'; import ReferenceFieldController from './ReferenceFieldController'; import ReferenceManyFieldController from './ReferenceManyFieldController'; -import useReferenceField from './useReferenceField'; +import getResourceLinkPath from './getResourceLinkPath'; import useReferenceArray from './useReferenceArray'; import useReferenceMany from './useReferenceMany'; @@ -9,7 +9,7 @@ export { useReferenceArray, ReferenceArrayFieldController, ReferenceFieldController, - useReferenceField, + getResourceLinkPath, useReferenceMany, ReferenceManyFieldController, }; diff --git a/packages/ra-core/src/controller/index.ts b/packages/ra-core/src/controller/index.ts index 1da352fc3b7..4212f351fd3 100644 --- a/packages/ra-core/src/controller/index.ts +++ b/packages/ra-core/src/controller/index.ts @@ -14,6 +14,7 @@ import useListController from './useListController'; import useEditController from './useEditController'; import useCreateController from './useCreateController'; import useShowController from './useShowController'; +import useReference from './useReference'; import { useCheckMinimumRequiredProps } from './checkMinimumRequiredProps'; export { getListControllerProps, @@ -31,6 +32,7 @@ export { useVersion, useSortState, usePaginationState, + useReference, }; export * from './field'; diff --git a/packages/ra-ui-materialui/src/field/ReferenceField.js b/packages/ra-ui-materialui/src/field/ReferenceField.js index def391fcf2b..2c4b3ec1097 100644 --- a/packages/ra-ui-materialui/src/field/ReferenceField.js +++ b/packages/ra-ui-materialui/src/field/ReferenceField.js @@ -2,7 +2,7 @@ import React, { Children } from 'react'; import PropTypes from 'prop-types'; import classnames from 'classnames'; import { withStyles, createStyles } from '@material-ui/core/styles'; -import { useReferenceField } from 'ra-core'; +import { useReference, getResourceLinkPath } from 'ra-core'; import LinearProgress from '../layout/LinearProgress'; import Link from '../Link'; @@ -137,9 +137,8 @@ const ReferenceField = ({ children, ...props }) => { throw new Error(' only accepts a single child'); } - const { isLoading, referenceRecord, resourceLinkPath } = useReferenceField( - props - ); + const { isLoading, referenceRecord } = useReference(props); + const resourceLinkPath = getResourceLinkPath(props); return ( Date: Wed, 10 Jul 2019 15:23:04 +0200 Subject: [PATCH 43/48] code review --- packages/ra-core/src/controller/usePaginationState.ts | 11 +++++++---- packages/ra-core/src/controller/useSortState.spec.ts | 2 +- 2 files changed, 8 insertions(+), 5 deletions(-) diff --git a/packages/ra-core/src/controller/usePaginationState.ts b/packages/ra-core/src/controller/usePaginationState.ts index e3a42735f53..da6e7c0f646 100644 --- a/packages/ra-core/src/controller/usePaginationState.ts +++ b/packages/ra-core/src/controller/usePaginationState.ts @@ -1,4 +1,4 @@ -import { useState, useEffect, useReducer, useMemo, useRef } from 'react'; +import { useState, useEffect, useReducer, useCallback, useRef } from 'react'; import { Pagination } from '../types'; interface PaginationProps { @@ -10,7 +10,10 @@ interface PaginationProps { setPagination: (pagination: Pagination) => void; } -const paginationReducer = (prevState, nextState) => { +const paginationReducer = ( + prevState: Pagination, + nextState: Partial +): Pagination => { return { ...prevState, ...nextState, @@ -56,8 +59,8 @@ export default ( }); const isFirstRender = useRef(true); - const setPerPage = useMemo(() => perPage => setPagination({ perPage }), []); - const setPage = useMemo(() => page => setPagination({ page }), []); + const setPerPage = useCallback(perPage => setPagination({ perPage }), []); + const setPage = useCallback(page => setPagination({ page }), []); useEffect(() => { if (isFirstRender.current) { diff --git a/packages/ra-core/src/controller/useSortState.spec.ts b/packages/ra-core/src/controller/useSortState.spec.ts index fd7ef836375..615e932912b 100644 --- a/packages/ra-core/src/controller/useSortState.spec.ts +++ b/packages/ra-core/src/controller/useSortState.spec.ts @@ -34,7 +34,7 @@ describe('useSortState', () => { }); }); - describe('should provide setSortField method that', () => { + describe('setSortField in return value', () => { it('should just change the order if receiving the current field', () => { const { hookValue, childrenMock } = renderHook(() => useSortState({ field: 'id', order: 'DESC' }) From d71b5abc1eb9b10cbc31b2a21d853580bcccbe55 Mon Sep 17 00:00:00 2001 From: thiery Date: Wed, 10 Jul 2019 16:46:32 +0200 Subject: [PATCH 44/48] refactor useGetMatchingReference to stop using createSelector --- .../input/useGetMatchingReferences.ts | 71 +++++++++---------- .../src/controller/input/useReferenceInput.ts | 1 + 2 files changed, 36 insertions(+), 36 deletions(-) diff --git a/packages/ra-core/src/controller/input/useGetMatchingReferences.ts b/packages/ra-core/src/controller/input/useGetMatchingReferences.ts index 049685f043e..a119bbabde9 100644 --- a/packages/ra-core/src/controller/input/useGetMatchingReferences.ts +++ b/packages/ra-core/src/controller/input/useGetMatchingReferences.ts @@ -1,7 +1,6 @@ -import { useMemo } from 'react'; +import { useMemo, useCallback } from 'react'; // @ts-ignore import { useSelector, useDispatch } from 'react-redux'; -import { createSelector } from 'reselect'; import { Filter } from '../useFilterState'; import { crudGetMatchingAccumulate } from '../../actions/accumulateActions'; @@ -21,6 +20,7 @@ interface UseMatchingReferencesOption { filter: Filter; pagination: Pagination; sort: Sort; + id: string; } interface UseMatchingReferencesProps { @@ -40,22 +40,10 @@ export default ({ filter, pagination, sort, + id, }: UseMatchingReferencesOption): UseMatchingReferencesProps => { const dispatch = useDispatch(); - const getMatchingReferences = useMemo(makeMatchingReferencesSelector, []); - - const options = { - dispatch, - filter, - reference, - referenceSource, - resource, - source, - pagination, - sort, - }; - useDeepCompareEffect(() => { dispatch( crudGetMatchingAccumulate( @@ -77,15 +65,14 @@ export default ({ sort, ]); - const matchingReferences = useSelector( - getMatchingReferences({ - referenceSource, - filter, - reference, - resource, - source, - }) - ); + const matchingReferences = useGetMatchingReferenceSelector({ + referenceSource, + filter, + reference, + resource, + source, + id, + }); if (!matchingReferences) { return { @@ -110,19 +97,31 @@ export default ({ }; }; -const makeMatchingReferencesSelector = () => { - const matchingReferencesSelector = createSelector( - [ - getReferenceResource, - getPossibleReferenceValues, - (_, props) => props.filterValue, - ], - (referenceState, possibleValues, inputId) => { - return getPossibleReferences(referenceState, possibleValues, [ - inputId, +const useGetMatchingReferenceSelector = ({ + referenceSource, + filter, + reference, + resource, + source, + id, +}) => { + const getMatchingReferences = useCallback( + state => { + const referenceResource = getReferenceResource(state, { + reference, + }); + const possibleValues = getPossibleReferenceValues(state, { + referenceSource, + resource, + source, + }); + + return getPossibleReferences(referenceResource, possibleValues, [ + id, ]); - } + }, + [referenceSource, reference, resource, source, id] ); - return props => state => matchingReferencesSelector(state, props); + return useSelector(getMatchingReferences); }; diff --git a/packages/ra-core/src/controller/input/useReferenceInput.ts b/packages/ra-core/src/controller/input/useReferenceInput.ts index ab28eead512..534571178e8 100644 --- a/packages/ra-core/src/controller/input/useReferenceInput.ts +++ b/packages/ra-core/src/controller/input/useReferenceInput.ts @@ -104,6 +104,7 @@ export default ({ sort, resource, source, + id: input.value, }); const { referenceRecord } = useReference({ From 0f51d8a2c6764e6ca710c6283b7c073df5fa1e8e Mon Sep 17 00:00:00 2001 From: thiery Date: Wed, 10 Jul 2019 16:59:19 +0200 Subject: [PATCH 45/48] fix ReferenceManyField --- packages/ra-ui-materialui/src/field/ReferenceManyField.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/ra-ui-materialui/src/field/ReferenceManyField.js b/packages/ra-ui-materialui/src/field/ReferenceManyField.js index e9a48e32d8e..1a02b542c0c 100644 --- a/packages/ra-ui-materialui/src/field/ReferenceManyField.js +++ b/packages/ra-ui-materialui/src/field/ReferenceManyField.js @@ -125,7 +125,7 @@ export const ReferenceManyField = props => { } const { sort, setSort } = useSortState(initialSort); const { page, perPage, setPage, setPerPage } = usePaginationState({ - page: initialPerPage, + perPage: initialPerPage, }); const useReferenceManyProps = useReferenceMany({ From 4bcb9514e3fdb6cb6ddb81fe0f3e0c770deaaa47 Mon Sep 17 00:00:00 2001 From: thiery Date: Wed, 10 Jul 2019 17:20:12 +0200 Subject: [PATCH 46/48] fix ReferenceFieldCOntroller --- .../src/controller/field/ReferenceFieldController.tsx | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/packages/ra-core/src/controller/field/ReferenceFieldController.tsx b/packages/ra-core/src/controller/field/ReferenceFieldController.tsx index 4be7bee2f11..b7deac98262 100644 --- a/packages/ra-core/src/controller/field/ReferenceFieldController.tsx +++ b/packages/ra-core/src/controller/field/ReferenceFieldController.tsx @@ -1,4 +1,6 @@ import { FunctionComponent, ReactNode, ReactElement } from 'react'; +import get from 'lodash/get'; + import { Record } from '../../types'; import getResourceLinkPath, { LinkToFunctionType } from './getResourceLinkPath'; @@ -9,7 +11,6 @@ interface childrenParams extends UseReferenceProps { } interface Props { - id: string; allowEmpty?: boolean; basePath: string; children: (params: childrenParams) => ReactNode; @@ -51,11 +52,14 @@ interface Props { */ export const ReferenceFieldController: FunctionComponent = ({ children, + record, + source, ...props }) => { + const id = get(record, source); return children({ - ...useReference(props), - resourceLinkPath: getResourceLinkPath(props), + ...useReference({ ...props, id }), + resourceLinkPath: getResourceLinkPath({ ...props, record, source }), }) as ReactElement; }; From 173cb123e2ba686ecbe0ad1e48239b1d616797d2 Mon Sep 17 00:00:00 2001 From: thiery Date: Thu, 18 Jul 2019 13:24:06 +0200 Subject: [PATCH 47/48] code review --- UPGRADE.md | 23 ++++++++++++++ .../controller/field/getResourceLinkPath.ts | 25 ++++++++-------- .../input/ReferenceInputController.spec.tsx | 1 - .../input/ReferenceInputController.tsx | 30 +++++-------------- .../input/useGetMatchingReferences.spec.tsx | 8 ++++- .../src/controller/input/useReferenceInput.ts | 4 +-- .../src/input/ReferenceInput.js | 6 ++-- .../src/input/ReferenceInput.spec.js | 2 +- 8 files changed, 55 insertions(+), 44 deletions(-) diff --git a/UPGRADE.md b/UPGRADE.md index 9a5661534a9..6675e85e35b 100644 --- a/UPGRADE.md +++ b/UPGRADE.md @@ -309,3 +309,26 @@ export default (type, params) => { // ... } ``` + +## ReferenceInputController isLoading injected props renamed to loading + +When using custom component with ReferenceInputController, you should rename the component `isLoading` props to `loading`. + +```diff +- +- {({ isLoading, otherProps }) => ( +- +- )} +- ++ ++ {({ loading, otherProps }) => ( ++ ++ )} ++ +``` \ No newline at end of file diff --git a/packages/ra-core/src/controller/field/getResourceLinkPath.ts b/packages/ra-core/src/controller/field/getResourceLinkPath.ts index 82c23463d10..5099288d959 100644 --- a/packages/ra-core/src/controller/field/getResourceLinkPath.ts +++ b/packages/ra-core/src/controller/field/getResourceLinkPath.ts @@ -28,34 +28,33 @@ interface Option { */ /** - * Fetch reference record, and return it when available also provide link toward the referenced resource - * - * The reference prop sould be the name of one of the components - * added as child. + * Get the link toward the referenced resource * * @example * - * const { loading, loaded, referenceRecord, resourceLinkPath } = useReference({ - * source: 'userId', - * reference: 'users', - * record: { - * userId: 7 - * } + * const linkPath = getResourceLinkPath({ + * basePath: '/comments', + * link: 'edit', + * reference: 'users', + * record: { + * userId: 7 + * }, + * resource: 'comments', + * source: 'userId', * }); * * @param {Object} option - * @param {boolean} option.allowEmpty do we allow for no referenced record (default to false) * @param {string} option.basePath basepath to current resource * @param {string | false | LinkToFunctionType} option.link="edit" The link toward the referenced record. 'edit', 'show' or false for no link (default to edit). Alternatively a function that returns a string * @param {string | false | LinkToFunctionType} [option.linkType] DEPRECATED : old name for link - * @param {Object} option.record The The current resource record * @param {string} option.reference The linked resource name + * @param {Object} option.record The The current resource record * @param {string} option.resource The current resource name * @param {string} option.source The key of the linked resource identifier * * @returns {ReferenceProps} The reference props */ -export const getResourceLinkPath = ({ +const getResourceLinkPath = ({ basePath, link = 'edit', linkType, diff --git a/packages/ra-core/src/controller/input/ReferenceInputController.spec.tsx b/packages/ra-core/src/controller/input/ReferenceInputController.spec.tsx index 8ddea66de50..f742b3a3d1c 100644 --- a/packages/ra-core/src/controller/input/ReferenceInputController.spec.tsx +++ b/packages/ra-core/src/controller/input/ReferenceInputController.spec.tsx @@ -56,7 +56,6 @@ describe('', () => { pagination: { page: 1, perPage: 25 }, sort: { field: 'id', order: 'DESC' }, warning: null, - basePath: '/comments', }); expect(dispatch).toBeCalledTimes(2); diff --git a/packages/ra-core/src/controller/input/ReferenceInputController.tsx b/packages/ra-core/src/controller/input/ReferenceInputController.tsx index 6aeb8f3d24b..e343b3fb034 100644 --- a/packages/ra-core/src/controller/input/ReferenceInputController.tsx +++ b/packages/ra-core/src/controller/input/ReferenceInputController.tsx @@ -6,31 +6,17 @@ import { } from 'react'; import { WrappedFieldInputProps } from 'redux-form'; -import { Sort, Record, Pagination } from '../../types'; -import useReferenceInput from './useReferenceInput'; +import { Sort, Record } from '../../types'; +import useReferenceInput, { ReferenceInputValue } from './useReferenceInput'; import { filter } from 'async'; const defaultReferenceSource = (resource: string, source: string) => `${resource}@${source}`; -interface ChildrenFuncParams { - choices: Record[]; - error?: string; - filter?: any; - loading: boolean; - onChange: (value: any) => void; - pagination: Pagination; - setFilter: (filter: string) => void; - setPagination: (pagination: Pagination) => void; - setSort: (sort: Sort) => void; - sort: Sort; - warning?: string; -} - interface Props { allowEmpty?: boolean; basePath: string; - children: (params: ChildrenFuncParams) => ReactNode; + children: (params: ReferenceInputValue) => ReactNode; filter?: any; filterToQuery?: (filter: string) => any; input?: WrappedFieldInputProps; @@ -133,11 +119,9 @@ export const ReferenceInputController: FunctionComponent = ({ referenceSource = defaultReferenceSource, resource, source, - ...props }) => { - return children({ - ...props, - ...useReferenceInput({ + return children( + useReferenceInput({ input, perPage, permanentFilter: filter, @@ -146,8 +130,8 @@ export const ReferenceInputController: FunctionComponent = ({ referenceSource, resource, source, - }), - }) as ReactElement; + }) + ) as ReactElement; }; export default ReferenceInputController as ComponentType; diff --git a/packages/ra-core/src/controller/input/useGetMatchingReferences.spec.tsx b/packages/ra-core/src/controller/input/useGetMatchingReferences.spec.tsx index 42671423c1f..03a72e3aa1e 100644 --- a/packages/ra-core/src/controller/input/useGetMatchingReferences.spec.tsx +++ b/packages/ra-core/src/controller/input/useGetMatchingReferences.spec.tsx @@ -18,7 +18,7 @@ describe('useMatchingReferences', () => { afterEach(cleanup); - it('should fetch matchingReferences on mount', () => { + it('should fetch matchingReferences only on mount', () => { const { dispatch, rerender } = renderHook(() => { return useMatchingReferences(defaultProps); }); @@ -44,6 +44,12 @@ describe('useMatchingReferences', () => { q: '', }, }); + }); + + it('should not fetch matchingReferences on subsequent rerender', () => { + const { dispatch, rerender } = renderHook(() => { + return useMatchingReferences(defaultProps); + }); rerender(() => { return useMatchingReferences({ diff --git a/packages/ra-core/src/controller/input/useReferenceInput.ts b/packages/ra-core/src/controller/input/useReferenceInput.ts index 534571178e8..7071737d9c5 100644 --- a/packages/ra-core/src/controller/input/useReferenceInput.ts +++ b/packages/ra-core/src/controller/input/useReferenceInput.ts @@ -12,7 +12,7 @@ import useFilterState from '../useFilterState'; const defaultReferenceSource = (resource: string, source: string) => `${resource}@${source}`; -interface ReferenceInputValue { +export interface ReferenceInputValue { choices: Record[]; error?: string; loading: boolean; @@ -56,7 +56,7 @@ interface Option { * source: 'post_id', * }); * - * The hook alos allow to filter results. it returns a `setFilter` + * The hook also allow to filter results. It returns a `setFilter` * function. It uses the value to create a filter * for the query - by default { q: [searchText] }. You can customize the mapping * searchText => searchQuery by setting a custom `filterToQuery` function option diff --git a/packages/ra-ui-materialui/src/input/ReferenceInput.js b/packages/ra-ui-materialui/src/input/ReferenceInput.js index 67446d5dff6..44e5d9aca19 100644 --- a/packages/ra-ui-materialui/src/input/ReferenceInput.js +++ b/packages/ra-ui-materialui/src/input/ReferenceInput.js @@ -55,7 +55,7 @@ export const ReferenceInputView = ({ error, input, isRequired, - isLoading, + loading, label, meta, onChange, @@ -68,7 +68,7 @@ export const ReferenceInputView = ({ warning, ...rest }) => { - if (isLoading) { + if (loading) { return ( ', () => { {...{ ...defaultProps, input: { value: 1 }, - isLoading: true, + loading: true, }} > From 70b13583dd9e37a9737c8a38f380c45f5e6a3dff Mon Sep 17 00:00:00 2001 From: sedy-bot Date: Mon, 22 Jul 2019 07:27:08 +0000 Subject: [PATCH 48/48] Typo fix s/props/prop/ As requested by djhi at https://github.com/marmelab/react-admin/pull/3313#discussion_r305703896 --- UPGRADE.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/UPGRADE.md b/UPGRADE.md index 6675e85e35b..5cb2bd40b8c 100644 --- a/UPGRADE.md +++ b/UPGRADE.md @@ -312,7 +312,7 @@ export default (type, params) => { ## ReferenceInputController isLoading injected props renamed to loading -When using custom component with ReferenceInputController, you should rename the component `isLoading` props to `loading`. +When using custom component with ReferenceInputController, you should rename the component `isLoading` prop to `loading`. ```diff -