-
Notifications
You must be signed in to change notification settings - Fork 32
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
refactor: Require memoizee and memoizeClear calls have a max size #2375
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
PR Overview
This PR refactors memoization calls by requiring that all memoizee and memoizeClear calls explicitly provide a maximum cache size to help prevent long-term caching issues. The changes include converting function expressions to proper function declarations in some cases and adding { max: } options across multiple components and modules.
- Refactored memoizeClear in packages/grid
- Added max cache size configurations to memoization calls in grid, iris-grid, dashboard, code-studio, and other packages
Reviewed Changes
File | Description |
---|---|
packages/grid/src/memoizeClear.ts | Converted the memoizeClear export to a named function and applied memoizee with max config |
packages/grid/src/MockTreeGridModel.ts | Added { max: 10000 } to each memoizeClear call for caching functions |
packages/iris-grid/src/sidebar/visibility-ordering-builder/VisibilityOrderingBuilder.tsx | Updated memoization calls to include specific max values |
packages/code-studio/src/settings/ColumnSpecificSectionContent.tsx | Updated memoization for ColumnTypeOptions with an added max size configuration |
packages/iris-grid/src/sidebar/RollupRows.tsx | Provided max cache size options for memoize calls ensuring consistency |
packages/chart/src/FigureChartModel.ts | Added max parameters to various memoize calls for chart formatting and value translation |
packages/iris-grid/src/IrisGrid.tsx | Added { max: } options to multiple memoize calls handling grid operations |
packages/code-studio/src/styleguide/DraggableLists.tsx | Updated memoization calls in event handlers and component producers with max size options |
packages/components/src/AutoCompleteInput.tsx | Applied a max cache size to the memoized filter function |
packages/dashboard-core-plugins/src/panels/DropdownFilterPanel.tsx | Updated memoized functions with a max option for caching values |
packages/dashboard-core-plugins/src/controls/input-filter/InputFilter.tsx | Added max configuration to memoizee call for generating item labels |
packages/dashboard-core-plugins/src/controls/dropdown-filter/DropdownFilter.tsx | Updated memoization to include an explicit max configuration |
packages/code-studio/src/settings/FormattingSectionContent.tsx | Added max configuration to memoized formatting components |
packages/components/src/ItemList.tsx | Updated multiple memoization calls for outer and inner component creation with max options |
packages/iris-grid/src/IrisGridPartitionSelector.tsx | Added max cache size options to memoizee calls for partition selector callbacks |
Copilot reviewed 16 out of 16 changed files in this pull request and generated no comments.
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #2375 +/- ##
=======================================
Coverage 46.80% 46.80%
=======================================
Files 710 710
Lines 39226 39231 +5
Branches 9975 9975
=======================================
+ Hits 18358 18361 +3
- Misses 20814 20816 +2
Partials 54 54
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
f96ad6c
to
d9845de
Compare
Related to #2374 (and might need that change before this can merge). This requires all
memoizee
andmemoizeClear
calls to provide a max cache size. Most of these were small caches most likely, but this should help prevent another issue that is hard to spot since it takes potentially hours to reproduce like DH-18798