You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@superset.apache.org by "sfirke (via GitHub)" <gi...@apache.org> on 2026/04/27 22:59:03 UTC

[PR] fix: preserve global filter, fix rerender bug [superset]

sfirke opened a new pull request, #39707:
URL: https://github.com/apache/superset/pull/39707

   ### SUMMARY
   Fix #39700
   
   ### TESTING INSTRUCTIONS
   I deployed this, tested, and found that the bug is fixed.
   
   ### ADDITIONAL INFORMATION
   - [x] Has associated issue: Fixes #39700


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org


Re: [PR] fix(table chart): fix rerender bug that continuously cleared search box [superset]

Posted by "codecov[bot] (via GitHub)" <gi...@apache.org>.
codecov[bot] commented on PR #39707:
URL: https://github.com/apache/superset/pull/39707#issuecomment-4331685601

   ## [Codecov](https://app.codecov.io/gh/apache/superset/pull/39707?dropdown=coverage&src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) Report
   :x: Patch coverage is `70.00000%` with `3 lines` in your changes missing coverage. Please review.
   :white_check_mark: Project coverage is 64.48%. Comparing base ([`5fe3a1c`](https://app.codecov.io/gh/apache/superset/commit/5fe3a1c2cd346b703704c0b78b26c06e5f29bb31?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)) to head ([`bcb9c01`](https://app.codecov.io/gh/apache/superset/commit/bcb9c01e0a04be33c352b22b3cef126327833a1b?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)).
   :warning: Report is 1 commits behind head on master.
   
   | [Files with missing lines](https://app.codecov.io/gh/apache/superset/pull/39707?dropdown=coverage&src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | Patch % | Lines |
   |---|---|---|
   | [...ins/plugin-chart-table/src/DataTable/DataTable.tsx](https://app.codecov.io/gh/apache/superset/pull/39707?src=pr&el=tree&filepath=superset-frontend%2Fplugins%2Fplugin-chart-table%2Fsrc%2FDataTable%2FDataTable.tsx&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-c3VwZXJzZXQtZnJvbnRlbmQvcGx1Z2lucy9wbHVnaW4tY2hhcnQtdGFibGUvc3JjL0RhdGFUYWJsZS9EYXRhVGFibGUudHN4) | 70.00% | [3 Missing :warning: ](https://app.codecov.io/gh/apache/superset/pull/39707?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
   
   <details><summary>Additional details and impacted files</summary>
   
   
   
   ```diff
   @@            Coverage Diff             @@
   ##           master   #39707      +/-   ##
   ==========================================
   + Coverage   64.46%   64.48%   +0.01%     
   ==========================================
     Files        2566     2566              
     Lines      133969   133978       +9     
     Branches    31104    31109       +5     
   ==========================================
   + Hits        86363    86390      +27     
   + Misses      46111    46093      -18     
     Partials     1495     1495              
   ```
   
   | [Flag](https://app.codecov.io/gh/apache/superset/pull/39707/flags?src=pr&el=flags&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | Coverage Δ | |
   |---|---|---|
   | [javascript](https://app.codecov.io/gh/apache/superset/pull/39707/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `66.58% <70.00%> (+0.02%)` | :arrow_up: |
   
   Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#carryforward-flags-in-the-pull-request-comment) to find out more.
   </details>
   
   [:umbrella: View full report in Codecov by Sentry](https://app.codecov.io/gh/apache/superset/pull/39707?dropdown=coverage&src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache).   
   :loudspeaker: Have feedback on the report? [Share it here](https://about.codecov.io/codecov-pr-comment-feedback/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache).
   <details><summary> :rocket: New features to boost your workflow: </summary>
   
   - :snowflake: [Test Analytics](https://docs.codecov.com/docs/test-analytics): Detect flaky tests, report on failures, and find test suite problems.
   - :package: [JS Bundle Analysis](https://docs.codecov.com/docs/javascript-bundle-analysis): Save yourself from yourself by tracking and limiting bundle sizes in JS merges.
   </details>


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org


Re: [PR] fix(table chart): fix rerender bug that continuously cleared search box [superset]

Posted by "sfirke (via GitHub)" <gi...@apache.org>.
sfirke commented on code in PR #39707:
URL: https://github.com/apache/superset/pull/39707#discussion_r3151009922


##########
superset-frontend/plugins/plugin-chart-table/src/DataTable/DataTable.tsx:
##########
@@ -147,7 +147,20 @@ export default typedMemo(function DataTable<D extends object>({
     hooks || [],
   ].flat();
 
-  const columnNames = Object.keys(data?.[0] || {});
+  const columnNames = columns.map((column, index) => {
+    const normalizedColumn = column as typeof column & {
+      accessor?: string;
+      columnKey?: string;
+      id?: string;
+    };
+
+    return (
+      normalizedColumn.columnKey ??
+      normalizedColumn.accessor ??
+      normalizedColumn.id ??
+      String(index)
+    );

Review Comment:
   Resolved in bcb9c01e0a



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org


Re: [PR] fix: preserve global filter, fix rerender bug [superset]

Posted by "bito-code-review[bot] (via GitHub)" <gi...@apache.org>.
bito-code-review[bot] commented on PR #39707:
URL: https://github.com/apache/superset/pull/39707#issuecomment-4331074138

   AI Code Review is in progress (usually takes 3 to 15 minutes unless it's a very large PR).


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org


Re: [PR] fix(table chart): fix rerender bug that continuously cleared search box [superset]

Posted by "bito-code-review[bot] (via GitHub)" <gi...@apache.org>.
bito-code-review[bot] commented on PR #39707:
URL: https://github.com/apache/superset/pull/39707#issuecomment-4331833629

   AI Code Review is in progress (usually takes 3 to 15 minutes unless it's a very large PR).
   
   <details>
   <summary>
   Bito Usage Guide
   </summary>
   
   **Commands**
         
   Type the following command in the pull request comment and save the comment.
         
   - `/review` - Manually triggers a full AI review.
   
   - `/pause` - Pauses automatic reviews on this pull request.
   - `/resume` - Resumes automatic reviews.
   - `/resolve` - Marks all Bito-posted review comments as resolved.
   - `/abort` - Cancels all in-progress reviews.
   
         
   Refer to the <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/available-commands" target="_blank" rel="noopener noreferrer">documentation</a> for additional commands.
         
   **Configuration**
   
   This repository uses `Superset` You can customize the agent settings <a href="https://alpha.bito.ai/home/ai-agents/code-review-agent" target="_blank" rel="noopener noreferrer">here</a> or contact your Bito workspace admin at evan@preset.io.
         
   **Documentation & Help**
   - <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/getting-started/install-run-using-bito-cloud/create-or-customize-an-agent-instance" target="_blank" rel="noopener noreferrer">Customize agent settings</a>
   - <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/implementing-custom-code-review-rules" target="_blank" rel="noopener noreferrer">Review rules</a>
   - <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent" target="_blank" rel="noopener noreferrer">General documentation</a>
   - <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/faqs" target="_blank" rel="noopener noreferrer">FAQ</a>
   
   </details>
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org


Re: [PR] fix(table chart): preserve global filter, fix rerender bug that cleared search box [superset]

Posted by "codeant-ai-for-open-source[bot] (via GitHub)" <gi...@apache.org>.
codeant-ai-for-open-source[bot] commented on code in PR #39707:
URL: https://github.com/apache/superset/pull/39707#discussion_r3150636585


##########
superset-frontend/plugins/plugin-chart-table/src/DataTable/DataTable.tsx:
##########
@@ -237,6 +237,7 @@ export default typedMemo(function DataTable<D extends object>({
       getTableSize: defaultGetTableSize,
       globalFilter: defaultGlobalFilter,
       sortTypes,
+      autoResetGlobalFilter: !isEqual(columnNames, previousColumnNames),

Review Comment:
   **Suggestion:** This reset condition is derived from `Object.keys(data?.[0])`, so when the dataset temporarily becomes empty (for example after upstream filtering), the key list becomes `[]` and is treated as a schema change. That will incorrectly reset the global search state and clear the search box even though table columns did not actually change. Compare against stable column definitions (or previous non-empty schema) instead of the first row keys. [logic error]
   
   <details>
   <summary><b>Severity Level:</b> Major ⚠️</summary>
   
   ```mdx
   - ❌ Table chart search cleared when results become temporarily empty.
   - ⚠️ Users lose client-side filters after zero-row intermediate queries.
   ```
   </details>
   <details>
   <summary><b>Steps of Reproduction ✅ </b></summary>
   
   ```mdx
   1. In Superset Explore, create or open a Table visualization, which renders the
   `TableChart` component
   (`superset-frontend/plugins/plugin-chart-table/src/TableChart.tsx:312-350`) backed by
   `transformProps`
   (`superset-frontend/plugins/plugin-chart-table/src/transformProps.ts:491-508`) for its
   `data` and `columns` props.
   
   2. Run a query that returns at least one row so `transformProps` produces non-empty `data`
   and stable `columns` (`transformProps.ts:706-717` and `transformProps.ts:769-776`);
   `TableChart` passes these into `<DataTable>` as `data={data}` and `columns={columns}`
   (`TableChart.tsx:1499-1503`), and inside `DataTable` the first render computes
   `columnNames = Object.keys(data?.[0] || {})` and `previousColumnNames =
   usePrevious(columnNames)` (`DataTable.tsx:150-151`).
   
   3. Type some text into the search box rendered by `GlobalFilter` in `DataTable`
   (`DataTable.tsx:176-190`), which updates the `globalFilter` state managed by `useTable`
   (exposed as `filterValue` in the `state` destructuring at `DataTable.tsx:225-231`); with
   `autoResetGlobalFilter` enabled, this search text is bound to `filterValue` and shown in
   the input.
   
   4. Change an upstream filter (for example time range or a dashboard/explore filter) so the
   query now returns zero rows but the schema (colnames) is unchanged; `transformProps` then
   sets `data: passedData` where `passedData` becomes `[]` (`transformProps.ts:730-771`)
   while `columns` remain the previous non-empty list from `passedColumns`
   (`transformProps.ts:730-776`), causing the next `DataTable` render to compute `columnNames
   = []` and `previousColumnNames` as the prior non-empty key list (`DataTable.tsx:150-151`),
   making `!isEqual(columnNames, previousColumnNames)` true at `autoResetGlobalFilter`
   (`DataTable.tsx:240`) so `react-table` auto-resets `globalFilter` and clears the search
   input even though the actual column definitions (`columns` prop) did not change.
   ```
   </details>
   
   [Fix in Cursor](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt=This%20is%20a%20comment%20left%20during%20a%20code%20review.%0A%0A%2A%2APath%3A%2A%2A%20superset-frontend%2Fplugins%2Fplugin-chart-table%2Fsrc%2FDataTable%2FDataTable.tsx%0A%2A%2ALine%3A%2A%2A%20240%3A240%0A%2A%2AComment%3A%2A%2A%0A%09%2ALogic%20Error%3A%20This%20reset%20condition%20is%20derived%20from%20%60Object.keys%28data%3F.%5B0%5D%29%60%2C%20so%20when%20the%20dataset%20temporarily%20becomes%20empty%20%28for%20example%20after%20upstream%20filtering%29%2C%20the%20key%20list%20becomes%20%60%5B%5D%60%20and%20is%20treated%20as%20a%20schema%20change.%20That%20will%20incorrectly%20reset%20the%20global%20search%20state%20and%20clear%20the%20search%20box%20even%20though%20table%20columns%20did%20not%20actually%20change.%20Compare%20against%20stable%20column%20definitions%20%28or%20previous%20non-empty%20schema%29%20instead%20of%20the%20first%20row%20keys.%0A%0AValidate%20the%20correctness%20of%20the%20flagged%20issu
 e.%20If%20correct%2C%20How%20can%20I%20resolve%20this%3F%20If%20you%20propose%20a%20fix%2C%20implement%20it%20and%20please%20make%20it%20concise.%0AOnce%20fix%20is%20implemented%2C%20also%20check%20other%20comments%20on%20the%20same%20PR%2C%20and%20ask%20user%20if%20the%20user%20wants%20to%20fix%20the%20rest%20of%20the%20comments%20as%20well.%20if%20said%20yes%2C%20then%20fetch%20all%20the%20comments%20validate%20the%20correctness%20and%20implement%20a%20minimal%20fix%0A) | [Fix in VSCode Claude](https://app.codeant.ai/fix-in-ide?tool=vscode-claude&prompt=This%20is%20a%20comment%20left%20during%20a%20code%20review.%0A%0A%2A%2APath%3A%2A%2A%20superset-frontend%2Fplugins%2Fplugin-chart-table%2Fsrc%2FDataTable%2FDataTable.tsx%0A%2A%2ALine%3A%2A%2A%20240%3A240%0A%2A%2AComment%3A%2A%2A%0A%09%2ALogic%20Error%3A%20This%20reset%20condition%20is%20derived%20from%20%60Object.keys%28data%3F.%5B0%5D%29%60%2C%20so%20when%20the%20dataset%20temporarily%20becomes%20empty%20%28for%20example%20after%
 20upstream%20filtering%29%2C%20the%20key%20list%20becomes%20%60%5B%5D%60%20and%20is%20treated%20as%20a%20schema%20change.%20That%20will%20incorrectly%20reset%20the%20global%20search%20state%20and%20clear%20the%20search%20box%20even%20though%20table%20columns%20did%20not%20actually%20change.%20Compare%20against%20stable%20column%20definitions%20%28or%20previous%20non-empty%20schema%29%20instead%20of%20the%20first%20row%20keys.%0A%0AValidate%20the%20correctness%20of%20the%20flagged%20issue.%20If%20correct%2C%20How%20can%20I%20resolve%20this%3F%20If%20you%20propose%20a%20fix%2C%20implement%20it%20and%20please%20make%20it%20concise.%0AOnce%20fix%20is%20implemented%2C%20also%20check%20other%20comments%20on%20the%20same%20PR%2C%20and%20ask%20user%20if%20the%20user%20wants%20to%20fix%20the%20rest%20of%20the%20comments%20as%20well.%20if%20said%20yes%2C%20then%20fetch%20all%20the%20comments%20validate%20the%20correctness%20and%20implement%20a%20minimal%20fix%0A)
   
   *(Use Cmd/Ctrl + Click for best experience)*
   <details>
   <summary><b>Prompt for AI Agent 🤖 </b></summary>
   
   ```mdx
   This is a comment left during a code review.
   
   **Path:** superset-frontend/plugins/plugin-chart-table/src/DataTable/DataTable.tsx
   **Line:** 240:240
   **Comment:**
   	*Logic Error: This reset condition is derived from `Object.keys(data?.[0])`, so when the dataset temporarily becomes empty (for example after upstream filtering), the key list becomes `[]` and is treated as a schema change. That will incorrectly reset the global search state and clear the search box even though table columns did not actually change. Compare against stable column definitions (or previous non-empty schema) instead of the first row keys.
   
   Validate the correctness of the flagged issue. If correct, How can I resolve this? If you propose a fix, implement it and please make it concise.
   Once fix is implemented, also check other comments on the same PR, and ask user if the user wants to fix the rest of the comments as well. if said yes, then fetch all the comments validate the correctness and implement a minimal fix
   ```
   </details>
   <a href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F39707&comment_hash=43bf648d51245a36ed4b6af18bea477b46d48ad98f6c0f560d597f26da01d003&reaction=like'>👍</a> | <a href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F39707&comment_hash=43bf648d51245a36ed4b6af18bea477b46d48ad98f6c0f560d597f26da01d003&reaction=dislike'>👎</a>



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org


Re: [PR] fix(table chart): preserve global filter, fix rerender bug that cleared search box [superset]

Posted by "sfirke (via GitHub)" <gi...@apache.org>.
sfirke commented on code in PR #39707:
URL: https://github.com/apache/superset/pull/39707#discussion_r3150773546


##########
superset-frontend/plugins/plugin-chart-table/src/DataTable/DataTable.tsx:
##########
@@ -237,6 +237,7 @@ export default typedMemo(function DataTable<D extends object>({
       getTableSize: defaultGetTableSize,
       globalFilter: defaultGlobalFilter,
       sortTypes,
+      autoResetGlobalFilter: !isEqual(columnNames, previousColumnNames),

Review Comment:
   Reworked code and added test to address this.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org


Re: [PR] fix(table chart): preserve global filter, fix rerender bug that cleared search box [superset]

Posted by "bito-code-review[bot] (via GitHub)" <gi...@apache.org>.
bito-code-review[bot] commented on PR #39707:
URL: https://github.com/apache/superset/pull/39707#issuecomment-4331111354

   <!-- Bito Reply -->
   The flagged issue is correct: the autoResetGlobalFilter condition derives column names from data?.[0], which becomes [] when data is empty, incorrectly resetting the search even if table columns haven't changed. To resolve, compute columnNames from the stable columns prop instead. Here's the concise fix.
   
   **superset-frontend/plugins/plugin-chart-table/src/DataTable/DataTable.tsx**
   ```
   const columnNames = columns.map(col => col.accessor);
     const previousColumnNames = usePrevious(columnNames);
   ```


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org


Re: [PR] fix(table chart): fix rerender bug that continuously cleared search box [superset]

Posted by "bito-code-review[bot] (via GitHub)" <gi...@apache.org>.
bito-code-review[bot] commented on PR #39707:
URL: https://github.com/apache/superset/pull/39707#issuecomment-4331483991

   AI Code Review is in progress (usually takes 3 to 15 minutes unless it's a very large PR).
   
   <details>
   <summary>
   Bito Usage Guide
   </summary>
   
   **Commands**
         
   Type the following command in the pull request comment and save the comment.
         
   - `/review` - Manually triggers a full AI review.
   
   - `/pause` - Pauses automatic reviews on this pull request.
   - `/resume` - Resumes automatic reviews.
   - `/resolve` - Marks all Bito-posted review comments as resolved.
   - `/abort` - Cancels all in-progress reviews.
   
         
   Refer to the <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/available-commands" target="_blank" rel="noopener noreferrer">documentation</a> for additional commands.
         
   **Configuration**
   
   This repository uses `Superset` You can customize the agent settings <a href="https://alpha.bito.ai/home/ai-agents/code-review-agent" target="_blank" rel="noopener noreferrer">here</a> or contact your Bito workspace admin at evan@preset.io.
         
   **Documentation & Help**
   - <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/getting-started/install-run-using-bito-cloud/create-or-customize-an-agent-instance" target="_blank" rel="noopener noreferrer">Customize agent settings</a>
   - <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/implementing-custom-code-review-rules" target="_blank" rel="noopener noreferrer">Review rules</a>
   - <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent" target="_blank" rel="noopener noreferrer">General documentation</a>
   - <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/faqs" target="_blank" rel="noopener noreferrer">FAQ</a>
   
   </details>
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org


Re: [PR] fix(table chart): fix rerender bug that continuously cleared search box [superset]

Posted by "bito-code-review[bot] (via GitHub)" <gi...@apache.org>.
bito-code-review[bot] commented on code in PR #39707:
URL: https://github.com/apache/superset/pull/39707#discussion_r3150949122


##########
superset-frontend/plugins/plugin-chart-table/src/DataTable/DataTable.tsx:
##########
@@ -147,7 +147,20 @@ export default typedMemo(function DataTable<D extends object>({
     hooks || [],
   ].flat();
 
-  const columnNames = Object.keys(data?.[0] || {});
+  const columnNames = columns.map((column, index) => {
+    const normalizedColumn = column as typeof column & {
+      accessor?: string;
+      columnKey?: string;
+      id?: string;
+    };
+
+    return (
+      normalizedColumn.columnKey ??
+      normalizedColumn.accessor ??
+      normalizedColumn.id ??
+      String(index)
+    );

Review Comment:
   <div>
   
   
   <div id="suggestion">
   <div id="issue"><b>Type safety bug in column name extraction</b></div>
   <div id="fix">
   
   The new columnNames derivation uses `normalizedColumn.accessor ??` which can return a function when accessor is a function (valid in react-table), but columnNames must be an array of strings for isEqual comparisons and potential downstream usage. This causes incorrect reset behavior for filters/sort when columns change.
   </div>
   
   
   <details>
   <summary>
   <b>Code suggestion</b>
   </summary>
   <blockquote>Check the AI-generated fix before applying</blockquote>
   <div id="code">
   
   
   ````suggestion
       return (
         normalizedColumn.columnKey ??
         (typeof normalizedColumn.accessor === 'string' ? normalizedColumn.accessor : undefined) ??
         normalizedColumn.id ??
         String(index)
       );
   ````
   
   </div>
   </details>
   
   
   
   </div>
   
   
   
   
   <small><i>Code Review Run #4d1a7b</i></small>
   </div>
   
   ---
   Should Bito avoid suggestions like this for future reviews? (<a href=https://alpha.bito.ai/home/ai-agents/review-rules>Manage Rules</a>)
   - [ ] Yes, avoid them



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org


Re: [PR] fix(table chart): fix rerender bug that continuously cleared search box [superset]

Posted by "michael-s-molina (via GitHub)" <gi...@apache.org>.
michael-s-molina merged PR #39707:
URL: https://github.com/apache/superset/pull/39707


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org