Merge pull request #3907 from voxel51/release/v0.23.1

Release/v0.23.1

app/packages/app/src/pages/datasets/DatasetPage.tsx

@@ -6,7 +6,6 @@ import { OperatorCore } from "@fiftyone/operators";
 import "@fiftyone/relay";
 import * as fos from "@fiftyone/state";
 import { datasetQueryContext } from "@fiftyone/state";
-import { NotFoundError } from "@fiftyone/utilities";
 import React, { useEffect } from "react";
 import { usePreloadedQuery } from "react-relay";
 import { useRecoilValue } from "recoil";
@@ -110,10 +109,6 @@ const DatasetPage: Route<DatasetPageQuery> = ({ prepared }) => {
       ?.classList.toggle("modalon", isModalActive);
   }, [isModalActive]);
 
-  if (!data.dataset?.name) {
-    throw new NotFoundError({ path: `/datasets/${prepared.variables.name}` });
-  }
-
   return (
     <>
       <Nav fragment={data} hasDataset={!isEmpty} />

app/packages/app/src/routing/RouterContext.ts

@@ -66,7 +66,8 @@ export interface Router<T extends OperationType> {
 
 export const createRouter = <T extends OperationType>(
   environment: Environment,
-  routes: RouteDefinition<T>[]
+  routes: RouteDefinition<T>[],
+  handleError?: (error: unknown) => void
 ): Router<T> => {
   const history =
     isElectron() || isNotebook()
@@ -90,7 +91,9 @@ export const createRouter = <T extends OperationType>(
       nextCurrentEntryResource = getEntryResource<T>(
         environment,
         routes,
-        location as FiftyOneLocation
+        location as FiftyOneLocation,
+        false,
+        handleError
       );
 
       const loadingResource = nextCurrentEntryResource;
@@ -120,7 +123,8 @@ export const createRouter = <T extends OperationType>(
           environment,
           routes,
           history.location as FiftyOneLocation,
-          hard
+          hard,
+          handleError
         );
       }
       runUpdate && update(history.location as FiftyOneLocation);
@@ -156,7 +160,8 @@ const getEntryResource = <T extends OperationType>(
   environment: Environment,
   routes: RouteDefinition<T>[],
   location: FiftyOneLocation,
-  hard = false
+  hard = false,
+  handleError?: (error: unknown) => void
 ): Resource<Entry<T>> => {
   let route: RouteDefinition<T>;
   let matchResult: MatchPathResult<T>;
@@ -194,10 +199,8 @@ const getEntryResource = <T extends OperationType>(
         );
 
         let resolveEntry: (entry: Entry<T>) => void;
-        let rejectEntry: (reason?: any) => void;
         const promise = new Promise<Entry<T>>((resolve, reject) => {
           resolveEntry = resolve;
-          rejectEntry = reject;
         });
         const subscription = fetchQuery(
           environment,
@@ -219,8 +222,7 @@ const getEntryResource = <T extends OperationType>(
               },
             });
           },
-
-          error: (error) => rejectEntry(error),
+          error: (error) => handleError(error),
         });
 
         return promise;

app/packages/app/src/routing/useRouter.ts

@@ -1,6 +1,7 @@
 import { getEnvironment, setCurrentEnvironment } from "@fiftyone/state";
 import { useMemo, useRef } from "react";
 
+import { useErrorHandler } from "react-error-boundary";
 import { createRouter, Router } from ".";
 import makeRoutes, { Queries } from "../makeRoutes";
 
@@ -9,11 +10,12 @@ setCurrentEnvironment(environment);
 
 const useRouter = () => {
   const router = useRef<Router<Queries>>();
+  const handleError = useErrorHandler();
 
   router.current = useMemo(() => {
     router.current && router.current.cleanup();
 
-    return createRouter<Queries>(environment, makeRoutes());
+    return createRouter<Queries>(environment, makeRoutes(), handleError);
   }, []);
 
   return { context: router.current.context, environment };

app/packages/app/src/useEvents/useSetFieldVisibilityStage.ts

@@ -24,10 +24,16 @@ const useSetFieldVisibilityStage: EventHandlerHook = () => {
           unsubscribe();
         });
 
-        router.history.replace(resolveURL(router), {
-          ...router.get().state,
-          fieldVisibility: stage,
-        });
+        router.history.replace(
+          resolveURL({
+            currentPathname: router.history.location.pathname,
+            currentSearch: router.history.location.search,
+          }),
+          {
+            ...router.get().state,
+            fieldVisibility: stage,
+          }
+        );
       },
     [session, setter]
   );

app/packages/app/src/useEvents/useStateUpdate.ts

@@ -16,11 +16,16 @@ const useStateUpdate: EventHandlerHook = ({
     (payload: any) => {
       const state = processState(session.current, payload.state);
       const stateless = env().VITE_NO_STATE;
-      const path = resolveURL(
-        router,
-        stateless ? getDatasetName() : payload.state.dataset,
-        stateless ? getSavedViewName() : payload.state.saved_view_slug
-      );
+      const path = resolveURL({
+        currentPathname: router.history.location.pathname,
+        currentSearch: router.history.location.search,
+        nextDataset: stateless
+          ? getDatasetName()
+          : payload.state.dataset ?? null,
+        nextView: stateless
+          ? getSavedViewName()
+          : payload.state.saved_view_slug,
+      });
       if (readyStateRef.current !== AppReadyState.OPEN) {
         router.history.replace(path, state);
         router.load().then(() => setReadyState(AppReadyState.OPEN));

app/packages/app/src/useSetters/onSetDataset.ts

@@ -39,9 +39,16 @@ const onSetDataset: RegisteredSetter =
         entry.data.dataset?.defaultGroupSlice || undefined;
     });
 
-    router.history.push(resolveURL(router, datasetName || null), {
-      view: [],
-    });
+    router.history.push(
+      resolveURL({
+        currentPathname: router.history.location.pathname,
+        currentSearch: router.history.location.search,
+        nextDataset: datasetName || null,
+      }),
+      {
+        view: [],
+      }
+    );
   };
 
 export default onSetDataset;

app/packages/app/src/useSetters/onSetFieldVisibilityStage.ts

@@ -31,10 +31,16 @@ const onSetFieldVisibilityStage: RegisteredSetter =
     });
 
     // reload page query with new extended view
-    router.history.replace(resolveURL(router), {
-      ...router.get().state,
-      fieldVisibility: stage,
-    });
+    router.history.replace(
+      resolveURL({
+        currentPathname: router.history.location.pathname,
+        currentSearch: router.history.location.search,
+      }),
+      {
+        ...router.get().state,
+        fieldVisibility: stage,
+      }
+    );
 
     // send event as side effect
     commitMutation<setFieldVisibilityStageMutation>(environment, {

app/packages/app/src/useSetters/onSetView.ts

@@ -43,9 +43,16 @@ const onSetView: RegisteredSetter =
         sessionRef.current.selectedLabels = [];
         sessionRef.current.selectedSamples = new Set();
         sessionRef.current.fieldVisibilityStage = undefined;
-        router.history.push(resolveURL(router, dataset), {
-          view,
-        });
+        router.history.push(
+          resolveURL({
+            currentPathname: router.history.location.pathname,
+            currentSearch: router.history.location.search,
+            nextDataset: dataset,
+          }),
+          {
+            view,
+          }
+        );
       },
     });
   };

app/packages/app/src/useSetters/onSetViewName.ts

@@ -33,9 +33,17 @@ const onSetViewName: RegisteredSetter =
     sessionRef.current.selectedLabels = [];
     sessionRef.current.selectedSamples = new Set();
     sessionRef.current.fieldVisibilityStage = undefined;
-    router.history.push(resolveURL(router, dataset, slug || undefined), {
-      view: [],
-    });
+    router.history.push(
+      resolveURL({
+        currentPathname: router.history.location.pathname,
+        currentSearch: router.history.location.search,
+        nextDataset: dataset,
+        nextView: slug || undefined,
+      }),
+      {
+        view: [],
+      }
+    );
   };
 
 export default onSetViewName;

app/packages/app/src/utils.test.ts

@@ -0,0 +1,82 @@
+import { describe, expect, it } from "vitest";
+import { resolveURL } from "./utils";
+
+describe("resolves datasets", () => {
+  it("resolves to /", () => {
+    expect(
+      resolveURL({
+        currentPathname: "/datasets/my-dataset",
+        currentSearch: "",
+        nextDataset: null,
+      })
+    ).toBe("/");
+    expect(
+      resolveURL({ currentPathname: "/datasets/my-dataset", currentSearch: "" })
+    ).toBe("/datasets/my-dataset");
+  });
+});
+
+describe("resolves wih proxy", () => {
+  it("resolves to /", () => {
+    expect(
+      resolveURL({
+        currentPathname: "/datasets/my-dataset",
+        currentSearch: "?proxy=/my/proxy",
+        nextDataset: null,
+      })
+    ).toBe(`/my/proxy?proxy=${encodeURIComponent("/my/proxy")}`);
+    expect(
+      resolveURL({
+        currentPathname: "/my/proxy/datasets/my-dataset",
+        currentSearch: "?proxy=/my/proxy",
+      })
+    ).toBe(
+      `/my/proxy/datasets/my-dataset?proxy=${encodeURIComponent("/my/proxy")}`
+    );
+  });
+});
+
+describe("resolves views", () => {
+  it("throws error", () => {
+    expect(() =>
+      resolveURL({
+        currentPathname: "",
+        currentSearch: "",
+        nextDataset: null,
+        nextView: "view",
+      })
+    ).toThrowError();
+  });
+
+  it("throws error", () => {
+    expect(() =>
+      resolveURL({
+        currentPathname: "",
+        currentSearch: "",
+        nextView: "view",
+      })
+    ).toThrowError();
+  });
+
+  it("resolves with saved view", () => {
+    expect(
+      resolveURL({
+        currentPathname: "/datasets/my-dataset",
+        currentSearch: "",
+        nextDataset: "my-dataset",
+        nextView: "view",
+      })
+    ).toBe(`/datasets/my-dataset?view=view`);
+  });
+});
+
+describe("resolves current", () => {
+  it("does not change location", () => {
+    expect(
+      resolveURL({
+        currentPathname: "/datasets/my-dataset",
+        currentSearch: "?view=view",
+      })
+    ).toBe("/datasets/my-dataset?view=view");
+  });
+});

app/packages/app/src/utils.ts

@@ -1,4 +1,3 @@
-import { Router } from "./makeRoutes";
 import { matchPath } from "./routing";
 
 export const getDatasetName = (pathname?: string) => {
@@ -28,33 +27,42 @@ export const getSavedViewName = (search?: string) => {
   return null;
 };
 
-export function resolveURL(router: Router): string;
-export function resolveURL(
-  router: Router,
-  dataset: string | null,
-  view?: string
-): string;
-export function resolveURL(
-  router: Router,
-  dataset?: string | null,
-  view?: string
-): string {
-  const params = new URLSearchParams(router.history.location.search);
-
-  if (dataset === undefined) {
-    return `${window.location.pathname}${params.toString()}`;
+export function resolveURL(params: {
+  currentSearch: string;
+  currentPathname: string;
+  nextDataset?: string | null;
+  nextView?: string;
+}): string {
+  const searchParams = new URLSearchParams(params.currentSearch);
+
+  if (!params.nextDataset && params.nextView) {
+    throw new Error("a view cannot be provided without a dataset");
+  }
+
+  if (params.nextDataset) {
+    params.nextView
+      ? searchParams.set("view", encodeURIComponent(params.nextView))
+      : searchParams.delete("view");
+  }
+
+  let newSearch = searchParams.toString();
+  if (newSearch.length) {
+    newSearch = `?${newSearch}`;
   }
 
-  view ? params.set("view", encodeURIComponent(view)) : params.delete("view");
-  let search = params.toString();
-  if (search.length) {
-    search = `?${search}`;
+  if (params.nextDataset === undefined) {
+    // update search params only
+    return `${params.currentPathname}${newSearch}`;
   }
 
-  const path = decodeURIComponent(params.get("proxy") ?? "");
-  if (dataset) {
-    return `${path}/datasets/${encodeURIComponent(dataset)}${search}`;
+  const path = decodeURIComponent(searchParams.get("proxy") ?? "");
+  if (params.nextDataset === null) {
+    // go to index page
+    return `${path.length ? path : "/"}${newSearch}`;
   }
 
-  return `${path.length ? path : "/"}${search}`;
+  // go to dataset
+  return `${path}/datasets/${encodeURIComponent(
+    params.nextDataset
+  )}${newSearch}`;
 }