codeant-ai-for-open-source[bot] commented on code in PR #43184:
URL: https://github.com/apache/superset/pull/43184#discussion_r3786862260
##########
superset-frontend/plugins/plugin-chart-table/src/DataTable/DataTable.tsx:
##########
@@ -289,6 +324,48 @@ export default typedMemo(function DataTable<D extends
object>({
onFilteredDataChange(rowsRef.current, searchText);
}, [filterValue, onFilteredDataChange, rowSignature]);
+ // Emit filtered rows to parent in client-side mode (debounced via RAF).
+ // These hooks must stay above the "no columns" early return below, otherwise
+ // the hook count changes when the column count crosses zero (see #42978).
+ const isMountedRef = useRef(true);
+ useEffect(() => {
+ isMountedRef.current = true;
+ return () => {
+ isMountedRef.current = false;
+ };
+ }, []);
+
+ const rafRef = useRef<number | null>(null);
+ const lastSigRef = useRef<string>('');
+
+ useEffect(() => {
+ if (serverPagination || typeof onFilteredRowsChange !== 'function') {
+ return;
+ }
+
+ const sig = signatureOfRows(rows);
+
+ if (sig !== lastSigRef.current) {
+ lastSigRef.current = sig;
Review Comment:
**Suggestion:** `lastSigRef.current` is updated before the animation frame
invokes the callback, and it is not reset when `onFilteredRowsChange` changes
or when client-side mode is re-entered. If the consumer callback is replaced,
or `serverPagination` changes from true back to false while the rows are
unchanged, the new client-side consumer receives no initial snapshot because
the signature is considered already emitted. Reset the signature whenever the
callback or client-side lifecycle changes, or track the consumer identity with
the signature. [stale reference]
<details>
<summary><b>Severity Level:</b> Major ⚠️</summary>
```mdx
- ⚠️ Client-side export can remain stale after pagination-mode changes.
- ⚠️ Re-entering client mode may omit the current filtered rows.
```
</details>
[](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=c2f262ef10c1490c8f43c381a1a6f636&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset)
[](https://app.codeant.ai/fix-in-ide?tool=vscode-claude&prompt_id=c2f262ef10c1490c8f43c381a1a6f636&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset)
<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:** 348:349
**Comment:**
*Stale Reference: `lastSigRef.current` is updated before the animation
frame invokes the callback, and it is not reset when `onFilteredRowsChange`
changes or when client-side mode is re-entered. If the consumer callback is
replaced, or `serverPagination` changes from true back to false while the rows
are unchanged, the new client-side consumer receives no initial snapshot
because the signature is considered already emitted. Reset the signature
whenever the callback or client-side lifecycle changes, or track the consumer
identity with the signature.
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%2F43184&comment_hash=63320e986d8412ff1d59dee5f2c7b8e4f0b4e4a8900704a2e24a55505c644605&reaction=like'>👍</a>
| <a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F43184&comment_hash=63320e986d8412ff1d59dee5f2c7b8e4f0b4e4a8900704a2e24a55505c644605&reaction=dislike'>👎</a>
##########
superset-frontend/plugins/plugin-chart-table/src/DataTable/DataTable.tsx:
##########
@@ -103,6 +103,41 @@ const sortTypes = {
alphanumeric: sortAlphanumericCaseInsensitive,
};
+// Prefer a stable identifier from original row data; otherwise use a
deterministic
+// concatenation of visible values (keys sorted so column order changes are
detected).
+function stableRowKey<D extends object>(r: Row<D>): string {
+ const orig = r.original as Record<string, unknown> | undefined;
+ if (orig) {
+ const idLike = orig.id ?? orig.ID ?? orig.key ?? orig.uuid;
+ if (idLike != null) return String(idLike);
Review Comment:
**Suggestion:** The signature uses only the first id-like field when one
exists, so changing any other value in a row with a stable identifier produces
the same signature. The RAF effect then suppresses `onFilteredRowsChange`,
leaving the parent’s client-side export snapshot stale. Include the row values
in the signature in addition to the identifier, or otherwise detect value
changes. [logic error]
<details>
<summary><b>Severity Level:</b> Major ⚠️</summary>
```mdx
- ❌ Client-side export snapshots can retain stale row values.
- ⚠️ `TableChart` ownState client-view data misses in-place row updates.
```
</details>
[](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=61b665f4fd0c4a82bf0ce984444afc3b&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset)
[](https://app.codeant.ai/fix-in-ide?tool=vscode-claude&prompt_id=61b665f4fd0c4a82bf0ce984444afc3b&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset)
<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:** 111:112
**Comment:**
*Logic Error: The signature uses only the first id-like field when one
exists, so changing any other value in a row with a stable identifier produces
the same signature. The RAF effect then suppresses `onFilteredRowsChange`,
leaving the parent’s client-side export snapshot stale. Include the row values
in the signature in addition to the identifier, or otherwise detect value
changes.
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%2F43184&comment_hash=38d5cb7dca17bb9a9ba1804026e3c2af9e52d3baae427f53474e140288d40af0&reaction=like'>👍</a>
| <a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F43184&comment_hash=38d5cb7dca17bb9a9ba1804026e3c2af9e52d3baae427f53474e140288d40af0&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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
