pierrejeambrun commented on code in PR #64271:
URL: https://github.com/apache/airflow/pull/64271#discussion_r3116552099
##########
airflow-core/src/airflow/ui/src/context/groups/GroupsProvider.tsx:
##########
@@ -25,28 +25,25 @@ import { allGroupsKey, dependenciesKey, openGroupsKey }
from "src/constants/loca
import useSelectedVersion from "src/hooks/useSelectedVersion";
import { flattenGraphNodes } from "src/layouts/Details/Grid/utils";
-import { OpenGroupsContext, type OpenGroupsContextType } from "./Context";
+import { GroupsContext, type GroupsContextType } from "./Context";
type Props = {
readonly dagId: string;
} & PropsWithChildren;
-export const OpenGroupsProvider = ({ children, dagId }: Props) => {
+export const GroupsProvider = ({ children, dagId }: Props) => {
const [openGroupIds, setOpenGroupIds] =
useLocalStorage<Array<string>>(openGroupsKey(dagId), []);
const [allGroupIds, setAllGroupIds] =
useLocalStorage<Array<string>>(allGroupsKey(dagId), []);
Review Comment:
`allGroupIds` is derived from the DAG structure API — it shouldn't live in
localStorage. The source of truth is the structure response; persisting this
means stale data can linger across DAG changes (e.g., user removes a task group
in code, localStorage still has it until the effect at line 60 rewrites). Just
compute it from `structure.nodes` each render (memoized) and pass through
context.
--
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]
