perf: getLabelName & store legends graph efficiently #9967
Conversation
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
This PR is being reviewed by Cursor Bugbot
Details
Your team is on the Bugbot Free tier. On this plan, Bugbot will review limited PRs each billing cycle for each member of your team.
To receive Bugbot reviews on all of your PRs, visit the Cursor dashboard to activate Pro and start your 14-day free trial.
| } | ||
| visibilityStateAndLegendEntry.graphVisibilityStates[i + 1] = item.inverted | ||
| ? index === -1 | ||
| : index !== -1; |
There was a problem hiding this comment.
Visibility state restoration logic is completely inverted
High Severity
The visibility state restoration from localStorage produces the opposite of the saved state. When inverted is true (meaning dataIndex stores visible items), the code returns index === -1 (not found = visible) when it should return index !== -1 (found = visible). Similarly when inverted is false (meaning dataIndex stores hidden items), the code returns index !== -1 (found = visible) when it should return index === -1 (not found = visible). This causes all legend visibility states to be completely inverted when the page is refreshed.
🔬 Verification Test
Why verification test was not possible: This is a frontend React application that requires a browser environment and localStorage. The bug can be verified by tracing the logic manually:
-
SAVE with storeInverted=true: If user hides 6/8 items (leaves items A, B visible):
amountOfFalse = 6,storeInverted = 6 > 4 = true- Stores
inverted: truewithdataIndex = [{label: "A"}, {label: "B"}](visible items)
-
RESTORE for item A (found in dataIndex, index=0):
item.inverted ? index === -1 : index !== -1true ? (0 === -1) : (0 !== -1)=true ? false : true= false- But A was VISIBLE, so result should be true
-
RESTORE for item C (not in dataIndex, index=-1):
item.inverted ? index === -1 : index !== -1true ? (-1 === -1) : (-1 !== -1)=true ? true : false= true- But C was HIDDEN, so result should be false
Why this proves the bug: The ternary conditions are swapped. The correct logic would be item.inverted ? index !== -1 : index === -1.
| ); | ||
| } | ||
|
|
||
| const newGraphVisibilityStates = Array(apiResponse.length + 1).fill(true); |
There was a problem hiding this comment.
Legend entries replaced with incomplete subset from storage
Medium Severity
The legendEntry property is directly assigned item.dataIndex, which only contains a subset of legends (either visible or hidden items, depending on the inverted flag) stored for efficiency. This replaces the complete legend list (initialized from showAllDataSetFromApiResponse) with an incomplete subset, potentially causing missing legend entries in the UI. The legendEntry needs to be reconstructed to include all entries, not just the stored subset.
🔬 Verification Test
Why verification test was not possible: This requires a browser environment with localStorage and the full React application context. The bug can be verified by tracing the logic:
- Initial
legendEntryis set with ALL legends from API response (lines 38-44) - When restoring from localStorage,
item.dataIndexcontains only a subset (e.g., if 3/10 items are hidden andinverted=false, only those 3 hidden items are stored) - Line 66 replaces the complete list with just this subset:
visibilityStateAndLegendEntry.legendEntry = item.dataIndex - The result is
legendEntrynow only has 3 items instead of 10
Why this proves the bug: The storage optimization stores only a subset of legends, but the restore logic incorrectly uses this subset as the complete legend list.
| } | ||
| visibilityStateAndLegendEntry.graphVisibilityStates[i + 1] = item.inverted | ||
| ? index === -1 | ||
| : index !== -1; |
There was a problem hiding this comment.
Old localStorage format ignored during migration
Medium Severity
When reading localStorage data saved in the old format (which included show: boolean for each entry and no inverted flag), the new code ignores the show property entirely. Since old data doesn't have inverted, it's undefined (falsy), causing the expression to evaluate to index !== -1. Because old data stored ALL items in dataIndex, every item gets index !== -1 → true, making all series visible regardless of what was actually saved. Users upgrading will lose their saved legend visibility preferences.
🔬 Verification Test
Why verification test was not possible: This requires a browser environment with localStorage. The bug can be verified by tracing the logic:
-
Old format in localStorage:
{ "name": "widget1", "dataIndex": [{ "label": "A", "show": true }, { "label": "B", "show": false }] } -
New code reading old format:
item.invertedisundefined(doesn't exist in old format)- Expression:
undefined ? index === -1 : index !== -1 - Since undefined is falsy: result is
index !== -1 - Both A and B are in dataIndex, so both get
index !== -1=true - Result: Both A and B are visible, even though B had
show: false
Why this proves the bug: The old show: false value is completely ignored, causing all previously hidden series to become visible after upgrade.
1 participant
📄 Summary
First, reduce the amount of array copies and loops inside the getLabelName since is called heavily.
Secondly, instead of storing the whole legends in the localStorage, storage only what is needed, this greatly improves the UI perf when you click a legend and then refresh the page.
✅ Changes
🧪 How to Test
🔍 Related Issues
Related #9784
📸 Screenshots / Screen Recording (if applicable / mandatory for UI related changes)
📋 Checklist
About the eslint ignores, I was not able to make it pass without them, so let me know what you want to do for that case.
👀 Notes for Reviewers
Note
Improves performance in label generation and legend persistence, reducing memory usage and work on render/refresh.
lib/getLabelNamewithgetLabelNameFromMetricto minimize allocations/loops and streamline legend variable substitutionSavedLegendGraphwithinvertedand sparsedataIndex(labels only); stops storing full boolean arrays inGRAPH_VISIBILITY_STATESLegendEntryProps.showoptional to align with the new storage formatWritten by Cursor Bugbot for commit 4fd98bd. This will update automatically on new commits. Configure here.