From 05fbf3557d6ef3bc6affa481bd80e3c0924ee0bd Mon Sep 17 00:00:00 2001 From: Camiel van Schoonhoven Date: Wed, 25 Feb 2026 14:34:51 -0800 Subject: [PATCH] fix: ReactFlow Warnings due Edge Rendering & Hydration Conflicts --- .../InputValueEditor.test.tsx | 46 ++++- .../PipelineRun/RunDetails.test.tsx | 154 +++++++++++----- .../PipelineRun/RunToolbar.test.tsx | 155 ++++++++++++----- .../ComponentEditorDialog.test.tsx | 164 ++++++++++-------- src/hooks/useHydrateComponentReference.ts | 38 +++- src/providers/ComponentSpecProvider.tsx | 19 +- tests/e2e/secrets-in-arguments.spec.ts | 7 +- 7 files changed, 396 insertions(+), 187 deletions(-) diff --git a/src/components/Editor/IOEditor/InputValueEditor/InputValueEditor.test.tsx b/src/components/Editor/IOEditor/InputValueEditor/InputValueEditor.test.tsx index a406886a6..d07009bd0 100644 --- a/src/components/Editor/IOEditor/InputValueEditor/InputValueEditor.test.tsx +++ b/src/components/Editor/IOEditor/InputValueEditor/InputValueEditor.test.tsx @@ -1,3 +1,4 @@ +import { QueryClient, QueryClientProvider } from "@tanstack/react-query"; import { fireEvent, render, screen } from "@testing-library/react"; import { beforeEach, describe, expect, it, vi } from "vitest"; @@ -15,6 +16,18 @@ vi.mock("@/components/Editor/IOEditor/IOZIndexEditor", () => ({ IOZIndexEditor: () => null, })); +vi.mock("@/services/componentService", () => ({ + hydrateComponentReference: vi.fn((ref) => + Promise.resolve({ + ...ref, + spec: ref.spec || { inputs: [], outputs: [] }, + text: ref.text || "", + digest: ref.digest || "mock-digest", + name: ref.name || "mock-component", + }), + ), +})); + vi.mock("@/providers/ComponentSpecProvider", () => ({ useComponentSpec: () => ({ componentSpec: { @@ -42,6 +55,9 @@ vi.mock("@/providers/ComponentSpecProvider", () => ({ currentSubgraphPath: ["root"], setComponentSpec: mockSetComponentSpec, }), + ComponentSpecProvider: ({ children }: { children: React.ReactNode }) => ( + <>{children} + ), })); vi.mock("@/providers/ContextPanelProvider", () => ({ @@ -95,12 +111,28 @@ describe("InputValueEditor", () => { default: "default value", }; + let queryClient: QueryClient; + beforeEach(() => { vi.clearAllMocks(); + + queryClient = new QueryClient({ + defaultOptions: { + queries: { + retry: false, + }, + }, + }); }); + const renderWithQueryClient = (ui: React.ReactElement) => { + return render( + {ui}, + ); + }; + it("displays input description in field", () => { - render(); + renderWithQueryClient(); const descriptionInput = screen.getByLabelText( "Description", @@ -109,7 +141,7 @@ describe("InputValueEditor", () => { }); it("calls onChange when input value changes", () => { - render(); + renderWithQueryClient(); const valueInput = screen.getByLabelText("Value") as HTMLTextAreaElement; fireEvent.change(valueInput, { target: { value: "new value" } }); @@ -120,7 +152,7 @@ describe("InputValueEditor", () => { }); it("calls onNameChange when input name changes", () => { - render(); + renderWithQueryClient(); const nameInput = screen.getByLabelText("Name") as HTMLInputElement; fireEvent.change(nameInput, { target: { value: "NewName" } }); fireEvent.blur(nameInput); @@ -131,7 +163,7 @@ describe("InputValueEditor", () => { }); it("shows validation error when renaming to existing input name", () => { - render(); + renderWithQueryClient(); const nameInput = screen.getByLabelText("Name") as HTMLInputElement; fireEvent.change(nameInput, { target: { value: "ExistingInput" } }); @@ -146,7 +178,7 @@ describe("InputValueEditor", () => { }); it("clears validation error when renaming to unique name", () => { - render(); + renderWithQueryClient(); const nameInput = screen.getByLabelText("Name") as HTMLInputElement; @@ -170,7 +202,7 @@ describe("InputValueEditor", () => { type: "String", }; - render(); + renderWithQueryClient(); const valueInput = screen.getByLabelText("Value") as HTMLInputElement; expect(valueInput.getAttribute("placeholder")).toBe( @@ -179,7 +211,7 @@ describe("InputValueEditor", () => { }); it("shows default value as placeholder when available", () => { - render(); + renderWithQueryClient(); const valueInput = screen.getByLabelText("Value") as HTMLInputElement; expect(valueInput.getAttribute("placeholder")).toBe("default value"); diff --git a/src/components/PipelineRun/RunDetails.test.tsx b/src/components/PipelineRun/RunDetails.test.tsx index b3206f15b..f077cf285 100644 --- a/src/components/PipelineRun/RunDetails.test.tsx +++ b/src/components/PipelineRun/RunDetails.test.tsx @@ -2,6 +2,7 @@ import { QueryClient, QueryClientProvider } from "@tanstack/react-query"; import { screen, waitFor } from "@testing-library/dom"; import { cleanup, render } from "@testing-library/react"; import { ReactFlowProvider } from "@xyflow/react"; +import { Suspense } from "react"; import { afterEach, beforeEach, describe, expect, test, vi } from "vitest"; import type { @@ -19,6 +20,22 @@ import type { ComponentSpec } from "@/utils/componentSpec"; import { RunDetails } from "./RunDetails"; +// Use vi.hoisted to create mocks that can be referenced in vi.mock +const { mockHydrateComponentReference } = vi.hoisted(() => ({ + mockHydrateComponentReference: vi.fn(), +})); + +vi.mock("@/services/componentService", () => ({ + hydrateComponentReference: mockHydrateComponentReference, + fetchAndStoreComponentLibrary: vi.fn(), + fetchAndStoreComponent: vi.fn(), + fetchAndStoreComponentByUrl: vi.fn(), + getComponentText: vi.fn(), + fetchComponentTextFromUrl: vi.fn(), + parseComponentData: vi.fn(), + getExistingAndNewUserComponent: vi.fn(), +})); + // Mock the hooks and services vi.mock("@tanstack/react-router", async (importOriginal) => { return { @@ -77,6 +94,11 @@ describe("", () => { }, inputs: [], outputs: [], + implementation: { + graph: { + tasks: {}, + }, + }, }, }, }, @@ -104,10 +126,8 @@ describe("", () => { inputs: [], outputs: [], implementation: { - container: { - image: "test-image", - command: ["test-command"], - args: ["test-arg"], + graph: { + tasks: {}, }, }, }; @@ -123,6 +143,14 @@ describe("", () => { // Reset all mocks vi.clearAllMocks(); + // Mock hydrateComponentReference to return the spec immediately + mockHydrateComponentReference.mockResolvedValue({ + spec: mockComponentSpec, + name: mockComponentSpec.name, + digest: "test-digest", + text: "name: Test Pipeline", + }); + vi.mocked(usePipelineRunData).mockReturnValue({ executionData: { details: mockExecutionDetails, @@ -159,15 +187,17 @@ describe("", () => { const renderWithProviders = (component: React.ReactElement) => { return render(component, { wrapper: ({ children }) => ( - - - - - {children} - - - - + + Loading...}> + + + + {children} + + + + + ), }); }; @@ -176,48 +206,63 @@ describe("", () => { test("should render pipeline name", async () => { renderWithProviders(); - await waitFor(() => { - expect(screen.getByText("Test Pipeline")).toBeInTheDocument(); - }); + await waitFor( + () => { + expect(screen.getByText("Test Pipeline")).toBeInTheDocument(); + }, + { timeout: 5000 }, + ); }); test("should render run metadata", async () => { renderWithProviders(); - await waitFor(() => { - expect(screen.getByText("Run Info")).toBeInTheDocument(); - expect(screen.getByText("123")).toBeInTheDocument(); - expect(screen.getByText("456")).toBeInTheDocument(); - expect(screen.getByText("test-user")).toBeInTheDocument(); - }); + await waitFor( + () => { + expect(screen.getByText("Run Info")).toBeInTheDocument(); + expect(screen.getByText("123")).toBeInTheDocument(); + expect(screen.getByText("456")).toBeInTheDocument(); + expect(screen.getByText("test-user")).toBeInTheDocument(); + }, + { timeout: 5000 }, + ); }); test("should render description", async () => { renderWithProviders(); - await waitFor(() => { - expect( - screen.getByText("Test pipeline description"), - ).toBeInTheDocument(); - }); + await waitFor( + () => { + expect( + screen.getByText("Test pipeline description"), + ).toBeInTheDocument(); + }, + { timeout: 5000 }, + ); }); test("should render status", async () => { renderWithProviders(); - await waitFor(() => { - expect(screen.getByText("Status")).toBeInTheDocument(); - }); + await waitFor( + () => { + expect(screen.getByText("Status")).toBeInTheDocument(); + }, + { timeout: 5000 }, + ); }); test("should render annotations", async () => { renderWithProviders(); - await waitFor(() => { - expect(screen.getByText("Annotations")).toBeInTheDocument(); - expect(screen.getByText("test-annotation")).toBeInTheDocument(); - expect(screen.getByText("test-value")).toBeInTheDocument(); - }); + await waitFor( + () => { + expect(screen.getByText("Annotations")).toBeInTheDocument(); + expect(screen.getByText("test-annotation")).toBeInTheDocument(); + expect(screen.getByText("test-value")).toBeInTheDocument(); + }, + { timeout: 5000 }, + ); }); }); @@ -235,11 +280,14 @@ describe("", () => { renderWithProviders(); - await waitFor(() => { - expect( - screen.getByText("Pipeline Run could not be loaded."), - ).toBeInTheDocument(); - }); + await waitFor( + () => { + expect( + screen.getByText("Pipeline Run could not be loaded."), + ).toBeInTheDocument(); + }, + { timeout: 5000 }, + ); }); test("should show loading screen when data is loading", async () => { @@ -255,9 +303,14 @@ describe("", () => { renderWithProviders(); - await waitFor(() => { - expect(screen.getByText("Loading run details...")).toBeInTheDocument(); - }); + await waitFor( + () => { + expect( + screen.getByText("Loading run details..."), + ).toBeInTheDocument(); + }, + { timeout: 5000 }, + ); }); test("should show backend not configured message when backend is not configured", async () => { @@ -276,11 +329,16 @@ describe("", () => { renderWithProviders(); - await waitFor(() => { - expect( - screen.getByText("Configure a backend to view execution artifacts."), - ).toBeInTheDocument(); - }); + await waitFor( + () => { + expect( + screen.getByText( + "Configure a backend to view execution artifacts.", + ), + ).toBeInTheDocument(); + }, + { timeout: 5000 }, + ); }); }); }); diff --git a/src/components/PipelineRun/RunToolbar.test.tsx b/src/components/PipelineRun/RunToolbar.test.tsx index d3c603686..8ec237850 100644 --- a/src/components/PipelineRun/RunToolbar.test.tsx +++ b/src/components/PipelineRun/RunToolbar.test.tsx @@ -2,6 +2,7 @@ import { QueryClient, QueryClientProvider } from "@tanstack/react-query"; import { screen, waitFor } from "@testing-library/dom"; import { cleanup, render } from "@testing-library/react"; import { ReactFlowProvider } from "@xyflow/react"; +import { Suspense } from "react"; import { afterEach, beforeEach, describe, expect, test, vi } from "vitest"; import type { @@ -18,6 +19,22 @@ import type { ComponentSpec } from "@/utils/componentSpec"; import { RunToolbar } from "./RunToolbar"; +// Use vi.hoisted to create mocks that can be referenced in vi.mock +const { mockHydrateComponentReference } = vi.hoisted(() => ({ + mockHydrateComponentReference: vi.fn(), +})); + +vi.mock("@/services/componentService", () => ({ + hydrateComponentReference: mockHydrateComponentReference, + fetchAndStoreComponentLibrary: vi.fn(), + fetchAndStoreComponent: vi.fn(), + fetchAndStoreComponentByUrl: vi.fn(), + getComponentText: vi.fn(), + fetchComponentTextFromUrl: vi.fn(), + parseComponentData: vi.fn(), + getExistingAndNewUserComponent: vi.fn(), +})); + vi.mock("@tanstack/react-router", async (importOriginal) => ({ ...(await importOriginal()), useNavigate: () => vi.fn(), @@ -59,6 +76,11 @@ describe("", () => { }, inputs: [], outputs: [], + implementation: { + graph: { + tasks: {}, + }, + }, }, }, }, @@ -86,10 +108,8 @@ describe("", () => { inputs: [], outputs: [], implementation: { - container: { - image: "test-image", - command: ["test-command"], - args: ["test-arg"], + graph: { + tasks: {}, }, }, }; @@ -104,6 +124,14 @@ describe("", () => { beforeEach(() => { vi.clearAllMocks(); + // Mock hydrateComponentReference to return the spec immediately + mockHydrateComponentReference.mockResolvedValue({ + spec: mockComponentSpec, + name: mockComponentSpec.name, + digest: "test-digest", + text: "name: Test Pipeline", + }); + vi.mocked(usePipelineRunData).mockReturnValue({ executionData: { details: mockExecutionDetails, @@ -140,13 +168,15 @@ describe("", () => { const renderWithProviders = (component: React.ReactElement) => { return render(component, { wrapper: ({ children }) => ( - - - - {children} - - - + + Loading...}> + + + {children} + + + + ), }); }; @@ -157,10 +187,13 @@ describe("", () => { renderWithProviders(); - await waitFor(() => { - const inspect = screen.getByTestId("inspect-pipeline-button"); - expect(inspect).toBeInTheDocument(); - }); + await waitFor( + () => { + const inspect = screen.getByTestId("inspect-pipeline-button"); + expect(inspect).toBeInTheDocument(); + }, + { timeout: 5000 }, + ); }); test("should NOT render inspect button when pipeline does not exist", async () => { @@ -168,10 +201,13 @@ describe("", () => { renderWithProviders(); - await waitFor(() => { - const inspect = screen.queryByTestId("inspect-pipeline-button"); - expect(inspect).not.toBeInTheDocument(); - }); + await waitFor( + () => { + const inspect = screen.queryByTestId("inspect-pipeline-button"); + expect(inspect).not.toBeInTheDocument(); + }, + { timeout: 5000 }, + ); }); }); @@ -179,10 +215,13 @@ describe("", () => { test("should render clone button", async () => { renderWithProviders(); - await waitFor(() => { - const cloneButton = screen.getByTestId("clone-pipeline-run-button"); - expect(cloneButton).toBeInTheDocument(); - }); + await waitFor( + () => { + const cloneButton = screen.getByTestId("clone-pipeline-run-button"); + expect(cloneButton).toBeInTheDocument(); + }, + { timeout: 5000 }, + ); }); }); @@ -190,10 +229,13 @@ describe("", () => { test("should render cancel button when status is RUNNING and user is the creator of the run", async () => { renderWithProviders(); - await waitFor(() => { - const cancelButton = screen.getByTestId("cancel-pipeline-run-button"); - expect(cancelButton).toBeInTheDocument(); - }); + await waitFor( + () => { + const cancelButton = screen.getByTestId("cancel-pipeline-run-button"); + expect(cancelButton).toBeInTheDocument(); + }, + { timeout: 5000 }, + ); }); test("should NOT render cancel button when status is not RUNNING", async () => { @@ -214,10 +256,15 @@ describe("", () => { renderWithProviders(); - await waitFor(() => { - const cancelButton = screen.queryByTestId("cancel-pipeline-run-button"); - expect(cancelButton).not.toBeInTheDocument(); - }); + await waitFor( + () => { + const cancelButton = screen.queryByTestId( + "cancel-pipeline-run-button", + ); + expect(cancelButton).not.toBeInTheDocument(); + }, + { timeout: 5000 }, + ); }); test("should NOT render cancel button when the user is not the creator of the run", async () => { @@ -233,10 +280,15 @@ describe("", () => { renderWithProviders(); - await waitFor(() => { - const cancelButton = screen.queryByTestId("cancel-pipeline-run-button"); - expect(cancelButton).not.toBeInTheDocument(); - }); + await waitFor( + () => { + const cancelButton = screen.queryByTestId( + "cancel-pipeline-run-button", + ); + expect(cancelButton).not.toBeInTheDocument(); + }, + { timeout: 5000 }, + ); }); }); @@ -259,19 +311,25 @@ describe("", () => { renderWithProviders(); - await waitFor(() => { - const rerunButton = screen.getByTestId("rerun-pipeline-button"); - expect(rerunButton).toBeInTheDocument(); - }); + await waitFor( + () => { + const rerunButton = screen.getByTestId("rerun-pipeline-button"); + expect(rerunButton).toBeInTheDocument(); + }, + { timeout: 5000 }, + ); }); test("should NOT render rerun button when status is RUNNING", async () => { renderWithProviders(); - await waitFor(() => { - const rerunButton = screen.queryByTestId("rerun-pipeline-button"); - expect(rerunButton).not.toBeInTheDocument(); - }); + await waitFor( + () => { + const rerunButton = screen.queryByTestId("rerun-pipeline-button"); + expect(rerunButton).not.toBeInTheDocument(); + }, + { timeout: 5000 }, + ); }); }); @@ -279,10 +337,13 @@ describe("", () => { test("should always render view yaml button", async () => { renderWithProviders(); - await waitFor(() => { - const viewYamlButton = screen.getByTestId("action-View"); - expect(viewYamlButton).toBeInTheDocument(); - }); + await waitFor( + () => { + const viewYamlButton = screen.getByTestId("action-View"); + expect(viewYamlButton).toBeInTheDocument(); + }, + { timeout: 5000 }, + ); }); }); }); diff --git a/src/components/shared/ComponentEditor/ComponentEditorDialog.test.tsx b/src/components/shared/ComponentEditor/ComponentEditorDialog.test.tsx index 55bcbae03..11ff05a5c 100644 --- a/src/components/shared/ComponentEditor/ComponentEditorDialog.test.tsx +++ b/src/components/shared/ComponentEditor/ComponentEditorDialog.test.tsx @@ -1,19 +1,31 @@ import { QueryClient, QueryClientProvider } from "@tanstack/react-query"; import { fireEvent, render, screen, waitFor } from "@testing-library/react"; -import { ReactFlowProvider } from "@xyflow/react"; import { type ReactNode } from "react"; -import { beforeEach, describe, expect, test, vi } from "vitest"; +import { afterEach, beforeEach, describe, expect, test, vi } from "vitest"; import useToastNotification from "@/hooks/useToastNotification"; import { useComponentLibrary } from "@/providers/ComponentLibraryProvider"; -import { ComponentSpecProvider } from "@/providers/ComponentSpecProvider"; -import { ContextPanelProvider } from "@/providers/ContextPanelProvider"; -import { hydrateComponentReference } from "@/services/componentService"; import { saveComponent } from "@/utils/localforage"; import { ComponentEditorDialog } from "./ComponentEditorDialog"; -// Mock only what's necessary +// Use vi.hoisted to create mocks that can be referenced in vi.mock +const { mockHydrateComponentReference } = vi.hoisted(() => ({ + mockHydrateComponentReference: vi.fn(), +})); + +// Mock the entire module +vi.mock("@/services/componentService", () => ({ + hydrateComponentReference: mockHydrateComponentReference, + fetchAndStoreComponentLibrary: vi.fn(), + fetchAndStoreComponent: vi.fn(), + fetchAndStoreComponentByUrl: vi.fn(), + getComponentText: vi.fn(), + fetchComponentTextFromUrl: vi.fn(), + parseComponentData: vi.fn(), + getExistingAndNewUserComponent: vi.fn(), +})); + vi.mock("@/hooks/useToastNotification", () => ({ default: vi.fn(), })); @@ -22,18 +34,10 @@ vi.mock("@/providers/ComponentLibraryProvider", () => ({ useComponentLibrary: vi.fn(), })); -vi.mock("@/services/componentService", async (importOriginal) => { - return { - ...(await importOriginal()), - hydrateComponentReference: vi.fn(), - }; -}); - vi.mock("@/utils/localforage", () => ({ saveComponent: vi.fn(), })); -// Mock the Python generator to avoid Pyodide loading in tests vi.mock("./generators/python", () => ({ usePythonYamlGenerator: () => { return vi.fn().mockResolvedValue(`name: Generated Component @@ -45,23 +49,11 @@ outputs: })); describe("", () => { - const queryClient = new QueryClient({ - defaultOptions: { - queries: { - retry: false, - }, - }, - }); + let queryClient: QueryClient; const TestWrapper = ({ children }: { children: ReactNode }) => { return ( - - - - {children} - - - + {children} ); }; @@ -69,42 +61,48 @@ describe("", () => { return render(component, { wrapper: TestWrapper }); }; - // Set up default mocks const mockToast = vi.fn(); const mockAddToComponentLibrary = vi.fn(); beforeEach(() => { vi.clearAllMocks(); + + queryClient = new QueryClient({ + defaultOptions: { + queries: { + retry: false, + }, + }, + }); + vi.mocked(useToastNotification).mockReturnValue(mockToast); vi.mocked(useComponentLibrary).mockReturnValue({ addToComponentLibrary: mockAddToComponentLibrary, } as any); - vi.mocked(hydrateComponentReference).mockResolvedValue(null); + + // Default: return null for components without Python code + mockHydrateComponentReference.mockResolvedValue(null); vi.mocked(saveComponent).mockImplementation(async (component) => component); }); + afterEach(() => { + queryClient.clear(); + }); + test("calls onClose when close button is clicked", async () => { const onCloseMock = vi.fn(); renderWithProviders(); - // Wait for the component to render (suspense boundary) await waitFor(() => { expect( screen.getByRole("heading", { name: "New Component" }), ).toBeInTheDocument(); }); - // Find all buttons and identify the close button by its variant and icon const buttons = screen.getAllByRole("button"); - - // The close button should be the last button (after the Save button) - // and should have variant="ghost" and size="icon" const closeButton = buttons[buttons.length - 1]; - expect(closeButton).toBeDefined(); - - // Click the close button fireEvent.click(closeButton); expect(onCloseMock).toHaveBeenCalledTimes(1); @@ -122,7 +120,6 @@ describe("", () => { ).toBeInTheDocument(); }); - // Should not show template name for empty template expect(screen.queryByText("(empty template)")).not.toBeInTheDocument(); }); @@ -137,7 +134,6 @@ describe("", () => { ).toBeInTheDocument(); }); - // Should show the template name in subtitle expect(screen.getByText("(python template)")).toBeInTheDocument(); }); @@ -152,7 +148,6 @@ describe("", () => { ).toBeInTheDocument(); }); - // Should show the template name in subtitle expect(screen.getByText("(bash template)")).toBeInTheDocument(); }); @@ -167,7 +162,6 @@ describe("", () => { ).toBeInTheDocument(); }); - // Should show the template name in subtitle expect(screen.getByText("(javascript template)")).toBeInTheDocument(); }); @@ -182,7 +176,6 @@ describe("", () => { ).toBeInTheDocument(); }); - // Should show the template name in subtitle expect(screen.getByText("(ruby template)")).toBeInTheDocument(); }); }); @@ -217,7 +210,12 @@ describe("", () => { test("renders PythonComponentEditor for component with python_original_code annotation", async () => { const mockComponent: any = { spec: { - implementation: { container: { image: "test" } }, + name: "test-component", + implementation: { + container: { + image: "python:3.12", + }, + }, metadata: { annotations: { python_original_code: "def my_function():\n return 'hello'", @@ -229,31 +227,33 @@ describe("", () => { text: "name: test-component", }; - vi.mocked(hydrateComponentReference).mockResolvedValueOnce(mockComponent); + mockHydrateComponentReference.mockResolvedValue(mockComponent); renderWithProviders( , ); - await waitFor(() => { - expect(screen.getByTestId("python-editor")).toBeInTheDocument(); - }); + await waitFor( + () => { + expect(screen.getByTestId("python-editor")).toBeInTheDocument(); + }, + { timeout: 10000 }, + ); expect( screen.queryByTestId("yaml-editor-preview"), ).not.toBeInTheDocument(); - // Verify we have both Python editor and preview sections expect(screen.getByTestId("python-editor-preview")).toBeInTheDocument(); }); test("renders YamlComponentEditor when component has no python annotations", async () => { const mockComponent: any = { spec: { + name: "test-component", implementation: { container: { image: "test" } }, metadata: { annotations: { - // No python_original_code annotation author: "test-author", }, }, @@ -263,7 +263,7 @@ describe("", () => { text: "name: test-component\ninputs:\n- {name: Input}", }; - vi.mocked(hydrateComponentReference).mockResolvedValueOnce(mockComponent); + mockHydrateComponentReference.mockResolvedValue(mockComponent); renderWithProviders( ", () => { const onCloseMock = vi.fn(); const mockHydratedComponent = { spec: { + name: "test-component", implementation: { container: { image: "test" } }, metadata: { annotations: {} }, }, @@ -293,9 +294,7 @@ describe("", () => { text: "name: test-component", }; - vi.mocked(hydrateComponentReference).mockResolvedValue( - mockHydratedComponent, - ); + mockHydrateComponentReference.mockResolvedValue(mockHydratedComponent); renderWithProviders( ", () => { />, ); - await waitFor(() => { - expect( - screen.getByRole("button", { name: /Save/i }), - ).toBeInTheDocument(); - }); + // Wait for component to fully load + await waitFor( + () => { + expect(screen.getByTestId("yaml-editor-preview")).toBeInTheDocument(); + }, + { timeout: 10000 }, + ); + + // Wait for Save button to be ready + await waitFor( + () => { + const saveButton = screen.getByRole("button", { name: /Save/i }); + expect(saveButton).not.toBeDisabled(); + }, + { timeout: 5000 }, + ); const saveButton = screen.getByRole("button", { name: /Save/i }); fireEvent.click(saveButton); - await waitFor(() => { - // Verify saveComponent was called - expect(saveComponent).toHaveBeenCalled(); - - // Verify addToComponentLibrary was called - expect(mockAddToComponentLibrary).toHaveBeenCalledWith( - mockHydratedComponent, - ); - - // Verify success toast notification was shown - expect(mockToast).toHaveBeenCalledWith( - `Component ${mockHydratedComponent.name} imported successfully`, - "success", - ); - - // Verify onClose was called after successful save - expect(onCloseMock).toHaveBeenCalledTimes(1); - }); + // Wait for save operations to complete + await waitFor( + () => { + expect(saveComponent).toHaveBeenCalled(); + }, + { timeout: 5000 }, + ); + + expect(mockAddToComponentLibrary).toHaveBeenCalledWith( + mockHydratedComponent, + ); + + expect(mockToast).toHaveBeenCalledWith( + `Component ${mockHydratedComponent.name} imported successfully`, + "success", + ); + + expect(onCloseMock).toHaveBeenCalledTimes(1); }); }); }); diff --git a/src/hooks/useHydrateComponentReference.ts b/src/hooks/useHydrateComponentReference.ts index cd3c7028f..6c4fc0a84 100644 --- a/src/hooks/useHydrateComponentReference.ts +++ b/src/hooks/useHydrateComponentReference.ts @@ -1,7 +1,12 @@ -import { useSuspenseQuery } from "@tanstack/react-query"; +import { useSuspenseQueries, useSuspenseQuery } from "@tanstack/react-query"; import { hydrateComponentReference } from "@/services/componentService"; -import type { ComponentReference } from "@/utils/componentSpec"; +import type { ComponentReference, ComponentSpec } from "@/utils/componentSpec"; +import { + isGraphImplementation, + isNotMaterializedComponentReference, +} from "@/utils/componentSpec"; +import { TWENTY_FOUR_HOURS_IN_MS } from "@/utils/constants"; import { componentSpecToText } from "@/utils/yaml"; /** @@ -30,6 +35,10 @@ function getComponentQueryKey(component: ComponentReference): string { return `empty:${JSON.stringify(component)}`; } +const generateHydrationQueryKey = (component: ComponentReference) => { + return ["component", "hydrate", getComponentQueryKey(component)]; +}; + /** * Hydrate a component reference by fetching the text and spec from the URL or local storage * This is experimental function, that potentially can replace all other methods of getting ComponentRef. @@ -43,12 +52,12 @@ export function useHydrateComponentReference(component: ComponentReference) { * Otherwise we dont cache result. */ - const componentQueryKey = getComponentQueryKey(component); + const componentQueryKey = generateHydrationQueryKey(component); - const staleTime = componentQueryKey ? 1000 * 60 * 60 * 1 : 0; + const staleTime = componentQueryKey[2] ? 1000 * 60 * 60 * 1 : 0; const { data: componentRef } = useSuspenseQuery({ - queryKey: ["component", "hydrate", componentQueryKey], + queryKey: componentQueryKey, staleTime, retryOnMount: true, queryFn: () => hydrateComponentReference(component), @@ -77,3 +86,22 @@ export function useGuaranteedHydrateComponentReference( return hydratedComponentRef; } + +export function usePreHydrateComponentRefs(componentSpec: ComponentSpec) { + const tasks = isGraphImplementation(componentSpec.implementation) + ? componentSpec.implementation.graph?.tasks + : {}; + const componentRefs = Object.values(tasks) + .map((task) => task.componentRef) + .filter((ref) => ref && !isNotMaterializedComponentReference(ref)); + + const results = useSuspenseQueries({ + queries: componentRefs.map((ref) => ({ + queryKey: generateHydrationQueryKey(ref), + queryFn: () => hydrateComponentReference(ref), + staleTime: TWENTY_FOUR_HOURS_IN_MS, + })), + }); + + return results.map((result) => result.data); +} diff --git a/src/providers/ComponentSpecProvider.tsx b/src/providers/ComponentSpecProvider.tsx index 4146a1fb7..97db97d74 100644 --- a/src/providers/ComponentSpecProvider.tsx +++ b/src/providers/ComponentSpecProvider.tsx @@ -8,6 +8,8 @@ import { useState, } from "react"; +import { SuspenseWrapper } from "@/components/shared/SuspenseWrapper"; +import { usePreHydrateComponentRefs } from "@/hooks/useHydrateComponentReference"; import { type UndoRedo, useUndoRedo } from "@/hooks/useUndoRedo"; import { loadPipelineByName } from "@/services/pipelineService"; import { USER_PIPELINES_LIST_NAME } from "@/utils/constants"; @@ -77,7 +79,7 @@ const ComponentSpecContext = createRequiredContext( "ComponentSpecProvider", ); -export const ComponentSpecProvider = ({ +const ComponentSpecProviderInternal = ({ spec, readOnly = false, children, @@ -89,6 +91,9 @@ export const ComponentSpecProvider = ({ const [componentSpec, setComponentSpec] = useState( spec ?? EMPTY_GRAPH_COMPONENT_SPEC, ); + + usePreHydrateComponentRefs(componentSpec); + const [digest, setDigest] = useState(""); const [isLoading, setIsLoading] = useState(!!spec); @@ -319,6 +324,18 @@ export const ComponentSpecProvider = ({ ); }; +export const ComponentSpecProvider = (props: { + spec?: ComponentSpec; + readOnly?: boolean; + children: ReactNode; +}) => { + return ( + + + + ); +}; + export const useComponentSpec = () => { return useRequiredContext(ComponentSpecContext); }; diff --git a/tests/e2e/secrets-in-arguments.spec.ts b/tests/e2e/secrets-in-arguments.spec.ts index 97fb5515a..ed91294f7 100644 --- a/tests/e2e/secrets-in-arguments.spec.ts +++ b/tests/e2e/secrets-in-arguments.spec.ts @@ -227,7 +227,9 @@ async function cleanupTestSecret( /** * Drops the Github - Fake Commit Push component onto the canvas - * by reading the fixture file and using drag-and-drop + * by reading the fixture file and using drag-and-drop. + * + * Waits for component hydration to complete before returning. */ async function dropGithubFakeCommitPushComponent(page: Page) { const buffer = readFileSync( @@ -260,8 +262,9 @@ async function dropGithubFakeCommitPushComponent(page: Page) { const componentName = "Github - Fake Commit Push"; const node = page.locator(`[data-testid="rf__node-task_${componentName}"]`); + await expect(node, "Component node should appear on canvas").toBeVisible({ - timeout: 10000, + timeout: 30000, }); return node;