pierrejeambrun commented on code in PR #53836:
URL: https://github.com/apache/airflow/pull/53836#discussion_r2239983697
##########
airflow-core/src/airflow/ui/src/layouts/Details/Graph/Graph.tsx:
##########
@@ -94,11 +94,14 @@ export const Graph = () => {
[graphData.nodes],
);
+ const prevObservedGroupIds = useRef<Array<string>>([]);
Review Comment:
We don't need a ref
##########
airflow-core/src/airflow/ui/src/layouts/Details/Graph/Graph.tsx:
##########
@@ -94,11 +94,14 @@ export const Graph = () => {
[graphData.nodes],
);
+ const prevObservedGroupIds = useRef<Array<string>>([]);
+
useEffect(() => {
- if (observedGroupIds !== allGroupIds) {
+ if (JSON.stringify(observedGroupIds) !==
JSON.stringify(prevObservedGroupIds.current)) {
Review Comment:
Even if we do that sometimes for comparing complex object (I think we really
shouldn't because things will fail for circular references), it's really easy
to do a proper comparison here for simple array of int object. At some point we
might want to install lodash library to defer all that logic to a helper
package.
##########
airflow-core/src/airflow/ui/src/layouts/Details/Graph/Graph.tsx:
##########
@@ -94,11 +94,14 @@ export const Graph = () => {
[graphData.nodes],
);
+ const prevObservedGroupIds = useRef<Array<string>>([]);
Review Comment:
We can also take the opportunity to remove the useMemo.
--
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]
