CardView - Header filter search is only triggered once when remoteOperations is true (T1305672)#33899
CardView - Header filter search is only triggered once when remoteOperations is true (T1305672)#33899markallenramirez wants to merge 2 commits into
Conversation
There was a problem hiding this comment.
Pull request overview
This PR updates the CardView/grid “new” DataController deferred caching behavior so cache hits are less likely to occur when call arguments differ only in deeper nested properties—addressing cases where header filter search with remoteOperations: true was only triggering once.
Changes:
- Adjusted
deferredCacheargument equality check to useequalByValue(..., { maxDepth: 5 })instead of the default depth. - Ensures deeper load-option changes are detected and do not incorrectly reuse cached results.
Alyar666
left a comment
Alyar666
left a comment
There was a problem hiding this comment.
Please add an end-to-end test that covers the actual user scenario.
| const hasPreviousCall = lastArgs !== null && cachedResult !== null; | ||
| const isArgsSame = hasPreviousCall | ||
| ? equalByValue(lastArgs, args) | ||
| ? equalByValue(lastArgs, args, { maxDepth: 5 }) |
There was a problem hiding this comment.
We already have the FILTER_OBJ_COMPARE_DEPTH constant defined in data_controller.ts. I suggest moving it to a more appropriate location (for example, creating a const.ts file inside the data_controller directory) and reusing it wherever needed instead of duplicating the value.
There was a problem hiding this comment.
Agree, also I'd suggest to refactor the following code also:
if (!equalByValue(prevValue, newValue, { maxDepth: 5 })) {
in the columns_controller.ts
| expect(originFn).toHaveBeenCalledTimes(2); | ||
| }); | ||
|
|
||
| it('should call origin fn if args differ on deep value (maxDepth=5)', async () => { |
There was a problem hiding this comment.
The test name should call origin fn if args differ on deep value (maxDepth=5) hardcodes the constant value into the description. If the comparison depth is changed in the future, we'll also need to rename the test. It would be better to keep the test name independent of the specific constant value and focus on the behavior being verified.
No description provided.