Re: [PR] chore: Improve performance to load the chart properties modal [superset]

[via GitHub]

korbit-ai[bot] commented on code in PR #34079:
URL: https://github.com/apache/superset/pull/34079#discussion_r2195186389


##########
superset-frontend/src/explore/components/PropertiesModal/index.tsx:
##########
@@ -101,11 +100,21 @@ function PropertiesModal({
     });
   }
 
-  const fetchChartOwners = useCallback(
-    async function fetchChartOwners() {
+  const fetchChartProperties = useCallback(
+    async function fetchChartProperties() {
+      const queryParams = rison.encode({
+        select_columns: [
+          'owners.id',
+          'owners.first_name',
+          'owners.last_name',
+          'tags.id',
+          'tags.name',
+          'tags.type',
+        ],
+      });
       try {
         const response = await SupersetClient.get({
-          endpoint: `/api/v1/chart/${slice.slice_id}`,
+          endpoint: `/api/v1/chart/${slice.slice_id}?q=${queryParams}`,
         });

Review Comment:
   ### Unmemoized Dependency in useCallback <sub>![category 
Performance](https://img.shields.io/badge/Performance-4f46e5)</sub>
   
   <details>
     <summary>Tell me more</summary>
   
   ###### What is the issue?
   The fetchChartProperties callback is recreated on every render because 
slice.slice_id is in the dependency array but isn't memoized.
   
   
   ###### Why this matters
   This can lead to unnecessary re-renders and API calls when the component 
updates for unrelated reasons.
   
   ###### Suggested change ∙ *Feature Preview*
   Memoize the slice.slice_id value using useMemo or move it to component state 
if it's expected to change:
   ```typescript
   const sliceId = useMemo(() => slice.slice_id, [slice.slice_id]);
   
   const fetchChartProperties = useCallback(
     async function fetchChartProperties() {
       // ... existing code
     },
     [sliceId]
   );
   ```
   
   
   ###### Provide feedback to improve future suggestions
   [![Nice 
Catch](https://img.shields.io/badge/👍%20Nice%20Catch-71BC78)](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/679c71fb-c76c-4155-b545-aa1c47a9dd48/upvote)
 
[![Incorrect](https://img.shields.io/badge/👎%20Incorrect-white)](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/679c71fb-c76c-4155-b545-aa1c47a9dd48?what_not_true=true)
  [![Not in 
Scope](https://img.shields.io/badge/👎%20Out%20of%20PR%20scope-white)](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/679c71fb-c76c-4155-b545-aa1c47a9dd48?what_out_of_scope=true)
 [![Not in coding 
standard](https://img.shields.io/badge/👎%20Not%20in%20our%20standards-white)](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/679c71fb-c76c-4155-b545-aa1c47a9dd48?what_not_in_standard=true)
 
[![Other](https://img.shields.io/badge/👎%20Other-white)](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/679c71fb-c76c-4155-b545-aa1c47a9dd48)
   </details>
   
   <sub>
   
   💬 Looking for more details? Reply to this comment to chat with Korbit.
   </sub>
   
   <!--- korbi internal id:7c5d223c-1d4b-4255-b8d8-ebc47cef8ba9 -->
   
   
   [](7c5d223c-1d4b-4255-b8d8-ebc47cef8ba9)



-- 
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-unsubscr...@superset.apache.org

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


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