Skip to content

Commit

Permalink
Reduce dependence on Recoil and useEntities() (#770)
Browse files Browse the repository at this point in the history
* Use hooks for clearing graph and removing entities

* Add useExplorer() hook

* Get node expansion limit from explorer connection

* Simplify useAllNeighbors()

* Refactor useAddToGraph() hook

* Add all refactoring to changelog
  • Loading branch information
kmcginnes authored Jan 29, 2025
1 parent e9809e9 commit 1c30d87
Show file tree
Hide file tree
Showing 22 changed files with 212 additions and 173 deletions.
9 changes: 9 additions & 0 deletions Changelog.md
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,15 @@
([#743](https://github.com/aws/graph-explorer/pull/743))
- **Improved** pagination controls by using a single shared component
([#742](https://github.com/aws/graph-explorer/pull/742))
- **Updated** graph foundations to accommodate loading a graph from a set of IDs
([#756](https://github.com/aws/graph-explorer/pull/756),
[#758](https://github.com/aws/graph-explorer/pull/758),
[#761](https://github.com/aws/graph-explorer/pull/761),
[#762](https://github.com/aws/graph-explorer/pull/762),
[#767](https://github.com/aws/graph-explorer/pull/767),
[#768](https://github.com/aws/graph-explorer/pull/768),
[#769](https://github.com/aws/graph-explorer/pull/769),
[#770](https://github.com/aws/graph-explorer/pull/770))
- **Updated** dependencies
([#764](https://github.com/aws/graph-explorer/pull/764))

Expand Down
5 changes: 3 additions & 2 deletions packages/graph-explorer/src/connector/queries.ts
Original file line number Diff line number Diff line change
Expand Up @@ -54,11 +54,10 @@ export type NeighborCountsQueryResponse = {
*/
export function neighborsCountQuery(
vertexId: VertexId,
limit: number | undefined,
explorer: Explorer | null
) {
return queryOptions({
queryKey: ["neighborsCount", vertexId, limit, explorer],
queryKey: ["neighborsCount", vertexId, explorer],
enabled: Boolean(explorer),
queryFn: async (): Promise<NeighborCountsQueryResponse> => {
if (!explorer) {
Expand All @@ -69,6 +68,8 @@ export function neighborsCountQuery(
};
}

const limit = explorer.connection.nodeExpansionLimit;

const result = await explorer.fetchNeighborsCount({
vertexId,
limit,
Expand Down
16 changes: 9 additions & 7 deletions packages/graph-explorer/src/core/StateProvider/neighbors.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { VertexId } from "@/core";
import { useDisplayVerticesInCanvas, VertexId } from "@/core";
import { selectorFamily, useRecoilCallback, useRecoilValue } from "recoil";
import { edgesAtom } from "./edges";
import { nodesAtom } from "./nodes";
Expand All @@ -9,7 +9,7 @@ import {
import { useEffect, useMemo } from "react";
import { useQueryClient } from "@tanstack/react-query";
import { neighborsCountQuery } from "@/connector";
import { activeConnectionSelector, explorerSelector } from "../connector";
import { explorerSelector } from "../connector";
import { useNotification } from "@/components/NotificationProvider";

export type NeighborCounts = {
Expand Down Expand Up @@ -44,9 +44,8 @@ export function useNeighborsCallback() {
fetchedNeighborsSelector(vertexId)
);
const explorer = await snapshot.getPromise(explorerSelector);
const connection = await snapshot.getPromise(activeConnectionSelector);
const response = await queryClient.ensureQueryData(
neighborsCountQuery(vertexId, connection?.nodeExpansionLimit, explorer)
neighborsCountQuery(vertexId, explorer)
);

const neighbors = calculateNeighbors(
Expand Down Expand Up @@ -97,11 +96,14 @@ export function useNeighborByType(vertexId: VertexId, type: string) {
return neighbors;
}

export function useAllNeighbors(vertices: VertexId[]) {
export function useAllNeighbors() {
const vertices = useDisplayVerticesInCanvas();
const vertexIds = useMemo(() => vertices.keys().toArray(), [vertices]);

const fetchedNeighbors = useRecoilValue(
allFetchedNeighborsSelector(vertices)
allFetchedNeighborsSelector(vertexIds)
);
const query = useAllNeighborCountsQuery(vertices);
const query = useAllNeighborCountsQuery(vertexIds);

const { enqueueNotification, clearNotification } = useNotification();

Expand Down
10 changes: 9 additions & 1 deletion packages/graph-explorer/src/core/connector.ts
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ import { Explorer } from "@/connector/useGEFetchTypes";
* Active connection where the value will only change when one of the
* properties we care about are changed.
*/
export const activeConnectionSelector = equalSelector({
const activeConnectionSelector = equalSelector({
key: "activeConnection",
get: ({ get }) => {
const config = get(mergedConfigurationSelector);
Expand Down Expand Up @@ -75,6 +75,14 @@ export const explorerSelector = selector({
},
});

export function useExplorer() {
const explorer = useRecoilValue(explorerSelector);
if (!explorer) {
throw new Error("No explorer found");
}
return explorer;
}

/** CAUTION: This atom is only for testing purposes. */
export const explorerForTestingAtom = atom<Explorer | null>({
key: "explorerForTesting",
Expand Down
18 changes: 9 additions & 9 deletions packages/graph-explorer/src/hooks/useAddToGraph.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,14 +13,14 @@ import { nodesAtom, toNodeMap } from "@/core/StateProvider/nodes";
test("should add one node", async () => {
const vertex = createRandomVertex();
const { result } = renderHookWithRecoilRoot(() => {
const callback = useAddToGraph(vertex);
const callback = useAddToGraph();
const [entities] = useEntities();

return { callback, entities };
});

await act(async () => {
result.current.callback();
result.current.callback({ vertices: [vertex] });
});

const actual = result.current.entities.nodes.get(vertex.id);
Expand All @@ -34,7 +34,7 @@ test("should add one edge", async () => {

const { result } = renderHookWithRecoilRoot(
() => {
const callback = useAddToGraph(edge);
const callback = useAddToGraph();
const [entities] = useEntities();

return { callback, entities };
Expand All @@ -45,7 +45,7 @@ test("should add one edge", async () => {
);

await act(async () => {
result.current.callback();
result.current.callback({ edges: [edge] });
});

const actual = result.current.entities.edges.get(edge.id);
Expand All @@ -55,17 +55,17 @@ test("should add one edge", async () => {
test("should add multiple nodes and edges", async () => {
const randomEntities = createRandomEntities();
const { result } = renderHookWithRecoilRoot(() => {
const callback = useAddToGraph(
...randomEntities.nodes.values(),
...randomEntities.edges.values()
);
const callback = useAddToGraph();
const [entities] = useEntities();

return { callback, entities };
});

await act(async () => {
result.current.callback();
result.current.callback({
vertices: [...randomEntities.nodes.values()],
edges: [...randomEntities.edges.values()],
});
});

const actualNodes = result.current.entities.nodes.values().toArray();
Expand Down
36 changes: 24 additions & 12 deletions packages/graph-explorer/src/hooks/useAddToGraph.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,20 +6,32 @@ import { useCallback } from "react";
import { useSetRecoilState } from "recoil";

/** Returns a callback that adds an array of nodes and edges to the graph. */
export function useAddToGraph(...entitiesToAdd: (Vertex | Edge)[]) {
export function useAddToGraph() {
const setEntities = useSetRecoilState(entitiesSelector);

return useCallback(() => {
const nodes = entitiesToAdd.filter(e => e.entityType === "vertex");
const edges = entitiesToAdd.filter(e => e.entityType === "edge");
return useCallback(
(entities: { vertices?: Vertex[]; edges?: Edge[] }) => {
const vertices = entities.vertices ?? [];
const edges = entities.edges ?? [];

if (nodes.length === 0 && edges.length === 0) {
return;
}
if (vertices.length === 0 && edges.length === 0) {
return;
}

setEntities({
nodes: toNodeMap(nodes),
edges: toEdgeMap(edges),
});
}, [entitiesToAdd, setEntities]);
setEntities({
nodes: toNodeMap(vertices),
edges: toEdgeMap(edges),
});
},
[setEntities]
);
}

/** Returns a callback the given vertex to the graph. */
export function useAddVertexToGraph(vertex: Vertex) {
const callback = useAddToGraph();
return useCallback(
() => callback({ vertices: [vertex] }),
[callback, vertex]
);
}
5 changes: 2 additions & 3 deletions packages/graph-explorer/src/hooks/useEdgeDetailsQuery.ts
Original file line number Diff line number Diff line change
@@ -1,10 +1,9 @@
import { edgeDetailsQuery } from "@/connector";
import { EdgeId, explorerSelector } from "@/core";
import { EdgeId, useExplorer } from "@/core";
import { useQuery } from "@tanstack/react-query";
import { useRecoilValue } from "recoil";

export function useEdgeDetailsQuery(edgeId: EdgeId) {
const explorer = useRecoilValue(explorerSelector);
const explorer = useExplorer();
const query = useQuery(edgeDetailsQuery({ edgeId }, explorer));
return query;
}
51 changes: 23 additions & 28 deletions packages/graph-explorer/src/hooks/useExpandNode.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -6,19 +6,13 @@ import {
type NeighborsRequest,
type NeighborsResponse,
} from "@/connector";
import {
activeConnectionSelector,
explorerSelector,
loggerSelector,
} from "@/core/connector";
import useEntities from "./useEntities";
import { loggerSelector, useExplorer } from "@/core/connector";
import { useRecoilValue } from "recoil";
import { useMutation, useQueryClient } from "@tanstack/react-query";
import { Vertex } from "@/core";
import { createDisplayError } from "@/utils/createDisplayError";
import { toNodeMap } from "@/core/StateProvider/nodes";
import { toEdgeMap } from "@/core/StateProvider/edges";
import { useNeighborsCallback } from "@/core";
import { useAddToGraph } from "./useAddToGraph";

export type ExpandNodeFilters = Omit<
NeighborsRequest,
Expand All @@ -36,22 +30,38 @@ export type ExpandNodeRequest = {
*/
export default function useExpandNode() {
const queryClient = useQueryClient();
const explorer = useRecoilValue(explorerSelector);
const [_, setEntities] = useEntities();
const explorer = useExplorer();
const addToGraph = useAddToGraph();
const { enqueueNotification, clearNotification } = useNotification();
const remoteLogger = useRecoilValue(loggerSelector);

const { isPending, mutate } = useMutation({
mutationFn: async (
expandNodeRequest: ExpandNodeRequest
): Promise<NeighborsResponse | null> => {
// Calculate the expansion limit based on the connection limit and the request limit
const limit = (() => {
if (!explorer.connection.nodeExpansionLimit) {
return expandNodeRequest.filters?.limit;
}
if (!expandNodeRequest.filters?.limit) {
return explorer.connection.nodeExpansionLimit;
}
// If both exists then use the smaller of the two
return Math.min(
explorer.connection.nodeExpansionLimit,
expandNodeRequest.filters.limit
);
})();

// Perform the query when a request exists
const request: NeighborsRequest | null = expandNodeRequest && {
vertexId: expandNodeRequest.vertex.id,
vertexType:
expandNodeRequest.vertex.types?.join("::") ??
expandNodeRequest.vertex.type,
...expandNodeRequest.filters,
limit,
};

if (!explorer || !request) {
Expand All @@ -70,10 +80,7 @@ export default function useExpandNode() {
updateEdgeDetailsCache(explorer, queryClient, data.edges);

// Update nodes and edges in the graph
setEntities({
nodes: toNodeMap(data.vertices),
edges: toEdgeMap(data.edges),
});
addToGraph(data);
},
onError: error => {
remoteLogger.error(`Failed to expand node: ${error.message}`);
Expand All @@ -92,10 +99,8 @@ export default function useExpandNode() {
if (!isPending) {
return;
}
// const displayName = getDisplayNames(expandNodeRequest.vertex);
const notificationId = enqueueNotification({
title: "Expanding Node",
// message: `Expanding the node ${displayName.name}`,
message: "Expanding neighbors for the given node.",
stackable: true,
});
Expand All @@ -104,7 +109,6 @@ export default function useExpandNode() {
}, [clearNotification, enqueueNotification, isPending]);

// Build the expand node callback
const connection = useRecoilValue(activeConnectionSelector);
const neighborCallback = useNeighborsCallback();
const expandNode = useCallback(
async (vertex: Vertex, filters?: ExpandNodeFilters) => {
Expand All @@ -118,10 +122,7 @@ export default function useExpandNode() {
}
const request: ExpandNodeRequest = {
vertex,
filters: {
...filters,
limit: filters?.limit || connection?.nodeExpansionLimit,
},
filters,
};

// Only allow expansion if we are not busy with another expansion
Expand All @@ -140,13 +141,7 @@ export default function useExpandNode() {

mutate(request);
},
[
connection?.nodeExpansionLimit,
enqueueNotification,
isPending,
mutate,
neighborCallback,
]
[enqueueNotification, isPending, mutate, neighborCallback]
);

return {
Expand Down
Loading

0 comments on commit 1c30d87

Please sign in to comment.