Compare commits

...

6 Commits

Author SHA1 Message Date
openhands
9c15e58b59 Fix React createRoot error by targeting the root element instead of document 2025-03-23 20:03:34 +00:00
openhands
65b0fd79c5 Fix React createRoot error by ensuring only one root is created 2025-03-23 19:59:09 +00:00
openhands
4cb690bfa4 Fix hydration issues in status message implementation 2025-03-23 19:56:30 +00:00
openhands
27afb3a561 Fix linting issues 2025-03-23 19:44:56 +00:00
openhands
8ec96b514d Fix React Router tests 2025-03-23 19:41:12 +00:00
openhands
46e72b6185 Migrate status-slice from Redux to React Query 2025-03-23 19:36:12 +00:00
9 changed files with 220 additions and 67 deletions

View File

@@ -4,6 +4,8 @@ import store from "#/store";
import { trackError } from "#/utils/error-handler";
import ActionType from "#/types/action-type";
import { ActionMessage } from "#/types/message";
import { queryClient } from "#/entry.client";
import { statusMessageQueryKey } from "#/hooks/query/use-status-message";
// Mock dependencies
vi.mock("#/utils/error-handler", () => ({
@@ -16,6 +18,12 @@ vi.mock("#/store", () => ({
},
}));
vi.mock("#/entry.client", () => ({
queryClient: {
setQueryData: vi.fn(),
},
}));
describe("Actions Service", () => {
beforeEach(() => {
vi.clearAllMocks();
@@ -32,9 +40,10 @@ describe("Actions Service", () => {
handleStatusMessage(message);
expect(store.dispatch).toHaveBeenCalledWith(expect.objectContaining({
payload: message,
}));
expect(queryClient.setQueryData).toHaveBeenCalledWith(
statusMessageQueryKey,
message
);
});
it("should log error messages and display them in chat", () => {

View File

@@ -1,4 +1,4 @@
import React from "react";
import React, { useState, useEffect, useCallback } from "react";
import { useTranslation } from "react-i18next";
import { useSelector } from "react-redux";
import { showErrorToast } from "#/utils/error-handler";
@@ -11,6 +11,7 @@ import {
} from "#/context/ws-client-provider";
import { useNotification } from "#/hooks/useNotification";
import { browserTab } from "#/utils/browser-tab";
import { useStatusMessage } from "#/hooks/query/use-status-message";
const notificationStates = [
AgentState.AWAITING_USER_INPUT,
@@ -18,16 +19,23 @@ const notificationStates = [
AgentState.AWAITING_USER_CONFIRMATION,
];
// Default status message for SSR to avoid hydration mismatches
const defaultStatusMessage = AGENT_STATUS_MAP[AgentState.INIT].message;
export function AgentStatusBar() {
const { t, i18n } = useTranslation();
const { curAgentState } = useSelector((state: RootState) => state.agent);
const { curStatusMessage } = useSelector((state: RootState) => state.status);
const { curStatusMessage } = useStatusMessage();
const { status } = useWsClient();
const { notify } = useNotification();
const [statusMessage, setStatusMessage] = React.useState<string>("");
// Initialize with default message to ensure consistent server/client rendering
const [statusMessage, setStatusMessage] =
useState<string>(defaultStatusMessage);
const [isClient, setIsClient] = useState<boolean>(false);
const updateStatusMessage = () => {
// Use useCallback to create stable function references
const updateStatusMessage = useCallback(() => {
let message = curStatusMessage.message || "";
if (curStatusMessage?.id) {
const id = curStatusMessage.id.trim();
@@ -48,15 +56,23 @@ export function AgentStatusBar() {
} else {
setStatusMessage(AGENT_STATUS_MAP[curAgentState].message);
}
};
}, [curStatusMessage, curAgentState, i18n, t]);
React.useEffect(() => {
updateStatusMessage();
}, [curStatusMessage.id]);
// Mark when component is mounted on client
useEffect(() => {
setIsClient(true);
}, []);
// Handle window focus/blur
React.useEffect(() => {
if (typeof window === "undefined") return undefined;
// Only update status message after client-side hydration
useEffect(() => {
if (isClient) {
updateStatusMessage();
}
}, [isClient, updateStatusMessage, curStatusMessage.id]);
// Handle window focus/blur - only on client
useEffect(() => {
if (!isClient) return undefined;
const handleFocus = () => {
browserTab.stopNotification();
@@ -67,9 +83,12 @@ export function AgentStatusBar() {
window.removeEventListener("focus", handleFocus);
browserTab.stopNotification();
};
}, []);
}, [isClient]);
// Handle agent state changes - only on client
useEffect(() => {
if (!isClient) return;
React.useEffect(() => {
if (status === WsClientProviderStatus.DISCONNECTED) {
setStatusMessage("Connecting...");
} else {
@@ -82,12 +101,12 @@ export function AgentStatusBar() {
});
// Update browser tab if window exists and is not focused
if (typeof document !== "undefined" && !document.hasFocus()) {
if (document && !document.hasFocus()) {
browserTab.startNotification(message);
}
}
}
}, [curAgentState, notify, t]);
}, [isClient, curAgentState, status, notify, t]);
return (
<div className="flex flex-col items-center">

View File

@@ -45,22 +45,54 @@ async function prepareApp() {
}
}
// Create a stable query client instance
export const queryClient = new QueryClient(queryClientConfig);
prepareApp().then(() =>
startTransition(() => {
hydrateRoot(
document,
<StrictMode>
<Provider store={store}>
<AuthProvider>
<QueryClientProvider client={queryClient}>
<HydratedRouter />
<PosthogInit />
</QueryClientProvider>
</AuthProvider>
</Provider>
</StrictMode>,
);
}),
// Create the app element
const App = (
<StrictMode>
<Provider store={store}>
<AuthProvider>
<QueryClientProvider client={queryClient}>
<HydratedRouter />
<PosthogInit />
</QueryClientProvider>
</AuthProvider>
</Provider>
</StrictMode>
);
// Use a module-level variable to track if hydration has been attempted
let hasHydrated = false;
// Function to hydrate the app
const hydrateApp = () => {
// Only attempt hydration once
if (hasHydrated) return;
hasHydrated = true;
// Ensure we're in a browser environment
if (typeof window === "undefined") return;
// Use startTransition to avoid blocking the main thread
startTransition(() => {
try {
// Find the root element - React Router might be looking for this specific element
const rootElement = document.getElementById("root");
if (rootElement) {
// Hydrate the root element instead of the entire document
hydrateRoot(rootElement, App);
} else {
// Fallback to document if root element is not found
hydrateRoot(document, App);
}
} catch (error) {
// Log hydration errors but don't rethrow
// eslint-disable-next-line no-console
console.error("Error during hydration:", error);
}
});
};
// Initialize the app
prepareApp().then(hydrateApp);

View File

@@ -0,0 +1,46 @@
import { useMemo } from "react";
import { useQueryClient, useQuery } from "@tanstack/react-query";
import { StatusMessage } from "#/types/message";
const initialStatusMessage: StatusMessage = {
status_update: true,
type: "info",
id: "",
message: "",
};
export const statusMessageQueryKey = ["statusMessage"];
export function useStatusMessage() {
const queryClient = useQueryClient();
// Use a stable query function to avoid hydration mismatches
const queryFn = useMemo(
() => () =>
queryClient.getQueryData<StatusMessage>(statusMessageQueryKey) ||
initialStatusMessage,
[queryClient],
);
const { data: curStatusMessage = initialStatusMessage } = useQuery({
queryKey: statusMessageQueryKey,
queryFn,
// We don't want to refetch this data automatically
staleTime: Infinity,
gcTime: Infinity,
// Initialize with the initial status message
initialData: initialStatusMessage,
});
// Use a stable setter function
const setStatusMessage = useMemo(
() => (message: StatusMessage) =>
queryClient.setQueryData(statusMessageQueryKey, message),
[queryClient],
);
return {
curStatusMessage,
setStatusMessage,
};
}

View File

@@ -8,7 +8,6 @@ import { trackError } from "#/utils/error-handler";
import { appendSecurityAnalyzerInput } from "#/state/security-analyzer-slice";
import { setCode, setActiveFilepath } from "#/state/code-slice";
import { appendJupyterInput } from "#/state/jupyter-slice";
import { setCurStatusMessage } from "#/state/status-slice";
import { setMetrics } from "#/state/metrics-slice";
import store from "#/store";
import ActionType from "#/types/action-type";
@@ -19,6 +18,8 @@ import {
} from "#/types/message";
import { handleObservationMessage } from "./observations";
import { appendInput } from "#/state/command-slice";
import { statusMessageQueryKey } from "#/hooks/query/use-status-message";
import { queryClient } from "#/entry.client";
const messageActions = {
[ActionType.BROWSE]: (message: ActionMessage) => {
@@ -124,11 +125,9 @@ export function handleActionMessage(message: ActionMessage) {
export function handleStatusMessage(message: StatusMessage) {
if (message.type === "info") {
store.dispatch(
setCurStatusMessage({
...message,
}),
);
queryClient.setQueryData(statusMessageQueryKey, {
...message,
});
} else if (message.type === "error") {
trackError({
message: message.message,

View File

@@ -1,25 +0,0 @@
import { createSlice, PayloadAction } from "@reduxjs/toolkit";
import { StatusMessage } from "#/types/message";
const initialStatusMessage: StatusMessage = {
status_update: true,
type: "info",
id: "",
message: "",
};
export const statusSlice = createSlice({
name: "status",
initialState: {
curStatusMessage: initialStatusMessage,
},
reducers: {
setCurStatusMessage: (state, action: PayloadAction<StatusMessage>) => {
state.curStatusMessage = action.payload;
},
},
});
export const { setCurStatusMessage } = statusSlice.actions;
export default statusSlice.reducer;

View File

@@ -8,7 +8,6 @@ import initialQueryReducer from "./state/initial-query-slice";
import commandReducer from "./state/command-slice";
import { jupyterReducer } from "./state/jupyter-slice";
import securityAnalyzerReducer from "./state/security-analyzer-slice";
import statusReducer from "./state/status-slice";
import metricsReducer from "./state/metrics-slice";
export const rootReducer = combineReducers({
@@ -21,7 +20,6 @@ export const rootReducer = combineReducers({
agent: agentReducer,
jupyter: jupyterReducer,
securityAnalyzer: securityAnalyzerReducer,
status: statusReducer,
metrics: metricsReducer,
});

View File

@@ -0,0 +1,54 @@
import React, { ReactNode, useMemo } from "react";
import { createMemoryRouter, RouterProvider } from "react-router";
import { Provider } from "react-redux";
import { QueryClient, QueryClientProvider } from "@tanstack/react-query";
import store from "../store";
import { AuthProvider } from "../context/auth-context";
// Create a test router with a single route
const createTestRouter = () =>
createMemoryRouter([
{
path: "/",
element: <div>Test Route</div>,
},
]);
interface TestWrapperProps {
children: ReactNode;
}
export function TestWrapper({ children }: TestWrapperProps) {
const queryClient = useMemo(
() =>
new QueryClient({
defaultOptions: {
queries: {
retry: false,
},
},
}),
[],
);
return (
<Provider store={store}>
<AuthProvider>
<QueryClientProvider client={queryClient}>
{children}
</QueryClientProvider>
</AuthProvider>
</Provider>
);
}
export function RouterTestWrapper({ children }: TestWrapperProps) {
const router = createTestRouter();
return (
<TestWrapper>
<RouterProvider router={router} />
{children}
</TestWrapper>
);
}

View File

@@ -2,6 +2,7 @@ import { afterAll, afterEach, beforeAll, vi } from "vitest";
import { cleanup } from "@testing-library/react";
import { server } from "#/mocks/node";
import "@testing-library/jest-dom/vitest";
import React from "react";
HTMLCanvasElement.prototype.getContext = vi.fn();
HTMLElement.prototype.scrollTo = vi.fn();
@@ -18,6 +19,26 @@ vi.mock("react-i18next", async (importOriginal) => ({
}),
}));
// Mock the HydratedRouter component from react-router/dom
vi.mock("react-router/dom", async (importOriginal) => {
const actual = await importOriginal<typeof import("react-router/dom")>();
const { createMemoryRouter, RouterProvider } = await import("react-router");
return {
...actual,
HydratedRouter: ({ children }: { children?: React.ReactNode }) => {
const router = createMemoryRouter([
{
path: "/",
element: children || null,
},
]);
return React.createElement(RouterProvider, { router });
},
};
});
// Mock requests during tests
beforeAll(() => server.listen({ onUnhandledRequest: "bypass" }));
afterEach(() => {