Keep destroy() from tearing down another active tour

packages/driver/src/context.ts

@@ -143,6 +143,9 @@ export type State = {
   __pendingWaitCancel?: () => void;
   __activeStagePosition?: StageDefinition;
   __overlaySvg?: SVGSVGElement;
+  // Marks the element this instance highlighted, so a stale destroy can
+  // unmark only its own target.
+  __instanceId?: string;
 
   __events?: {
     onKeyup: (e: KeyboardEvent) => void;

packages/driver/src/driver.ts

@@ -1,12 +1,17 @@
 import { destroyPopover, Popover } from "./popover";
 import { destroyOverlay } from "./overlay";
 import { destroyEvents, initEvents, requireRefresh } from "./events";
-import { Config, createContext, DriverHook } from "./context";
-import { destroyHighlight, highlight } from "./highlight";
+import { Config, Context, createContext, DriverHook } from "./context";
+import { destroyHighlight, highlight, releaseHighlight } from "./highlight";
 import { findReachableIndex, resolveNextHook, resolvePrevHook, resolveTourStep, shouldSkipStep } from "./step";
 import { resolveElement } from "./utils";
 import "./driver.css";
 
+// The instance that last mounted the shared overlay. A destroy on any other
+// instance must not strip that overlay.
+let mountedContext: Context | null = null;
+let nextInstanceId = 0;
+
 // Re-export the public types so they remain part of the package's type surface.
 export type { Config, DriverHook, State } from "./context";
 export type { StageDefinition } from "./stage";
@@ -52,6 +57,7 @@ export interface Driver {
 
 export function driver(options: Config = {}): Driver {
   const ctx = createContext(options);
+  const instanceId = `driver-${++nextInstanceId}`;
 
   function handleClose() {
     if (!ctx.getConfig("allowClose")) {
@@ -210,7 +216,19 @@ export function driver(options: Config = {}): Driver {
       return;
     }
 
+    // Stop a previous instance's animation from painting over this one.
+    if (mountedContext && mountedContext !== ctx) {
+      const pendingFrame = mountedContext.getState("__resizeTimeout");
+      if (pendingFrame) {
+        window.cancelAnimationFrame(pendingFrame);
+      }
+      mountedContext.setState("__transitionCallback", undefined);
+      mountedContext.setState("__resizeTimeout", undefined);
+    }
+
     ctx.setState("isInitialized", true);
+    ctx.setState("__instanceId", instanceId);
+    mountedContext = ctx;
     document.body.classList.add("driver-active", ctx.getConfig("animate") ? "driver-fade" : "driver-simple");
     if (!ctx.getConfig("allowScroll")) {
       document.body.classList.add("driver-no-scroll");
@@ -328,16 +346,23 @@ export function driver(options: Config = {}): Driver {
   }
 
   function destroy(withOnDestroyStartedHook = true) {
+    // Already torn down, or never started. Touching the document here would
+    // remove the overlay that belongs to a different instance (#504).
+    if (!ctx.getState("isInitialized")) {
+      return;
+    }
+
     const activeElement = ctx.getState("__activeElement");
     const activeStep = ctx.getState("__activeStep");
-
     const activeOnDestroyed = ctx.getState("__activeOnDestroyed");
+    const ownsPage = mountedContext === ctx;
 
     const onDestroyStarted = ctx.getConfig("onDestroyStarted");
     // `onDestroyStarted` is used to confirm the exit of tour. If we trigger
     // the hook for when user calls `destroy`, driver will get into infinite loop
-    // not causing tour to be destroyed.
-    if (withOnDestroyStartedHook && onDestroyStarted) {
+    // not causing tour to be destroyed. A superseded instance skips the hook:
+    // the visible tour is no longer this one.
+    if (ownsPage && withOnDestroyStartedHook && onDestroyStarted) {
       const isActiveDummyElement = !activeElement || activeElement?.id === "driver-dummy-element";
       onDestroyStarted(isActiveDummyElement ? undefined : activeElement, activeStep!, ctx.getHookOpts());
       return;
@@ -346,13 +371,20 @@ export function driver(options: Config = {}): Driver {
     const onDeselected = activeStep?.onDeselected || ctx.getConfig("onDeselected");
     const onDestroyed = ctx.getConfig("onDestroyed");
 
-    document.body.classList.remove("driver-active", "driver-fade", "driver-simple", "driver-no-scroll");
-    document.body.style.removeProperty("--driver-animation-duration");
+    if (ownsPage) {
+      document.body.classList.remove("driver-active", "driver-fade", "driver-simple", "driver-no-scroll");
+      document.body.style.removeProperty("--driver-animation-duration");
+      destroyHighlight();
+      if (mountedContext === ctx) {
+        mountedContext = null;
+      }
+    } else {
+      releaseHighlight(activeElement, ctx.getState("__instanceId"));
+    }
 
     cancelElementWait();
     destroyEvents(ctx);
     destroyPopover(ctx.getState("popover"));
-    destroyHighlight();
     destroyOverlay(ctx);
     ctx.resetEmitter();
 
@@ -371,7 +403,7 @@ export function driver(options: Config = {}): Driver {
       }
     }
 
-    if (activeOnDestroyed) {
+    if (ownsPage && activeOnDestroyed) {
       (activeOnDestroyed as HTMLElement).focus();
     }
   }

packages/driver/src/highlight.ts

@@ -167,19 +167,46 @@ function transferHighlight(ctx: Context, toElement: Element, toStep: DriveStep)
   toElement.setAttribute("aria-haspopup", "dialog");
   toElement.setAttribute("aria-expanded", "true");
   toElement.setAttribute("aria-controls", "driver-popover-content");
+  const instanceId = ctx.getState("__instanceId");
+  if (instanceId) {
+    toElement.setAttribute("data-driver-owner", instanceId);
+  }
+}
+
+function unmarkHighlightedElement(element: Element) {
+  const parent = element.parentElement;
+  if (parent && parent !== document.body) {
+    parent.classList.remove("driver-active-element-parent", "driver-active-element-parent-no-scroll");
+  }
+
+  element.classList.remove("driver-active-element", "driver-no-interaction");
+  element.removeAttribute("aria-haspopup");
+  element.removeAttribute("aria-expanded");
+  element.removeAttribute("aria-controls");
+  element.removeAttribute("data-driver-owner");
+}
+
+// Drops the highlight class from this instance's element only. A later tour
+// may have taken the same node; its owner id then no longer matches.
+export function releaseHighlight(element: Element | undefined, instanceId: string | undefined) {
+  if (!element || !instanceId) {
+    return;
+  }
+  if (element.getAttribute("data-driver-owner") !== instanceId) {
+    return;
+  }
+
+  if (element.id === "driver-dummy-element") {
+    element.remove();
+    return;
+  }
+
+  unmarkHighlightedElement(element);
 }
 
 export function destroyHighlight() {
   document.getElementById("driver-dummy-element")?.remove();
   document.querySelectorAll(".driver-active-element").forEach(element => {
-    const parent = element.parentElement;
-    if (parent && parent !== document.body) {
-      parent.classList.remove("driver-active-element-parent", "driver-active-element-parent-no-scroll");
-    }
-
-    element.classList.remove("driver-active-element", "driver-no-interaction");
-    element.removeAttribute("aria-haspopup");
-    element.removeAttribute("aria-expanded");
-    element.removeAttribute("aria-controls");
+    unmarkHighlightedElement(element);
   });
 }

packages/driver/tests/destroy-isolation.test.ts

@@ -0,0 +1,136 @@
+import { afterEach, describe, expect, it, vi } from "vitest";
+import { driver, type Driver } from "../src/driver";
+import { nextFrame, popoverEl, popoverTitle, useDriverHarness } from "./utils";
+
+// #504: destroy() on a stale instance must not tear down the tour that is
+// actually on the page.
+
+useDriverHarness();
+
+const extra: Driver[] = [];
+function track(d: Driver): Driver {
+  extra.push(d);
+  return d;
+}
+afterEach(() => {
+  while (extra.length) {
+    extra.pop()?.destroy();
+  }
+});
+
+function liveTour(): Driver {
+  return track(
+    driver({
+      animate: false,
+      steps: [{ element: "#intro", popover: { title: "Live tour" } }],
+    })
+  );
+}
+
+function expectTourIntact(tour: Driver) {
+  expect(tour.isActive()).toBe(true);
+  expect(popoverTitle()).toBe("Live tour");
+  expect(popoverEl()).not.toBeNull();
+  expect(document.body.classList.contains("driver-active")).toBe(true);
+  expect(document.querySelector("#intro")?.classList.contains("driver-active-element")).toBe(true);
+}
+
+describe("destroy isolation (#504)", () => {
+  it("ignores a second destroy after the instance already tore down", async () => {
+    const onDestroyed = vi.fn();
+    const demo = track(driver({ animate: false, onDestroyed }));
+    demo.highlight({ element: "#card-1", popover: { title: "Highlight" } });
+    await nextFrame();
+    demo.destroy();
+
+    const tour = liveTour();
+    tour.drive();
+    await nextFrame();
+
+    demo.destroy();
+
+    expect(demo.isActive()).toBe(false);
+    expectTourIntact(tour);
+    expect(onDestroyed).toHaveBeenCalledTimes(1);
+  });
+
+  it("does not let a still-active stale instance wipe the tour that replaced it", async () => {
+    const demoDestroyed = vi.fn();
+    const tourDestroyed = vi.fn();
+    const demo = track(
+      driver({
+        animate: false,
+        onDestroyed: demoDestroyed,
+      })
+    );
+    demo.highlight({ element: "#card-1", popover: { title: "Highlight" } });
+    await nextFrame();
+
+    const tour = track(
+      driver({
+        animate: false,
+        onDestroyed: tourDestroyed,
+        steps: [{ element: "#intro", popover: { title: "Live tour" } }],
+      })
+    );
+    tour.drive();
+    await nextFrame();
+
+    // The highlight object is still active. That is the timeout in #504:
+    // isActive() is true, so the caller does call destroy().
+    expect(demo.isActive()).toBe(true);
+    const popovers = document.querySelectorAll(".driver-popover");
+    const overlays = document.querySelectorAll(".driver-overlay");
+    const livePopover = popovers[popovers.length - 1];
+    const liveOverlay = overlays[overlays.length - 1];
+    demo.destroy();
+
+    expect(demo.isActive()).toBe(false);
+    expect(demoDestroyed).toHaveBeenCalledTimes(1);
+    expect(tourDestroyed).not.toHaveBeenCalled();
+    const remainingPopovers = document.querySelectorAll(".driver-popover");
+    const remainingOverlays = document.querySelectorAll(".driver-overlay");
+    expect(remainingPopovers[remainingPopovers.length - 1]).toBe(livePopover);
+    expect(remainingOverlays[remainingOverlays.length - 1]).toBe(liveOverlay);
+    expect(document.querySelector("#card-1")?.classList.contains("driver-active-element")).toBe(false);
+    expectTourIntact(tour);
+  });
+
+  it("leaves the live tour alone when destroy() runs on an instance that never started", () => {
+    const idle = track(driver({ animate: false }));
+    const tour = liveTour();
+    tour.drive();
+
+    idle.destroy();
+
+    expect(idle.isActive()).toBe(false);
+    expectTourIntact(tour);
+  });
+
+  it("can highlight again after destroy", () => {
+    const demo = track(driver({ animate: false }));
+    demo.highlight({ element: "#intro", popover: { title: "First" } });
+    demo.destroy();
+    demo.highlight({ element: "#card-1", popover: { title: "Second" } });
+
+    expect(demo.isActive()).toBe(true);
+    expect(popoverTitle()).toBe("Second");
+    expect(document.querySelector("#card-1")?.classList.contains("driver-active-element")).toBe(true);
+    expect(document.querySelector("#intro")?.classList.contains("driver-active-element")).toBe(false);
+  });
+
+  it("keeps the replacement tour after an animated highlight is destroyed", async () => {
+    const demo = track(driver({ animate: true, duration: 50 }));
+    demo.highlight({ element: "#card-1", popover: { title: "Highlight" } });
+    await nextFrame();
+    demo.destroy();
+
+    const tour = liveTour();
+    tour.drive();
+    await nextFrame();
+    demo.destroy();
+    await nextFrame();
+
+    expectTourIntact(tour);
+  });
+});