Skip to content

Commit

Permalink
deoptimize fields filtering in the data module
Browse files Browse the repository at this point in the history
  • Loading branch information
youknowriad committed Oct 13, 2020
1 parent 5193869 commit aa701ad
Show file tree
Hide file tree
Showing 6 changed files with 32 additions and 35 deletions.
13 changes: 9 additions & 4 deletions packages/core-data/src/queried-data/get-query-parts.js
Original file line number Diff line number Diff line change
Expand Up @@ -65,11 +65,16 @@ export function getQueryParts( query ) {
);
break;

case '_fields':
parts.fields = getNormalizedCommaSeparable( value );
break;

default:
// While in theory, we could exclude "_fields" from the stableKey
// because two request with different fields have the same results
// We're not able to ensure that because the server can decide to omit
// fields from the response even if we explicitely asked for it.
// Example: Asking for titles in posts without title support.
if ( key === '_fields' ) {
parts.fields = getNormalizedCommaSeparable( value );
}

// While it could be any deterministic string, for simplicity's
// sake mimic querystring encoding for stable key.
//
Expand Down
10 changes: 1 addition & 9 deletions packages/core-data/src/queried-data/selectors.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
*/
import createSelector from 'rememo';
import EquivalentKeyMap from 'equivalent-key-map';
import { get, has, set } from 'lodash';
import { get, set } from 'lodash';

/**
* Internal dependencies
Expand Down Expand Up @@ -31,7 +31,6 @@ function getQueriedItemsUncached( state, query ) {
const { stableKey, page, perPage, include, fields } = getQueryParts(
query
);

let itemIds;
if ( Array.isArray( include ) && ! stableKey ) {
// If the parsed query yields a set of IDs, but otherwise no filtering,
Expand Down Expand Up @@ -73,14 +72,7 @@ function getQueriedItemsUncached( state, query ) {
filteredItem = {};

for ( let f = 0; f < fields.length; f++ ) {
// Abort the entire request if a field is missing from the item.
// This accounts for the fact that queried items are stored by
// stable key without an associated fields query. Other requests
// may have included fewer fields properties.
const field = fields[ f ].split( '.' );
if ( ! has( item, field ) ) {
return null;
}
const value = get( item, field );
set( filteredItem, field, value );
}
Expand Down
12 changes: 0 additions & 12 deletions packages/core-data/src/queried-data/test/get-query-parts.js
Original file line number Diff line number Diff line change
Expand Up @@ -28,18 +28,6 @@ describe( 'getQueryParts', () => {
} );
} );

it( 'parses out `_fields` property filtering', () => {
const parts = getQueryParts( { _fields: 'content', a: 1 } );

expect( parts ).toEqual( {
page: 1,
perPage: 10,
stableKey: 'a=1',
fields: [ 'content' ],
include: null,
} );
} );

it( 'encodes stable string key', () => {
const first = getQueryParts( { '?': '&', b: 2 } );
const second = getQueryParts( { b: 2, '?': '&' } );
Expand Down
4 changes: 2 additions & 2 deletions packages/core-data/src/queried-data/test/selectors.js
Original file line number Diff line number Diff line change
Expand Up @@ -94,7 +94,7 @@ describe( 'getQueriedItems', () => {
2: true,
},
queries: {
'': [ 1, 2 ],
'_fields%5B0%5D=content': [ 1, 2 ],
},
};

Expand Down Expand Up @@ -133,7 +133,7 @@ describe( 'getQueriedItems', () => {
2: true,
},
queries: {
'': [ 1, 2 ],
'_fields%5B0%5D=content&_fields%5B0%5D=meta.template': [ 1, 2 ],
},
};

Expand Down
18 changes: 15 additions & 3 deletions packages/core-data/src/selectors.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
* External dependencies
*/
import createSelector from 'rememo';
import { first, map, find, get, filter, compact, defaultTo } from 'lodash';
import { set, map, find, get, filter, compact, defaultTo } from 'lodash';

/**
* WordPress dependencies
Expand All @@ -16,6 +16,7 @@ import deprecated from '@wordpress/deprecated';
import { REDUCER_KEY } from './name';
import { getQueriedItems } from './queried-data';
import { DEFAULT_ENTITY_KEY } from './entities';
import { getNormalizedCommaSeparable } from './utils';

/**
* Returns true if a request is in progress for embed preview data, or false
Expand Down Expand Up @@ -135,8 +136,19 @@ export function getEntityRecord( state, kind, name, key, query ) {
return queriedState.items[ key ];
}

query = { ...query, include: [ key ] };
return first( getQueriedItems( queriedState, query ) );
const item = queriedState.items[ key ];
if ( item && query._fields ) {
const filteredItem = {};
const fields = getNormalizedCommaSeparable( query._fields );
for ( let f = 0; f < fields.length; f++ ) {
const field = fields[ f ].split( '.' );
const value = get( item, field );
set( filteredItem, field, value );
}
return filteredItem;
}

return item;
}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -64,11 +64,11 @@ describe( 'Test Custom Post Types', () => {
// Wait for the list of suggestions to fetch
// There should be a better way to do that.
// eslint-disable-next-line no-restricted-syntax
await page.waitFor( 1000 );
const selectedValue = await page.$eval(
PARENT_PAGE_INPUT,
( el ) => el.value
await page.waitFor(
( [ value, inputSelector ] ) =>
document.querySelector( inputSelector ).value === value,
{},
[ valueToSelect, PARENT_PAGE_INPUT ]
);
expect( selectedValue ).toEqual( valueToSelect );
} );
} );

0 comments on commit aa701ad

Please sign in to comment.