Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Propose useEntityRecords (experimental) #38782

Merged
merged 5 commits into from
Feb 15, 2022
Merged

Propose useEntityRecords (experimental) #38782

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
5654efa
Propose useEntityRecords
adamziel Feb 14, 2022
6b0e67c
Expose __experimentalUseEntityRecords as a public API
adamziel Feb 14, 2022
2246075
Pluralize the docstring
adamziel Feb 14, 2022
2a93ed2
Move the status computation inside the enriched selectors
adamziel Feb 14, 2022
179e77f
Remove the confusing unit test – it's covered by the other one
adamziel Feb 15, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
32 changes: 6 additions & 26 deletions packages/core-data/src/hooks/test/use-entity-record.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,11 +33,11 @@ describe( 'useEntityRecord', () => {

const TEST_RECORD = { id: 1, hello: 'world' };

it( 'retrieves the relevant entity record', async () => {
it( 'resolves the entity record when missing from the state', async () => {
// Provide response
triggerFetch.mockImplementation( () => TEST_RECORD );

let data;
await registry
.dispatch( coreDataStore )
.receiveEntityRecords( 'root', 'widget', [ TEST_RECORD ] );
const TestComponent = () => {
data = useEntityRecord( 'root', 'widget', 1 );
return <div />;
Expand All @@ -47,34 +47,14 @@ describe( 'useEntityRecord', () => {
<TestComponent />
</RegistryProvider>
);

expect( data ).toEqual( {
record: TEST_RECORD,
records: undefined,
hasResolved: false,
isResolving: false,
status: 'IDLE',
} );

// Required to make sure no updates happen outside of act()
await act( async () => {
jest.advanceTimersByTime( 1 );
} );
} );

it( 'resolves the entity if missing from state', async () => {
// Provide response
triggerFetch.mockImplementation( () => TEST_RECORD );

let data;
const TestComponent = () => {
data = useEntityRecord( 'root', 'widget', 1 );
return <div />;
};
render(
<RegistryProvider value={ registry }>
<TestComponent />
</RegistryProvider>
);

await act( async () => {
jest.advanceTimersByTime( 1 );
} );
Expand Down
78 changes: 78 additions & 0 deletions packages/core-data/src/hooks/test/use-entity-records.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,78 @@
/**
* WordPress dependencies
*/
import triggerFetch from '@wordpress/api-fetch';
import { createRegistry, RegistryProvider } from '@wordpress/data';

jest.mock( '@wordpress/api-fetch' );

/**
* External dependencies
*/
import { act, render } from '@testing-library/react';

/**
* Internal dependencies
*/
import { store as coreDataStore } from '../../index';
import useEntityRecords from '../use-entity-records';

describe( 'useEntityRecords', () => {
let registry;
beforeEach( () => {
jest.useFakeTimers();

registry = createRegistry();
registry.register( coreDataStore );
} );

afterEach( () => {
jest.runOnlyPendingTimers();
jest.useRealTimers();
} );

const TEST_RECORDS = [
{ id: 1, hello: 'world1' },
{ id: 2, hello: 'world2' },
{ id: 3, hello: 'world3' },
];

it( 'resolves the entity records when missing from the state', async () => {
// Provide response
triggerFetch.mockImplementation( () => TEST_RECORDS );

let data;
const TestComponent = () => {
data = useEntityRecords( 'root', 'widget', { status: 'draft' } );
return <div />;
};
render(
<RegistryProvider value={ registry }>
<TestComponent />
</RegistryProvider>
);

expect( data ).toEqual( {
records: null,
hasResolved: false,
isResolving: false,
status: 'IDLE',
} );

await act( async () => {
jest.advanceTimersByTime( 1 );
} );

// Fetch request should have been issued
expect( triggerFetch ).toHaveBeenCalledWith( {
path: '/wp/v2/widgets?context=edit&status=draft',
} );

expect( data ).toEqual( {
records: TEST_RECORDS,
hasResolved: true,
isResolving: false,
status: 'SUCCESS',
} );
} );
} );
6 changes: 3 additions & 3 deletions packages/core-data/src/hooks/test/use-query-select.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -123,8 +123,8 @@ describe( 'useQuerySelect', () => {
expect( querySelectData ).toEqual( {
data: 'bar',
isResolving: false,
hasStarted: false,
hasResolved: false,
status: 'IDLE',
} );
} );

Expand Down Expand Up @@ -171,8 +171,8 @@ describe( 'useQuerySelect', () => {
expect( querySelectData ).toEqual( {
data: 10,
isResolving: false,
hasStarted: false,
hasResolved: false,
status: 'IDLE',
} );

await act( async () => {
Expand All @@ -188,8 +188,8 @@ describe( 'useQuerySelect', () => {
expect( querySelectData ).toEqual( {
data: 15,
isResolving: false,
hasStarted: true,
hasResolved: true,
status: 'SUCCESS',
} );
} );
} );
23 changes: 4 additions & 19 deletions packages/core-data/src/hooks/use-entity-record.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,7 @@ interface EntityRecordResolution< RecordType > {
* ```
*
* In the above example, when `PageTitleDisplay` is rendered into an
* application, the price and the resolution details will be retrieved from
* application, the page and the resolution details will be retrieved from
* the store state using `getEntityRecord()`, or resolved if missing.
*
* @return {EntityRecordResolution<RecordType>} Entity record data.
Expand All @@ -60,28 +60,13 @@ export default function __experimentalUseEntityRecord< RecordType >(
name: string,
recordId: string | number
): EntityRecordResolution< RecordType > {
const { data, isResolving, hasResolved } = useQuerySelect(
const { data: record, ...rest } = useQuerySelect(
( query ) => query( coreStore ).getEntityRecord( kind, name, recordId ),
[ kind, name, recordId ]
);

let status;
if ( isResolving ) {
status = Status.Resolving;
} else if ( hasResolved ) {
if ( data ) {
status = Status.Success;
} else {
status = Status.Error;
}
} else {
status = Status.Idle;
}

return {
status,
record: data,
isResolving,
hasResolved,
record,
...rest,
};
}
79 changes: 79 additions & 0 deletions packages/core-data/src/hooks/use-entity-records.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,79 @@
/**
* Internal dependencies
*/
import useQuerySelect from './use-query-select';
import { store as coreStore } from '../';
import { Status } from './constants';

interface EntityRecordsResolution< RecordType > {
/** The requested entity record */
records: RecordType[] | null;

/**
* Is the record still being resolved?
*/
isResolving: boolean;

/**
* Is the record resolved by now?
*/
hasResolved: boolean;

/** Resolution status */
status: Status;
}
adamziel marked this conversation as resolved.
Show resolved Hide resolved

/**
* Resolves the specified entity records.
*
* @param kind Kind of the requested entities.
* @param name Name of the requested entities.
adamziel marked this conversation as resolved.
Show resolved Hide resolved
* @param queryArgs HTTP query for the requested entities.
adamziel marked this conversation as resolved.
Show resolved Hide resolved
*
* @example
* ```js
* import { useEntityRecord } from '@wordpress/core-data';
*
* function PageTitlesList() {
* const { records, isResolving } = useEntityRecords( 'postType', 'page' );
*
* if ( isResolving ) {
* return 'Loading...';
* }
*
* return (
* <ul>
* {records.map(( page ) => (
* <li>{ page.title }</li>
* ))}
* </ul>
* );
* }
*
* // Rendered in the application:
* // <PageTitlesList />
* ```
*
* In the above example, when `PageTitlesList` is rendered into an
* application, the list of records and the resolution details will be retrieved from
* the store state using `getEntityRecords()`, or resolved if missing.
*
* @return {EntityRecordsResolution<RecordType>} Entity records data.
adamziel marked this conversation as resolved.
Show resolved Hide resolved
* @template RecordType
*/
export default function __experimentalUseEntityRecords< RecordType >(
kind: string,
name: string,
queryArgs: unknown = {}
Copy link
Member

Choose a reason for hiding this comment

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

how hard would it be to surface a type of queryArgs here from wherever it's consumed? if not, can we add a link in the JSDoc comment to point people to where they can find out? I know it's passed-through to getEntityRecords but unknown isn't that helpful.

Given that these are just passed through we might be able to reuse the types from getEntityRecords. I'm curious if the rename from data to records is fully worth it considering that it incurs a needless object clone, but I get how it's nice to match with useEntityRecord

interface EntityRecordHook<RecordType> {
    (kind: string; name: string; queryArgs: HttpQuery) => EntityRecordResolution<RecordType>;
}


// use-entity-record.ts
export default useEntityRecord: EntityRecordHook = (kind, name, queryArgs) => …;

// use-entity-records.ts
export default useEntityRecords: EntityRecordHook = …
EntityRecordHook.mov

playground link

Copy link
Contributor Author

Choose a reason for hiding this comment

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

how hard would it be to surface a type of queryArgs here from wherever it's consumed? if not, can we add a link in the JSDoc comment to point people to where they can find out? I know it's passed-through to getEntityRecords but unknown isn't that helpful. Given that these are just passed through we might be able to reuse the types from getEntityRecords. I

Good idea! The EntityQuery type from #40024 would make a good fit here once that PR lands.

I'm curious if the rename from data to records is fully worth it considering that it incurs a needless object clone, but I get how it's nice to match with useEntityRecord

It doesn't match, though 😅 it is record and records. The reason I went for that versus data is for the sake of the consumer of this hook. Referring to a record inside of a component is clearer than referring to an opaque data. I am not a fan of that destructuring, though.

): EntityRecordsResolution< RecordType > {
const { data: records, ...rest } = useQuerySelect(
( query ) =>
query( coreStore ).getEntityRecords( kind, name, queryArgs ),
[ kind, name, queryArgs ]
);

return {
records,
...rest,
};
}
33 changes: 23 additions & 10 deletions packages/core-data/src/hooks/use-query-select.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import { useSelect } from '@wordpress/data';
* Internal dependencies
*/
import memoize from './memoize';
import { Status } from './constants';

export const META_SELECTORS = [
'getIsResolving',
Expand Down Expand Up @@ -97,19 +98,31 @@ const enrichSelectors = memoize( ( selectors ) => {
}
Object.defineProperty( resolvers, selectorName, {
get: () => ( ...args ) => {
const {
getIsResolving,
hasStartedResolution,
hasFinishedResolution,
} = selectors;
const { getIsResolving, hasFinishedResolution } = selectors;
const isResolving = !! getIsResolving( selectorName, args );
const hasResolved =
! isResolving &&
hasFinishedResolution( selectorName, args );
const data = selectors[ selectorName ]( ...args );

let status;
if ( isResolving ) {
status = Status.Resolving;
} else if ( hasResolved ) {
if ( data ) {
status = Status.Success;
adamziel marked this conversation as resolved.
Show resolved Hide resolved
} else {
status = Status.Error;
}
} else {
status = Status.Idle;
}

return {
data: selectors[ selectorName ]( ...args ),
data,
status,
isResolving,
hasStarted: hasStartedResolution( selectorName, args ),
adamziel marked this conversation as resolved.
Show resolved Hide resolved
hasResolved:
! isResolving &&
hasFinishedResolution( selectorName, args ),
hasResolved,
};
},
} );
Expand Down
1 change: 1 addition & 0 deletions packages/core-data/src/index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -75,5 +75,6 @@ register( store );

export { default as EntityProvider } from './entity-provider';
export { default as __experimentalUseEntityRecord } from './hooks/use-entity-record';
export { default as __experimentalUseEntityRecords } from './hooks/use-entity-records';
export * from './entity-provider';
export * from './fetch';