fix: remove scene hack from export.ts & remove pass elementsMap to getContainingFrame (#7713)

* fix: remove scene hack from export.ts as its not needed anymore

* remove

* pass elementsMap to getContainingFrame

* remove unused `mapElementIds` param

---------

Co-authored-by: dwelle <5153846+dwelle@users.noreply.github.com>

packages/excalidraw/clipboard.ts

@@ -16,7 +16,7 @@ import {
 import { deepCopyElement } from "./element/newElement";
 import { mutateElement } from "./element/mutateElement";
 import { getContainingFrame } from "./frame";
-import { isMemberOf, isPromiseLike } from "./utils";
+import { arrayToMap, isMemberOf, isPromiseLike } from "./utils";
 import { t } from "./i18n";
 
 type ElementsClipboard = {
@@ -126,6 +126,7 @@ export const serializeAsClipboardJSON = ({
   elements: readonly NonDeletedExcalidrawElement[];
   files: BinaryFiles | null;
 }) => {
+  const elementsMap = arrayToMap(elements);
   const framesToCopy = new Set(
     elements.filter((element) => isFrameLikeElement(element)),
   );
@@ -152,8 +153,8 @@ export const serializeAsClipboardJSON = ({
     type: EXPORT_DATA_TYPES.excalidrawClipboard,
     elements: elements.map((element) => {
       if (
-        getContainingFrame(element) &&
-        !framesToCopy.has(getContainingFrame(element)!)
+        getContainingFrame(element, elementsMap) &&
+        !framesToCopy.has(getContainingFrame(element, elementsMap)!)
       ) {
         const copiedElement = deepCopyElement(element);
         mutateElement(copiedElement, {

packages/excalidraw/components/App.tsx

@@ -1131,7 +1131,7 @@ class App extends React.Component<AppProps, AppState> {
                 display: isVisible ? "block" : "none",
                 opacity: getRenderOpacity(
                   el,
-                  getContainingFrame(el),
+                  getContainingFrame(el, this.scene.getNonDeletedElementsMap()),
                   this.elementsPendingErasure,
                 ),
                 ["--embeddable-radius" as string]: `${getCornerRadius(
@@ -4399,7 +4399,7 @@ class App extends React.Component<AppProps, AppState> {
       ),
     ).filter((element) => {
       // hitting a frame's element from outside the frame is not considered a hit
-      const containingFrame = getContainingFrame(element);
+      const containingFrame = getContainingFrame(element, elementsMap);
       return containingFrame &&
         this.state.frameRendering.enabled &&
         this.state.frameRendering.clip
@@ -7789,7 +7789,7 @@ class App extends React.Component<AppProps, AppState> {
             );
 
             if (linearElement?.frameId) {
-              const frame = getContainingFrame(linearElement);
+              const frame = getContainingFrame(linearElement, elementsMap);
 
               if (frame && linearElement) {
                 if (

packages/excalidraw/frame.ts

@@ -21,7 +21,7 @@ import { mutateElement } from "./element/mutateElement";
 import { AppClassProperties, AppState, StaticCanvasAppState } from "./types";
 import { getElementsWithinSelection, getSelectedElements } from "./scene";
 import { getElementsInGroup, selectGroupsFromGivenElements } from "./groups";
-import Scene, { ExcalidrawElementsIncludingDeleted } from "./scene/Scene";
+import type { ExcalidrawElementsIncludingDeleted } from "./scene/Scene";
 import { getElementLineSegments } from "./element/bounds";
 import {
   doLineSegmentsIntersect,
@@ -377,25 +377,13 @@ export const getElementsInNewFrame = (
 
 export const getContainingFrame = (
   element: ExcalidrawElement,
-  /**
-   * Optionally an elements map, in case the elements aren't in the Scene yet.
-   * Takes precedence over Scene elements, even if the element exists
-   * in Scene elements and not the supplied elements map.
-   */
-  elementsMap?: Map<string, ExcalidrawElement>,
+  elementsMap: ElementsMap,
 ) => {
-  if (element.frameId) {
-    if (elementsMap) {
-      return (elementsMap.get(element.frameId) ||
-        null) as null | ExcalidrawFrameLikeElement;
-    }
-    return (
-      (Scene.getScene(element)?.getElement(
-        element.frameId,
-      ) as ExcalidrawFrameLikeElement) || null
-    );
+  if (!element.frameId) {
+    return null;
   }
-  return null;
+  return (elementsMap.get(element.frameId) ||
+    null) as null | ExcalidrawFrameLikeElement;
 };
 
 // --------------------------- Frame Operations -------------------------------
@@ -697,7 +685,7 @@ export const getTargetFrame = (
   return appState.selectedElementIds[_element.id] &&
     appState.selectedElementsAreBeingDragged
     ? appState.frameToHighlight
-    : getContainingFrame(_element);
+    : getContainingFrame(_element, elementsMap);
 };
 
 // TODO: this a huge bottleneck for large scenes, optimise

packages/excalidraw/renderer/renderElement.ts

@@ -257,7 +257,8 @@ const generateElementCanvas = (
     canvasOffsetY,
     boundTextElementVersion:
       getBoundTextElement(element, elementsMap)?.version || null,
-    containingFrameOpacity: getContainingFrame(element)?.opacity || 100,
+    containingFrameOpacity:
+      getContainingFrame(element, elementsMap)?.opacity || 100,
   };
 };
 
@@ -440,7 +441,8 @@ const generateElementWithCanvas = (
   const boundTextElementVersion =
     getBoundTextElement(element, elementsMap)?.version || null;
 
-  const containingFrameOpacity = getContainingFrame(element)?.opacity || 100;
+  const containingFrameOpacity =
+    getContainingFrame(element, elementsMap)?.opacity || 100;
 
   if (
     !prevElementWithCanvas ||
@@ -652,7 +654,7 @@ export const renderElement = (
 ) => {
   context.globalAlpha = getRenderOpacity(
     element,
-    getContainingFrame(element),
+    getContainingFrame(element, elementsMap),
     renderConfig.elementsPendingErasure,
   );
 
@@ -924,11 +926,12 @@ const maybeWrapNodesInFrameClipPath = (
   root: SVGElement,
   nodes: SVGElement[],
   frameRendering: AppState["frameRendering"],
+  elementsMap: RenderableElementsMap,
 ) => {
   if (!frameRendering.enabled || !frameRendering.clip) {
     return null;
   }
-  const frame = getContainingFrame(element);
+  const frame = getContainingFrame(element, elementsMap);
   if (frame) {
     const g = root.ownerDocument!.createElementNS(SVG_NS, "g");
     g.setAttributeNS(SVG_NS, "clip-path", `url(#${frame.id})`);
@@ -990,7 +993,9 @@ export const renderElementToSvg = (
   };
 
   const opacity =
-    ((getContainingFrame(element)?.opacity ?? 100) * element.opacity) / 10000;
+    ((getContainingFrame(element, elementsMap)?.opacity ?? 100) *
+      element.opacity) /
+    10000;
 
   switch (element.type) {
     case "selection": {
@@ -1024,6 +1029,7 @@ export const renderElementToSvg = (
         root,
         [node],
         renderConfig.frameRendering,
+        elementsMap,
       );
 
       addToRoot(g || node, element);
@@ -1215,6 +1221,7 @@ export const renderElementToSvg = (
         root,
         [group, maskPath],
         renderConfig.frameRendering,
+        elementsMap,
       );
       if (g) {
         addToRoot(g, element);
@@ -1258,6 +1265,7 @@ export const renderElementToSvg = (
         root,
         [node],
         renderConfig.frameRendering,
+        elementsMap,
       );
 
       addToRoot(g || node, element);
@@ -1355,6 +1363,7 @@ export const renderElementToSvg = (
           root,
           [g],
           renderConfig.frameRendering,
+          elementsMap,
         );
         addToRoot(clipG || g, element);
       }
@@ -1442,6 +1451,7 @@ export const renderElementToSvg = (
           root,
           [node],
           renderConfig.frameRendering,
+          elementsMap,
         );
 
         addToRoot(g || node, element);

packages/excalidraw/scene/Scene.ts

@@ -80,29 +80,16 @@ class Scene {
   private static sceneMapByElement = new WeakMap<ExcalidrawElement, Scene>();
   private static sceneMapById = new Map<string, Scene>();
 
-  static mapElementToScene(
-    elementKey: ElementKey,
-    scene: Scene,
-    /**
-     * needed because of frame exporting hack.
-     * elementId:Scene mapping will be removed completely, soon.
-     */
-    mapElementIds = true,
-  ) {
+  static mapElementToScene(elementKey: ElementKey, scene: Scene) {
     if (isIdKey(elementKey)) {
-      if (!mapElementIds) {
-        return;
-      }
       // for cases where we don't have access to the element object
       // (e.g. restore serialized appState with id references)
       this.sceneMapById.set(elementKey, scene);
     } else {
       this.sceneMapByElement.set(elementKey, scene);
-      if (!mapElementIds) {
-        // if mapping element objects, also cache the id string when later
-        // looking up by id alone
-        this.sceneMapById.set(elementKey.id, scene);
-      }
+      // if mapping element objects, also cache the id string when later
+      // looking up by id alone
+      this.sceneMapById.set(elementKey.id, scene);
     }
   }
 
@@ -256,7 +243,7 @@ class Scene {
     return didChange;
   }
 
-  replaceAllElements(nextElements: ElementsMapOrArray, mapElementIds = true) {
+  replaceAllElements(nextElements: ElementsMapOrArray) {
     this.elements =
       // ts doesn't like `Array.isArray` of `instanceof Map`
       nextElements instanceof Array
@@ -269,7 +256,7 @@ class Scene {
         nextFrameLikes.push(element);
       }
       this.elementsMap.set(element.id, element);
-      Scene.mapElementToScene(element, this, mapElementIds);
+      Scene.mapElementToScene(element, this);
     });
     const nonDeletedElements = getNonDeletedElements(this.elements);
     this.nonDeletedElements = nonDeletedElements.elements;

packages/excalidraw/scene/export.ts

@@ -12,13 +12,7 @@ import {
   getElementAbsoluteCoords,
 } from "../element/bounds";
 import { renderSceneToSvg, renderStaticScene } from "../renderer/renderScene";
-import {
-  arrayToMap,
-  cloneJSON,
-  distance,
-  getFontString,
-  toBrandedType,
-} from "../utils";
+import { arrayToMap, distance, getFontString, toBrandedType } from "../utils";
 import { AppState, BinaryFiles } from "../types";
 import {
   DEFAULT_EXPORT_PADDING,
@@ -42,35 +36,11 @@ import {
 import { newTextElement } from "../element";
 import { Mutable } from "../utility-types";
 import { newElementWith } from "../element/mutateElement";
-import Scene from "./Scene";
 import { isFrameElement, isFrameLikeElement } from "../element/typeChecks";
 import { RenderableElementsMap } from "./types";
 
 const SVG_EXPORT_TAG = `<!-- svg-source:excalidraw -->`;
 
-// getContainerElement and getBoundTextElement and potentially other helpers
-// depend on `Scene` which will not be available when these pure utils are
-// called outside initialized Excalidraw editor instance or even if called
-// from inside Excalidraw if the elements were never cached by Scene (e.g.
-// for library elements).
-//
-// As such, before passing the elements down, we need to initialize a custom
-// Scene instance and assign them to it.
-//
-// FIXME This is a super hacky workaround and we'll need to rewrite this soon.
-const __createSceneForElementsHack__ = (
-  elements: readonly ExcalidrawElement[],
-) => {
-  const scene = new Scene();
-  // we can't duplicate elements to regenerate ids because we need the
-  // orig ids when embedding. So we do another hack of not mapping element
-  // ids to Scene instances so that we don't override the editor elements
-  // mapping.
-  // We still need to clone the objects themselves to regen references.
-  scene.replaceAllElements(cloneJSON(elements), false);
-  return scene;
-};
-
 const truncateText = (element: ExcalidrawTextElement, maxWidth: number) => {
   if (element.width <= maxWidth) {
     return element;
@@ -213,9 +183,6 @@ export const exportToCanvas = async (
     return { canvas, scale: appState.exportScale };
   },
 ) => {
-  const tempScene = __createSceneForElementsHack__(elements);
-  elements = tempScene.getNonDeletedElements();
-
   const frameRendering = getFrameRenderingConfig(
     exportingFrame ?? null,
     appState.frameRendering ?? null,
@@ -281,8 +248,6 @@ export const exportToCanvas = async (
     },
   });
 
-  tempScene.destroy();
-
   return canvas;
 };
 
@@ -306,9 +271,6 @@ export const exportToSvg = async (
     exportingFrame?: ExcalidrawFrameLikeElement | null;
   },
 ): Promise<SVGSVGElement> => {
-  const tempScene = __createSceneForElementsHack__(elements);
-  elements = tempScene.getNonDeletedElements();
-
   const frameRendering = getFrameRenderingConfig(
     opts?.exportingFrame ?? null,
     appState.frameRendering ?? null,
@@ -470,8 +432,6 @@ export const exportToSvg = async (
     },
   );
 
-  tempScene.destroy();
-
   return svgRoot;
 };
 

packages/excalidraw/scene/selection.ts

@@ -57,7 +57,7 @@ export const getElementsWithinSelection = (
       elementsMap,
     );
 
-    const containingFrame = getContainingFrame(element);
+    const containingFrame = getContainingFrame(element, elementsMap);
     if (containingFrame) {
       const [fx1, fy1, fx2, fy2] = getElementBounds(
         containingFrame,
@@ -86,7 +86,7 @@ export const getElementsWithinSelection = (
     : elementsInSelection;
 
   elementsInSelection = elementsInSelection.filter((element) => {
-    const containingFrame = getContainingFrame(element);
+    const containingFrame = getContainingFrame(element, elementsMap);
 
     if (containingFrame) {
       return elementOverlapsWithFrame(element, containingFrame, elementsMap);