fix: show table summary is not reactive in pivot table

[islxyqwe]

This PR fixed a bug that caused by a dependency is missed so the table summary feature cannot reactive immediately.

[vercel]

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name	Status	Preview	Comments	Updated (UTC)
graphic-walker	✅ Ready (Inspect)	Visit Preview	💬 Add feedback	May 24, 2024 3:45am

[github-actions]

Risk Level 2 - /home/runner/work/graphic-walker/graphic-walker/packages/graphic-walker/src/components/pivotTable/utils.ts

The code changes seem to be well written and follow good practices. However, there are a few areas that could be improved for better readability and performance:

1. Use of Array.prototype.find in insertNode function: This could potentially slow down the performance if the tree.children array is large. Consider using a Map or Set for constant time lookup if the key values are unique.

2. Use of Array.prototype.includes in insertNode function: Similar to the above point, this could slow down the performance if the collapsedKeyList array is large. Consider using a Set for constant time lookup.

3. Use of Array.prototype.filter in buildMetricTableFromNestTree function: This could potentially slow down the performance if the data array is large. Consider using a different data structure or algorithm that can perform this operation more efficiently.

Here are some example code snippets for the suggested changes:

// Use a Set for constant time lookup
const collapsedKeySet = new Set(collapsedKeyList);
if (collapsedKeySet.has(tree.uniqueKey)) {
    tree.isCollapsed = true;
}

// Use a Map for constant time lookup
const childMap = new Map(tree.children.map(c => [c.key, c]));
let child = childMap.get(key);

---

Risk Level 2 - /home/runner/work/graphic-walker/graphic-walker/packages/graphic-walker/src/components/pivotTable/index.tsx

The code changes seem to be well-structured and follow good practices. However, there are a few areas that could be improved:

1. Error Handling: In the catch block of the generateNewTable function, the error is logged to the console but not handled further. It would be better to handle the error in a way that doesn't break the application and provides feedback to the user. For example:

.catch((err) => {
  appRef.current?.updateRenderStatus('error');
  setIsLoading(false);
  // Add user-friendly error handling
  alert('An error occurred while generating the table. Please try again.');
});

2. Code Duplication: The setIsLoading(false); line is repeated in both the then and catch blocks of the generateNewTable function. This could be moved to a finally block to reduce code duplication:

.finally(() => {
  setIsLoading(false);
});

3. Use of any Type: The metricTable state is initialized with any[][]. It's generally a good practice to avoid using any in TypeScript. If possible, replace any with a more specific type.

---

🔍📈🔧

---

Powered by Code Review GPT