From 6e4ccb5b75faec81a55085a886bb99fd9879a29e Mon Sep 17 00:00:00 2001 From: Benjamin Kane Date: Wed, 30 Oct 2024 15:33:45 -0400 Subject: [PATCH 1/8] fix spotlight cleanup --- .../spotlight/src/createScrollReader.ts | 15 ++++++---- app/packages/spotlight/src/index.ts | 10 +++---- app/packages/spotlight/src/row.ts | 28 +++++++++++-------- app/packages/spotlight/src/section.ts | 11 ++++---- app/packages/spotlight/src/types.ts | 3 +- 5 files changed, 39 insertions(+), 28 deletions(-) diff --git a/app/packages/spotlight/src/createScrollReader.ts b/app/packages/spotlight/src/createScrollReader.ts index dee64107f2..d164f1b5ab 100644 --- a/app/packages/spotlight/src/createScrollReader.ts +++ b/app/packages/spotlight/src/createScrollReader.ts @@ -15,14 +15,17 @@ export default function createScrollReader( let timeout: ReturnType; let zooming = false; - element.addEventListener("scroll", () => { + const scroll = () => { scrolling = true; - }); + }; + element.addEventListener("scroll", scroll); - element.addEventListener("scrollend", () => { + const scrollEnd = () => { scrolling = false; requestAnimationFrame(() => render(zooming, true)); - }); + }; + + element.addEventListener("scrollend", scrollEnd); const updateScrollStatus = () => { const threshold = getScrollSpeedThreshold(); @@ -68,7 +71,9 @@ export default function createScrollReader( return { destroy: () => { destroyed = true; + element.removeEventListener("scroll", scroll); + element.removeEventListener("scrollend", scrollEnd); }, - zooming: () => zooming + zooming: () => zooming, }; } diff --git a/app/packages/spotlight/src/index.ts b/app/packages/spotlight/src/index.ts index 689d69e8b1..8a7c6fe155 100644 --- a/app/packages/spotlight/src/index.ts +++ b/app/packages/spotlight/src/index.ts @@ -18,7 +18,7 @@ import { SCROLLBAR_WIDTH, TWO, ZERO, - ZOOMING_COEFFICIENT + ZOOMING_COEFFICIENT, } from "./constants"; import createScrollReader from "./createScrollReader"; import { Load, RowChange } from "./events"; @@ -107,8 +107,8 @@ export default class Spotlight extends EventTarget { throw new Error("spotlight is not attached"); } - this.#backward?.remove(); - this.#forward?.remove(); + this.#backward?.destroy(); + this.#forward?.destroy(); this.#element?.classList.remove(styles.spotlightLoaded); this.#element?.remove(); this.#scrollReader?.destroy(); @@ -203,7 +203,7 @@ export default class Spotlight extends EventTarget { const backward = this.#forward; this.#forward = section; this.#forward.attach(this.#element); - this.#backward.remove(); + this.#backward.destroy(); this.#backward = backward; offset = before - this.#containerHeight + this.#config.spacing; } @@ -271,7 +271,7 @@ export default class Spotlight extends EventTarget { this.#backward = result.section; this.#backward.attach(this.#element); - this.#forward.remove(); + this.#forward.destroy(); this.#forward = forward; } diff --git a/app/packages/spotlight/src/row.ts b/app/packages/spotlight/src/row.ts index d904e86eb0..f420de2049 100644 --- a/app/packages/spotlight/src/row.ts +++ b/app/packages/spotlight/src/row.ts @@ -13,6 +13,7 @@ export default class Row { #from: number; #hidden: boolean; + readonly #aborter: AbortController = new AbortController(); readonly #config: SpotlightConfig; readonly #dangle?: boolean; readonly #container: HTMLDivElement = create(DIV); @@ -47,7 +48,7 @@ export default class Row { element.style.top = pixels(ZERO); if (config.onItemClick) { - element.addEventListener("click", (event) => { + const handler = (event) => { if (event.metaKey || event.shiftKey) { return; } @@ -59,18 +60,13 @@ export default class Row { item, iter, }); + }; + + element.addEventListener("click", handler, { + signal: this.#aborter.signal, }); - element.addEventListener("contextmenu", (event) => { - if (event.metaKey || event.shiftKey) { - return; - } - event.preventDefault(); - focus(item.id); - config.onItemClick({ - event, - item, - iter, - }); + element.addEventListener("contextmenu", handler, { + signal: this.#aborter.signal, }); } @@ -123,6 +119,11 @@ export default class Row { return this.#row[this.#row.length - ONE].item.id; } + destroy() { + for (const item of this.#row) this.#config.destroy(item.item.id); + this.#aborter.abort(); + } + has(item: string) { for (const i of this.#row) { if (i.item.id.description === item) { @@ -138,6 +139,9 @@ export default class Row { } this.#container.remove(); + for (const { item } of this.#row) { + this.#config.destroy(item.id); + } } show( diff --git a/app/packages/spotlight/src/section.ts b/app/packages/spotlight/src/section.ts index c495aa3dc3..cd0d39b2b3 100644 --- a/app/packages/spotlight/src/section.ts +++ b/app/packages/spotlight/src/section.ts @@ -108,6 +108,12 @@ export default class Section { : element.appendChild(this.#section); } + destroy() { + this.#section.remove(); + for (const row of this.#rows) row.destroy(); + this.#rows = []; + } + find(item: string): Row | null { for (const row of this.#rows) { if (row.has(item)) { @@ -118,11 +124,6 @@ export default class Section { return null; } - remove() { - this.#section.remove(); - this.#rows = []; - } - render({ config, target, diff --git a/app/packages/spotlight/src/types.ts b/app/packages/spotlight/src/types.ts index 7954fd0bb3..a613eb2632 100644 --- a/app/packages/spotlight/src/types.ts +++ b/app/packages/spotlight/src/types.ts @@ -56,8 +56,9 @@ export type Request = (key: K) => Promise<{ }>; export interface SpotlightConfig { - get: Get; at?: At; + destroy?: (id: ID) => void; + get: Get; key: K; offset?: number; onItemClick?: ItemClick; From 45e10dd91ec772abe660a201148367929a7d1065 Mon Sep 17 00:00:00 2001 From: Benjamin Kane Date: Thu, 17 Oct 2024 16:50:56 -0400 Subject: [PATCH 2/8] remove modal navigation from router transactions --- app/packages/app/src/routing/RouterContext.ts | 203 +++++++++++------- .../core/src/components/Grid/useRefreshers.ts | 9 +- .../state/src/recoil/sidebarExpanded.ts | 14 -- 3 files changed, 123 insertions(+), 103 deletions(-) diff --git a/app/packages/app/src/routing/RouterContext.ts b/app/packages/app/src/routing/RouterContext.ts index e684acaf8d..d33b0623b4 100644 --- a/app/packages/app/src/routing/RouterContext.ts +++ b/app/packages/app/src/routing/RouterContext.ts @@ -69,6 +69,8 @@ export const createRouter = ( ): Router => { const history = isNotebook() ? createMemoryHistory() : createBrowserHistory(); + const getEntryResource = makeGetEntryResource(); + let currentEntryResource: Resource>; let nextCurrentEntryResource: Resource>; @@ -79,17 +81,22 @@ export const createRouter = ( >(); const update = (location: FiftyOneLocation, action?: Action) => { - requestAnimationFrame(() => { - for (const [_, [__, onPending]] of subscribers) onPending?.(); - }); currentEntryResource.load().then(({ cleanup }) => { - nextCurrentEntryResource = getEntryResource( - environment, - routes, - location as FiftyOneLocation, - false, - handleError - ); + try { + nextCurrentEntryResource = getEntryResource({ + environment, + routes, + location: location as FiftyOneLocation, + hard: false, + handleError, + }); + } catch { + return; + } + + requestAnimationFrame(() => { + for (const [_, [__, onPending]] of subscribers) onPending?.(); + }); const loadingResource = nextCurrentEntryResource; loadingResource.load().then((entry) => { @@ -135,13 +142,13 @@ export const createRouter = ( load(hard = false) { const runUpdate = !currentEntryResource || hard; if (!currentEntryResource || hard) { - currentEntryResource = getEntryResource( + currentEntryResource = getEntryResource({ environment, - routes, - history.location as FiftyOneLocation, hard, - handleError - ); + handleError, + location: history.location as FiftyOneLocation, + routes, + }); } runUpdate && update(history.location as FiftyOneLocation); return currentEntryResource.load(); @@ -171,79 +178,111 @@ export const createRouter = ( }; }; -const getEntryResource = ( - environment: Environment, - routes: RouteDefinition[], - location: FiftyOneLocation, - hard = false, - handleError?: (error: unknown) => void -): Resource> => { - let route: RouteDefinition; - let matchResult: MatchPathResult | undefined = undefined; - for (let index = 0; index < routes.length; index++) { - route = routes[index]; - const match = matchPath( - location.pathname, - route, - location.search, - location.state - ); - - if (match) { - matchResult = match; - break; +const makeGetEntryResource = () => { + let currentLocation: FiftyOneLocation; + let currentResource: Resource>; + + const isReusable = (location: FiftyOneLocation) => { + if (currentLocation) { + return ( + location.state.event === "modal" || + currentLocation?.state.event === "modal" + ); } - } - if (matchResult == null) { - throw new NotFoundError({ path: location.pathname }); - } + return false; + }; - const fetchPolicy = hard ? "network-only" : "store-or-network"; + const getEntryResource = ({ + environment, + handleError, + hard = false, + location, + routes, + }: { + current?: FiftyOneLocation; + environment: Environment; + routes: RouteDefinition[]; + location: FiftyOneLocation; + hard: boolean; + handleError?: (error: unknown) => void; + }): Resource> => { + if (isReusable(location)) { + throw currentResource; + } - return new Resource(() => { - return Promise.all([route.component.load(), route.query.load()]).then( - ([component, concreteRequest]) => { - const preloadedQuery = loadQuery( - environment, - concreteRequest, - matchResult.variables || {}, - { - fetchPolicy, - } - ); - - let resolveEntry: (entry: Entry) => void; - const promise = new Promise>((resolve) => { - resolveEntry = resolve; - }); - const subscription = fetchQuery( - environment, - concreteRequest, - matchResult.variables || {}, - { fetchPolicy } - ).subscribe({ - next: (data) => { - const { state: _, ...rest } = location; - resolveEntry({ - state: matchResult.variables as LocationState, - ...rest, - component, - data, - concreteRequest, - preloadedQuery, - cleanup: () => { - subscription?.unsubscribe(); - }, - }); - }, - error: (error) => handleError?.(error), - }); + let route: RouteDefinition; + let matchResult: MatchPathResult | undefined = undefined; + for (let index = 0; index < routes.length; index++) { + route = routes[index]; + const match = matchPath( + location.pathname, + route, + location.search, + location.state + ); - return promise; + if (match) { + matchResult = match; + break; } - ); - }); + } + + if (matchResult == null) { + throw new NotFoundError({ path: location.pathname }); + } + + const fetchPolicy = hard ? "network-only" : "store-or-network"; + + currentLocation = location; + currentResource = new Resource(() => { + return Promise.all([route.component.load(), route.query.load()]).then( + ([component, concreteRequest]) => { + const preloadedQuery = loadQuery( + environment, + concreteRequest, + matchResult.variables || {}, + { + fetchPolicy, + } + ); + + let resolveEntry: (entry: Entry) => void; + const promise = new Promise>((resolve) => { + resolveEntry = resolve; + }); + const subscription = fetchQuery( + environment, + concreteRequest, + matchResult.variables || {}, + { fetchPolicy } + ).subscribe({ + next: (data) => { + const { state: _, ...rest } = location; + resolveEntry({ + state: matchResult.variables as LocationState, + ...rest, + component, + data, + concreteRequest, + preloadedQuery, + cleanup: () => { + subscription?.unsubscribe(); + }, + }); + }, + error: (error) => handleError?.(error), + }); + + return promise; + } + ); + }); + + return currentResource; + }; + + return getEntryResource; }; export const RouterContext = React.createContext< diff --git a/app/packages/core/src/components/Grid/useRefreshers.ts b/app/packages/core/src/components/Grid/useRefreshers.ts index f811d36b1a..10922ab09f 100644 --- a/app/packages/core/src/components/Grid/useRefreshers.ts +++ b/app/packages/core/src/components/Grid/useRefreshers.ts @@ -63,13 +63,8 @@ export default function useRefreshers() { useEffect( () => - subscribe(({ event }, { reset }, previous) => { - if ( - event === "fieldVisibility" || - event === "modal" || - previous?.event === "modal" - ) - return; + subscribe(({ event }, { reset }) => { + if (event === "fieldVisibility") return; // if not a modal page change, reset the grid location reset(gridAt); diff --git a/app/packages/state/src/recoil/sidebarExpanded.ts b/app/packages/state/src/recoil/sidebarExpanded.ts index 2c83c5f8cd..2e3044e9d3 100644 --- a/app/packages/state/src/recoil/sidebarExpanded.ts +++ b/app/packages/state/src/recoil/sidebarExpanded.ts @@ -1,4 +1,3 @@ -import { subscribe } from "@fiftyone/relay"; import { DefaultValue, atom, atomFamily, selectorFamily } from "recoil"; export const sidebarExpandedStore = atomFamily< @@ -7,12 +6,6 @@ export const sidebarExpandedStore = atomFamily< >({ key: "sidebarExpandedStore", default: {}, - effects: [ - ({ node }) => - subscribe(({ event }, { reset }, previous) => { - event !== "modal" && previous?.event !== "modal" && reset(node); - }), - ], }); export const sidebarExpanded = selectorFamily< @@ -36,13 +29,6 @@ export const sidebarExpanded = selectorFamily< export const granularSidebarExpandedStore = atom<{ [key: string]: boolean }>({ key: "granularSidebarExpandedStore", default: {}, - effects: [ - ({ node }) => - subscribe( - ({ event }, { set }, previous) => - event !== "modal" && previous?.event !== "modal" && set(node, {}) - ), - ], }); export const granularSidebarExpanded = selectorFamily({ From 3d34c24a13f437c7c3291599ed61c0891bb99848 Mon Sep 17 00:00:00 2001 From: Benjamin Kane Date: Thu, 17 Oct 2024 16:51:15 -0400 Subject: [PATCH 3/8] cache gathered paths --- .../state/src/recoil/pathData/counts.ts | 17 ++++++++++++++++- 1 file changed, 16 insertions(+), 1 deletion(-) diff --git a/app/packages/state/src/recoil/pathData/counts.ts b/app/packages/state/src/recoil/pathData/counts.ts index 6059375fcd..1de0e8d8eb 100644 --- a/app/packages/state/src/recoil/pathData/counts.ts +++ b/app/packages/state/src/recoil/pathData/counts.ts @@ -146,6 +146,21 @@ export const counts = selectorFamily({ }, }); +export const gatheredPaths = selectorFamily({ + key: "gatheredPaths", + get: + ({ + embeddedDocType, + ftype, + }: { + embeddedDocType?: string | string[]; + ftype: string | string[]; + }) => + ({ get }) => { + return [...new Set(gatherPaths(get, ftype, embeddedDocType))]; + }, +}); + export const cumulativeCounts = selectorFamily< { [key: string]: number }, { @@ -160,7 +175,7 @@ export const cumulativeCounts = selectorFamily< get: ({ extended, path: key, modal, ftype, embeddedDocType }) => ({ get }) => { - return [...new Set(gatherPaths(get, ftype, embeddedDocType))].reduce( + return get(gatheredPaths({ ftype, embeddedDocType })).reduce( (result, path) => { const data = get(counts({ extended, modal, path: `${path}.${key}` })); for (const value in data) { From 6d10c4fad33ac686b0ab3d064c3f266ee9921b00 Mon Sep 17 00:00:00 2001 From: Benjamin Kane Date: Mon, 21 Oct 2024 09:44:57 -0400 Subject: [PATCH 4/8] e2e updates --- app/packages/app/src/routing/RouterContext.ts | 6 ++++-- app/packages/app/src/routing/matchPath.ts | 2 +- app/packages/app/src/useEvents/useSetGroupSlice.ts | 6 +++++- app/packages/app/src/useWriters/onSetGroupSlice.ts | 6 +++++- e2e-pw/src/oss/poms/grid/index.ts | 8 +++----- e2e-pw/src/oss/poms/modal/index.ts | 14 +++++--------- 6 files changed, 23 insertions(+), 19 deletions(-) diff --git a/app/packages/app/src/routing/RouterContext.ts b/app/packages/app/src/routing/RouterContext.ts index d33b0623b4..6f25d6d46f 100644 --- a/app/packages/app/src/routing/RouterContext.ts +++ b/app/packages/app/src/routing/RouterContext.ts @@ -178,6 +178,8 @@ export const createRouter = ( }; }; +const SKIP_EVENTS = new Set(["modal", "slice"]); + const makeGetEntryResource = () => { let currentLocation: FiftyOneLocation; let currentResource: Resource>; @@ -185,8 +187,8 @@ const makeGetEntryResource = () => { const isReusable = (location: FiftyOneLocation) => { if (currentLocation) { return ( - location.state.event === "modal" || - currentLocation?.state.event === "modal" + SKIP_EVENTS.has(location.state.event || "") || + SKIP_EVENTS.has(currentLocation?.state.event || "") ); } diff --git a/app/packages/app/src/routing/matchPath.ts b/app/packages/app/src/routing/matchPath.ts index b8c7533359..1223647081 100644 --- a/app/packages/app/src/routing/matchPath.ts +++ b/app/packages/app/src/routing/matchPath.ts @@ -10,7 +10,7 @@ const compilePath = (path: string) => }); export type LocationState = { - event?: "modal"; + event?: "modal" | "slice"; fieldVisibility?: State.FieldVisibilityStage; groupSlice?: string; modalSelector?: ModalSelector; diff --git a/app/packages/app/src/useEvents/useSetGroupSlice.ts b/app/packages/app/src/useEvents/useSetGroupSlice.ts index 9ef152cea5..a58ba62754 100644 --- a/app/packages/app/src/useEvents/useSetGroupSlice.ts +++ b/app/packages/app/src/useEvents/useSetGroupSlice.ts @@ -15,7 +15,11 @@ const useSetGroupSlice: EventHandlerHook = ({ router, session }) => { session.current.sessionGroupSlice = slice; }); - router.push(pathname, { ...router.location.state, groupSlice: slice }); + router.push(pathname, { + ...router.location.state, + event: "slice", + groupSlice: slice, + }); }, [router, session] ); diff --git a/app/packages/app/src/useWriters/onSetGroupSlice.ts b/app/packages/app/src/useWriters/onSetGroupSlice.ts index f2a58d37ba..4045cac469 100644 --- a/app/packages/app/src/useWriters/onSetGroupSlice.ts +++ b/app/packages/app/src/useWriters/onSetGroupSlice.ts @@ -13,7 +13,11 @@ const onSetGroupSlice: RegisteredWriter<"sessionGroupSlice"> = const pathname = router.history.location.pathname + string; - router.push(pathname, { ...router.location.state, groupSlice: slice }); + router.push(pathname, { + ...router.location.state, + event: "slice", + groupSlice: slice, + }); if (env().VITE_NO_STATE) return; commitMutation(environment, { diff --git a/e2e-pw/src/oss/poms/grid/index.ts b/e2e-pw/src/oss/poms/grid/index.ts index daa42cd6f0..b05ab5e870 100644 --- a/e2e-pw/src/oss/poms/grid/index.ts +++ b/e2e-pw/src/oss/poms/grid/index.ts @@ -1,4 +1,4 @@ -import { expect, Locator, Page } from "src/oss/fixtures"; +import { Locator, Page, expect } from "src/oss/fixtures"; import { EventUtils } from "src/shared/event-utils"; import { GridActionsRowPom } from "../action-row/grid-actions-row"; import { GridSliceSelectorPom } from "../action-row/grid-slice-selector"; @@ -52,9 +52,7 @@ export class GridPom { } async openNthSample(n: number) { - await this.url.pageChange(() => - this.getNthLooker(n).click({ position: { x: 10, y: 80 } }) - ); + await this.getNthLooker(n).click({ position: { x: 10, y: 80 } }); } async openFirstSample() { @@ -80,7 +78,7 @@ export class GridPom { } async selectSlice(slice: string) { - await this.url.pageChange(() => this.sliceSelector.selectSlice(slice)); + await this.sliceSelector.selectSlice(slice); } /** diff --git a/e2e-pw/src/oss/poms/modal/index.ts b/e2e-pw/src/oss/poms/modal/index.ts index 176856107f..33a8e58cdc 100644 --- a/e2e-pw/src/oss/poms/modal/index.ts +++ b/e2e-pw/src/oss/poms/modal/index.ts @@ -1,4 +1,4 @@ -import { expect, Locator, Page } from "src/oss/fixtures"; +import { Locator, Page, expect } from "src/oss/fixtures"; import { EventUtils } from "src/shared/event-utils"; import { Duration } from "../../utils"; import { ModalTaggerPom } from "../action-row/tagger/modal-tagger"; @@ -118,11 +118,9 @@ export class ModalPom { ) { const currentSampleId = await this.sidebar.getSampleId(); - await this.url.pageChange(() => - this.locator - .getByTestId(`nav-${direction === "forward" ? "right" : "left"}-button`) - .click() - ); + await this.locator + .getByTestId(`nav-${direction === "forward" ? "right" : "left"}-button`) + .click(); // wait for sample id to change await this.page.waitForFunction((currentSampleId) => { @@ -219,9 +217,7 @@ export class ModalPom { async close() { // close by clicking outside of modal - await this.url.pageChange(() => - this.page.click("body", { position: { x: 0, y: 0 } }) - ); + await this.page.click("body", { position: { x: 0, y: 0 } }); } async navigateNextSample(allowErrorInfo = false) { From e0774c305967323a7ff9a90c2b4021dd00f395bf Mon Sep 17 00:00:00 2001 From: Benjamin Kane Date: Mon, 21 Oct 2024 11:17:47 -0400 Subject: [PATCH 5/8] update selection test --- e2e-pw/src/oss/specs/selection.spec.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/e2e-pw/src/oss/specs/selection.spec.ts b/e2e-pw/src/oss/specs/selection.spec.ts index 604b0c015b..858ab29d3f 100644 --- a/e2e-pw/src/oss/specs/selection.spec.ts +++ b/e2e-pw/src/oss/specs/selection.spec.ts @@ -95,7 +95,7 @@ extensionDatasetNamePairs.forEach(([extension, datasetName]) => { await grid.openNthSample(1); await modal.assert.verifySelectionCount(1); - await grid.url.back(); + await modal.close(); await modal.assert.isClosed(); await grid.assert.isSelectionCountEqualTo(1); }); From bd25de2b494bc26cf01f3bd46c28d34e0dd0077e Mon Sep 17 00:00:00 2001 From: Benjamin Kane Date: Tue, 22 Oct 2024 11:06:44 -0400 Subject: [PATCH 6/8] lint, add comments --- app/packages/app/src/routing/RouterContext.ts | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/app/packages/app/src/routing/RouterContext.ts b/app/packages/app/src/routing/RouterContext.ts index 6f25d6d46f..92a2114e16 100644 --- a/app/packages/app/src/routing/RouterContext.ts +++ b/app/packages/app/src/routing/RouterContext.ts @@ -90,8 +90,13 @@ export const createRouter = ( hard: false, handleError, }); - } catch { - return; + } catch (e) { + if (e instanceof Resource) { + // skip the page change if a resource is thrown + return; + } + + throw e; } requestAnimationFrame(() => { @@ -210,6 +215,7 @@ const makeGetEntryResource = () => { handleError?: (error: unknown) => void; }): Resource> => { if (isReusable(location)) { + // throw the current resource (page) if it can be reused throw currentResource; } From 3afa8c29f62655d10d8032a19bedcbb40ed41ec5 Mon Sep 17 00:00:00 2001 From: Br2850 Date: Mon, 28 Oct 2024 10:28:34 -0400 Subject: [PATCH 7/8] freeze tensorflow dep --- requirements/github.txt | 2 +- requirements/test.txt | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/requirements/github.txt b/requirements/github.txt index 64ae132326..cc56602780 100644 --- a/requirements/github.txt +++ b/requirements/github.txt @@ -4,7 +4,7 @@ numpy<2 pydicom>=2.2.0 shapely>=1.7.1 -tensorflow +tensorflow==2.17.0 tensorflow-datasets torch torchvision diff --git a/requirements/test.txt b/requirements/test.txt index 9173566d9c..5e619a4ee7 100644 --- a/requirements/test.txt +++ b/requirements/test.txt @@ -7,5 +7,5 @@ pytest-cov==4.0.0 pytest-mock==3.10.0 pytest-asyncio shapely -tensorflow +tensorflow==2.17.0 twine>=3 From fbc57ebc16d4926ca83470bfe874e7596fe322a5 Mon Sep 17 00:00:00 2001 From: Benjamin Kane Date: Thu, 31 Oct 2024 12:36:52 -0400 Subject: [PATCH 8/8] add destroy items --- app/packages/spotlight/src/row.ts | 26 ++++++++++++++++++++++---- 1 file changed, 22 insertions(+), 4 deletions(-) diff --git a/app/packages/spotlight/src/row.ts b/app/packages/spotlight/src/row.ts index f420de2049..7d4242bec9 100644 --- a/app/packages/spotlight/src/row.ts +++ b/app/packages/spotlight/src/row.ts @@ -120,7 +120,7 @@ export default class Row { } destroy() { - for (const item of this.#row) this.#config.destroy(item.item.id); + this.#destroyItems(); this.#aborter.abort(); } @@ -139,9 +139,7 @@ export default class Row { } this.#container.remove(); - for (const { item } of this.#row) { - this.#config.destroy(item.id); - } + this.#destroyItems(); } show( @@ -229,4 +227,24 @@ export default class Row { const set = new Set(this.#row.map(({ item }) => item.aspectRatio)); return set.size === ONE ? this.#row[ZERO].item.aspectRatio : null; } + + #destroyItems() { + const destroy = this.#config.destroy; + if (!destroy) { + return; + } + + const errors = []; + for (const item of this.#row) { + try { + destroy(item.item.id); + } catch (e) { + errors.push(e); + } + } + + if (errors.length > 0) { + console.error("Errors occurred during row destruction:", errors); + } + } }