Skip to content
New issue

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

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

Already on GitHub? Sign in to your account

Fix resolver resolution status key types mismatch causing unwanted resolver calls for identical state data #52120

Merged
merged 33 commits into from
Oct 16, 2023
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
33 commits
Select commit Hold shift + click to select a range
6bcec6e
Normalize
getdave Jun 29, 2023
9f8f093
Add optional method property to normalize arguments before usage
getdave Jun 29, 2023
355ee98
Check for ID-like keys rather than numeric things
getdave Jun 29, 2023
b0598a2
Extract util and update to use suggestion from Code Review
getdave Jun 30, 2023
99419ce
Test for util
getdave Jul 4, 2023
82352d7
Add tests for normalizeArgs
getdave Jul 4, 2023
49eea82
Fix bug with normalizeArgs called even if there are no args
getdave Jul 4, 2023
f569a98
Add direct test on `normalizeArgs` method of getEntityRecord
getdave Jul 4, 2023
cd31934
Allow defining normalize method on selector and call for both selecto…
getdave Jul 4, 2023
775f32b
Move normalization function to getEntityRecord selector
getdave Jul 4, 2023
240075b
Move normalization functino to getEntityRecord selector
getdave Jul 4, 2023
f9a3fb7
Add rough outline of documentation
getdave Jul 4, 2023
728b5f7
Add test to ensure normalization is applied to selector even without …
getdave Jul 4, 2023
3c43549
Improve documentation to better explain concept of normalizeArgs
getdave Jul 5, 2023
4c5aa43
Correct grammar
getdave Jul 5, 2023
959fe56
Apply types optimisation from code review
getdave Jul 5, 2023
7aa49ed
Extract conditionals to helper function
getdave Jul 5, 2023
3965284
Document getEntityRecord normalization function
getdave Jul 5, 2023
54d76c7
Add type defs to normalization function
getdave Jul 5, 2023
0d5856e
Fix nits in README
getdave Jul 5, 2023
7683ab3
Remove new line
getdave Jul 5, 2023
7b4cdfb
Add test for arguments length before invoking normalization function
getdave Jul 5, 2023
6702752
Remove unwanted comment
getdave Jul 5, 2023
be322a4
Addition to docs as per code review
getdave Jul 5, 2023
c178aa5
Fix bug with metadata selector args not being normalized
getdave Jul 6, 2023
f375fd7
Add tests for normalization to metadata selector tests
getdave Jul 6, 2023
d4899d2
Add test case for decimals
getdave Jul 7, 2023
9ad9744
Prefix with __unstable to denote non “settled” approach amongst contr…
getdave Jul 12, 2023
4598fa0
Remove check for args and conditionally access args index
getdave Jul 12, 2023
3a1dbb5
Simplify coercing to number
getdave Jul 12, 2023
2c5dc7a
Revert confusing DRY typescript
getdave Jul 12, 2023
b0b5b45
Fix renaming selector to `unstable`
getdave Jul 12, 2023
5d2c7b3
Copy to new args
getdave Jul 12, 2023
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
Prev Previous commit
Next Next commit
Allow defining normalize method on selector and call for both selecto…
…r and resolver
  • Loading branch information
getdave committed Oct 13, 2023
commit cd31934e8637cb0218a7d38627b4b5898e4fda53
21 changes: 19 additions & 2 deletions packages/data/src/redux-store/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -235,11 +235,24 @@ export default function createReduxStore( key, options ) {
selector.registry = registry;
}
const boundSelector = ( ...args ) => {
if (
selector.normalizeArgs &&
typeof selector.normalizeArgs === 'function' &&
args?.length
) {
args = selector.normalizeArgs( args );
}
const state = store.__unstableOriginalGetState();
return selector( state.root, ...args );
};

// Expose normalization method on the bound selector
// in order that it can be called when fullfilling
// the resolver.
boundSelector.normalizeArgs = selector.normalizeArgs;
getdave marked this conversation as resolved.
Show resolved Hide resolved

const resolver = resolvers[ selectorName ];

if ( ! resolver ) {
boundSelector.hasResolver = false;
return boundSelector;
Expand Down Expand Up @@ -604,8 +617,12 @@ function mapSelectorWithResolver(
}

const selectorResolver = ( ...args ) => {
if ( resolver.normalizeArgs && args?.length ) {
args = resolver.normalizeArgs( args );
if (
selector.normalizeArgs &&
typeof selector.normalizeArgs === 'function' &&
args?.length
) {
args = selector.normalizeArgs( args );
}
fulfillSelector( args );
return selector( ...args );
Expand Down
36 changes: 21 additions & 15 deletions packages/data/src/redux-store/test/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -288,46 +288,52 @@ describe( 'resolveSelect', () => {
} );

describe( 'normalizing args', () => {
it( 'should call the .normalizeArgs method on the resolver if it exists', async () => {
it( 'should call the normalizeArgs method of the selector for both the selector and the resolver', async () => {
const registry = createRegistry();
const resolver = () => {};
const selector = () => {};

resolver.normalizeArgs = jest.fn( ( ...args ) => args );
const normalizingFunction = jest.fn( ( ...args ) => args );

selector.normalizeArgs = normalizingFunction;

registry.registerStore( 'store', {
reducer: () => {},
selectors: {
getItems: () => 'items',
getItems: selector,
},
resolvers: {
getItems: resolver,
getItems: () => 'items',
},
} );
registry.select( 'store' ).getItems( 'foo', 'bar' );

expect( resolver.normalizeArgs ).toHaveBeenCalledWith( [
'foo',
'bar',
] );
expect( normalizingFunction ).toHaveBeenCalledWith( [ 'foo', 'bar' ] );

// Needs to be call twice:
// 1. When the selector is called.
// 2. When the resolver is fullfilled.
expect( normalizingFunction ).toHaveBeenCalledTimes( 2 );
} );

it( 'should not call normalizeArgs if there are no arguments passed to the resolver', async () => {
it( 'should not call the normalizeArgs method if there are no arguments passed to the selector (and thus the resolver)', async () => {
getdave marked this conversation as resolved.
Show resolved Hide resolved
const registry = createRegistry();
const resolver = () => {};
const selector = () => {};

resolver.normalizeArgs = jest.fn( ( ...args ) => args );
selector.normalizeArgs = jest.fn( ( ...args ) => args );

registry.registerStore( 'store', {
reducer: () => {},
selectors: {
getItems: () => 'items',
getItems: selector,
},
resolvers: {
getItems: resolver,
getItems: () => 'items',
},
} );

// Called with no args so the normalizeArgs method should not be called.
registry.select( 'store' ).getItems();

expect( resolver.normalizeArgs ).not.toHaveBeenCalled();
expect( selector.normalizeArgs ).not.toHaveBeenCalled();
} );
} );