From 0c24a7042f3abdbada1281dc5dfa61160f6b881b Mon Sep 17 00:00:00 2001 From: David Luzar <5153846+dwelle@users.noreply.github.com> Date: Thu, 11 Jan 2024 17:42:51 +0100 Subject: [PATCH] feat: remove `ExcalidrawEmbeddableElement.validated` flag (#7539) --- packages/excalidraw/CHANGELOG.md | 5 ++- packages/excalidraw/components/App.tsx | 33 +++++++++++++++---- packages/excalidraw/data/restore.ts | 5 +-- packages/excalidraw/element/Hyperlink.tsx | 15 +++++---- packages/excalidraw/element/newElement.ts | 6 +--- packages/excalidraw/element/types.ts | 8 ----- packages/excalidraw/renderer/renderScene.ts | 4 ++- packages/excalidraw/scene/Shape.ts | 25 +++++++++++--- packages/excalidraw/scene/ShapeCache.ts | 4 ++- packages/excalidraw/scene/export.ts | 17 +++++++++- packages/excalidraw/scene/types.ts | 3 ++ .../tests/fixtures/elementFixture.ts | 1 - packages/excalidraw/tests/helpers/api.ts | 1 - packages/excalidraw/types.ts | 6 ++++ 14 files changed, 94 insertions(+), 39 deletions(-) diff --git a/packages/excalidraw/CHANGELOG.md b/packages/excalidraw/CHANGELOG.md index 3fd3f3783..9f59bd4af 100644 --- a/packages/excalidraw/CHANGELOG.md +++ b/packages/excalidraw/CHANGELOG.md @@ -14,9 +14,12 @@ Please add the latest change on the top under the correct section. ## Unreleased - Expose `getVisibleSceneBounds` helper to get scene bounds of visible canvas area. [#7450](https://github.com/excalidraw/excalidraw/pull/7450) +- Remove `ExcalidrawEmbeddableElement.validated` attribute. [#7539](https://github.com/excalidraw/excalidraw/pull/7539) ### Breaking Changes +- `ExcalidrawEmbeddableElement.validated` was removed and moved to private editor state. This should largely not affect your apps unless you were reading from this attribute. We keep validating embeddable urls internally, and the public [`props.validateEmbeddable`](https://docs.excalidraw.com/docs/@excalidraw/excalidraw/api/props#validateembeddable) still applies. [#7539](https://github.com/excalidraw/excalidraw/pull/7539) + - Create an `ESM` build for `@excalidraw/excalidraw`. The API is in progress and subject to change before stable release. There are some changes on how the package will be consumed #### Bundler @@ -265,7 +268,7 @@ define: { - Support creating containers, linear elements, text containers, labelled arrows and arrow bindings programatically [#6546](https://github.com/excalidraw/excalidraw/pull/6546) - Introducing Web-Embeds (alias iframe element)[#6691](https://github.com/excalidraw/excalidraw/pull/6691) -- Added [`props.validateEmbeddable`](https://docs.excalidraw.com/docs/@excalidraw/excalidraw/api/props#validateEmbeddable) to customize embeddable src url validation. [#6691](https://github.com/excalidraw/excalidraw/pull/6691) +- Added [`props.validateEmbeddable`](https://docs.excalidraw.com/docs/@excalidraw/excalidraw/api/props#validateembeddable) to customize embeddable src url validation. [#6691](https://github.com/excalidraw/excalidraw/pull/6691) - Add support for `opts.fitToViewport` and `opts.viewportZoomFactor` in the [`ExcalidrawAPI.scrollToContent`](https://docs.excalidraw.com/docs/@excalidraw/excalidraw/api/props/excalidraw-api#scrolltocontent) API. [#6581](https://github.com/excalidraw/excalidraw/pull/6581). - Properly sanitize element `link` urls. [#6728](https://github.com/excalidraw/excalidraw/pull/6728). - Sidebar component now supports tabs — for more detailed description of new behavior and breaking changes, see the linked PR. [#6213](https://github.com/excalidraw/excalidraw/pull/6213) diff --git a/packages/excalidraw/components/App.tsx b/packages/excalidraw/components/App.tsx index 2d88d3a63..295c9dc08 100644 --- a/packages/excalidraw/components/App.tsx +++ b/packages/excalidraw/components/App.tsx @@ -246,6 +246,7 @@ import { ToolType, OnUserFollowedPayload, UnsubscribeCallback, + EmbedsValidationStatus, ElementsPendingErasure, } from "../types"; import { @@ -529,6 +530,15 @@ class App extends React.Component { public files: BinaryFiles = {}; public imageCache: AppClassProperties["imageCache"] = new Map(); private iFrameRefs = new Map(); + /** + * Indicates whether the embeddable's url has been validated for rendering. + * If value not set, indicates that the validation is pending. + * Initially or on url change the flag is not reset so that we can guarantee + * the validation came from a trusted source (the editor). + **/ + private embedsValidationStatus: EmbedsValidationStatus = new Map(); + /** embeds that have been inserted to DOM (as a perf optim, we don't want to + * insert to DOM before user initially scrolls to them) */ private initializedEmbeds = new Set(); private elementsPendingErasure: ElementsPendingErasure = new Set(); @@ -869,6 +879,14 @@ class App extends React.Component { ); } + private updateEmbedValidationStatus = ( + element: ExcalidrawEmbeddableElement, + status: boolean, + ) => { + this.embedsValidationStatus.set(element.id, status); + ShapeCache.delete(element); + }; + private updateEmbeddables = () => { const iframeLikes = new Set(); @@ -876,7 +894,7 @@ class App extends React.Component { this.scene.getNonDeletedElements().filter((element) => { if (isEmbeddableElement(element)) { iframeLikes.add(element.id); - if (element.validated == null) { + if (!this.embedsValidationStatus.has(element.id)) { updated = true; const validated = embeddableURLValidator( @@ -884,8 +902,7 @@ class App extends React.Component { this.props.validateEmbeddable, ); - mutateElement(element, { validated }, false); - ShapeCache.delete(element); + this.updateEmbedValidationStatus(element, validated); } } else if (isIframeElement(element)) { iframeLikes.add(element.id); @@ -914,7 +931,9 @@ class App extends React.Component { .getNonDeletedElements() .filter( (el): el is NonDeleted => - (isEmbeddableElement(el) && !!el.validated) || isIframeElement(el), + (isEmbeddableElement(el) && + this.embedsValidationStatus.get(el.id) === true) || + isIframeElement(el), ); return ( @@ -1507,6 +1526,9 @@ class App extends React.Component { setAppState={this.setAppState} onLinkOpen={this.props.onLinkOpen} setToast={this.setToast} + updateEmbedValidationStatus={ + this.updateEmbedValidationStatus + } /> )} {this.props.aiEnabled !== false && @@ -1617,6 +1639,7 @@ class App extends React.Component { renderGrid: true, canvasBackgroundColor: this.state.viewBackgroundColor, + embedsValidationStatus: this.embedsValidationStatus, elementsPendingErasure: this.elementsPendingErasure, }} /> @@ -6425,7 +6448,6 @@ class App extends React.Component { width: embedLink.intrinsicSize.w, height: embedLink.intrinsicSize.h, link, - validated: null, }); this.scene.replaceAllElements([ @@ -6659,7 +6681,6 @@ class App extends React.Component { if (elementType === "embeddable") { element = newEmbeddableElement({ type: "embeddable", - validated: null, ...baseElementAttributes, }); } else { diff --git a/packages/excalidraw/data/restore.ts b/packages/excalidraw/data/restore.ts index 76a8c32ca..ff4063593 100644 --- a/packages/excalidraw/data/restore.ts +++ b/packages/excalidraw/data/restore.ts @@ -295,11 +295,8 @@ const restoreElement = ( case "rectangle": case "diamond": case "iframe": - return restoreElementWithProperties(element, {}); case "embeddable": - return restoreElementWithProperties(element, { - validated: null, - }); + return restoreElementWithProperties(element, {}); case "magicframe": case "frame": return restoreElementWithProperties(element, { diff --git a/packages/excalidraw/element/Hyperlink.tsx b/packages/excalidraw/element/Hyperlink.tsx index e5e7a5a14..a69fdeb83 100644 --- a/packages/excalidraw/element/Hyperlink.tsx +++ b/packages/excalidraw/element/Hyperlink.tsx @@ -39,7 +39,6 @@ import "./Hyperlink.scss"; import { trackEvent } from "../analytics"; import { useAppProps, useExcalidrawAppState } from "../components/App"; import { isEmbeddableElement } from "./typeChecks"; -import { ShapeCache } from "../scene/ShapeCache"; const CONTAINER_WIDTH = 320; const SPACE_BOTTOM = 85; @@ -64,6 +63,7 @@ export const Hyperlink = ({ setAppState, onLinkOpen, setToast, + updateEmbedValidationStatus, }: { element: NonDeletedExcalidrawElement; setAppState: React.Component["setState"]; @@ -71,6 +71,10 @@ export const Hyperlink = ({ setToast: ( toast: { message: string; closable?: boolean; duration?: number } | null, ) => void; + updateEmbedValidationStatus: ( + element: ExcalidrawEmbeddableElement, + status: boolean, + ) => void; }) => { const appState = useExcalidrawAppState(); const appProps = useAppProps(); @@ -98,9 +102,9 @@ export const Hyperlink = ({ } if (!link) { mutateElement(element, { - validated: false, link: null, }); + updateEmbedValidationStatus(element, false); return; } @@ -110,10 +114,9 @@ export const Hyperlink = ({ } element.link && embeddableLinkCache.set(element.id, element.link); mutateElement(element, { - validated: false, link, }); - ShapeCache.delete(element); + updateEmbedValidationStatus(element, false); } else { const { width, height } = element; const embedLink = getEmbedLink(link); @@ -142,10 +145,9 @@ export const Hyperlink = ({ : height, } : {}), - validated: true, link, }); - ShapeCache.delete(element); + updateEmbedValidationStatus(element, true); if (embeddableLinkCache.has(element.id)) { embeddableLinkCache.delete(element.id); } @@ -159,6 +161,7 @@ export const Hyperlink = ({ appProps.validateEmbeddable, appState.activeEmbeddable, setAppState, + updateEmbedValidationStatus, ]); useLayoutEffect(() => { diff --git a/packages/excalidraw/element/newElement.ts b/packages/excalidraw/element/newElement.ts index 91b30beb7..00cae296c 100644 --- a/packages/excalidraw/element/newElement.ts +++ b/packages/excalidraw/element/newElement.ts @@ -136,13 +136,9 @@ export const newElement = ( export const newEmbeddableElement = ( opts: { type: "embeddable"; - validated: ExcalidrawEmbeddableElement["validated"]; } & ElementConstructorOpts, ): NonDeleted => { - return { - ..._newElementBase("embeddable", opts), - validated: opts.validated, - }; + return _newElementBase("embeddable", opts); }; export const newIframeElement = ( diff --git a/packages/excalidraw/element/types.ts b/packages/excalidraw/element/types.ts index 38be1bda6..c468eac82 100644 --- a/packages/excalidraw/element/types.ts +++ b/packages/excalidraw/element/types.ts @@ -88,14 +88,6 @@ export type ExcalidrawEllipseElement = _ExcalidrawElementBase & { export type ExcalidrawEmbeddableElement = _ExcalidrawElementBase & Readonly<{ type: "embeddable"; - /** - * indicates whether the embeddable src (url) has been validated for rendering. - * null value indicates that the validation is pending. We reset the - * value on each restore (or url change) so that we can guarantee - * the validation came from a trusted source (the editor). Also because we - * may not have access to host-app supplied url validator during restore. - */ - validated: boolean | null; }>; export type ExcalidrawIframeElement = _ExcalidrawElementBase & diff --git a/packages/excalidraw/renderer/renderScene.ts b/packages/excalidraw/renderer/renderScene.ts index c41d59bd3..a5b78d3b8 100644 --- a/packages/excalidraw/renderer/renderScene.ts +++ b/packages/excalidraw/renderer/renderScene.ts @@ -1007,7 +1007,9 @@ const _renderStaticScene = ({ if ( isIframeLikeElement(element) && (isExporting || - (isEmbeddableElement(element) && !element.validated)) && + (isEmbeddableElement(element) && + renderConfig.embedsValidationStatus.get(element.id) !== + true)) && element.width && element.height ) { diff --git a/packages/excalidraw/scene/Shape.ts b/packages/excalidraw/scene/Shape.ts index 190f7562f..1d43aef71 100644 --- a/packages/excalidraw/scene/Shape.ts +++ b/packages/excalidraw/scene/Shape.ts @@ -21,6 +21,7 @@ import { isLinearElement, } from "../element/typeChecks"; import { canChangeRoundness } from "./comparisons"; +import { EmbedsValidationStatus } from "../types"; const getDashArrayDashed = (strokeWidth: number) => [8, 8 + strokeWidth]; @@ -118,10 +119,13 @@ export const generateRoughOptions = ( const modifyIframeLikeForRoughOptions = ( element: NonDeletedExcalidrawElement, isExporting: boolean, + embedsValidationStatus: EmbedsValidationStatus | null, ) => { if ( isIframeLikeElement(element) && - (isExporting || (isEmbeddableElement(element) && !element.validated)) && + (isExporting || + (isEmbeddableElement(element) && + embedsValidationStatus?.get(element.id) !== true)) && isTransparent(element.backgroundColor) && isTransparent(element.strokeColor) ) { @@ -278,7 +282,12 @@ export const _generateElementShape = ( { isExporting, canvasBackgroundColor, - }: { isExporting: boolean; canvasBackgroundColor: string }, + embedsValidationStatus, + }: { + isExporting: boolean; + canvasBackgroundColor: string; + embedsValidationStatus: EmbedsValidationStatus | null; + }, ): Drawable | Drawable[] | null => { switch (element.type) { case "rectangle": @@ -299,7 +308,11 @@ export const _generateElementShape = ( h - r } L 0 ${r} Q 0 0, ${r} 0`, generateRoughOptions( - modifyIframeLikeForRoughOptions(element, isExporting), + modifyIframeLikeForRoughOptions( + element, + isExporting, + embedsValidationStatus, + ), true, ), ); @@ -310,7 +323,11 @@ export const _generateElementShape = ( element.width, element.height, generateRoughOptions( - modifyIframeLikeForRoughOptions(element, isExporting), + modifyIframeLikeForRoughOptions( + element, + isExporting, + embedsValidationStatus, + ), false, ), ); diff --git a/packages/excalidraw/scene/ShapeCache.ts b/packages/excalidraw/scene/ShapeCache.ts index e5a08c1f2..3bca88e85 100644 --- a/packages/excalidraw/scene/ShapeCache.ts +++ b/packages/excalidraw/scene/ShapeCache.ts @@ -8,7 +8,7 @@ import { elementWithCanvasCache } from "../renderer/renderElement"; import { _generateElementShape } from "./Shape"; import { ElementShape, ElementShapes } from "./types"; import { COLOR_PALETTE } from "../colors"; -import { AppState } from "../types"; +import { AppState, EmbedsValidationStatus } from "../types"; export class ShapeCache { private static rg = new RoughGenerator(); @@ -51,6 +51,7 @@ export class ShapeCache { renderConfig: { isExporting: boolean; canvasBackgroundColor: AppState["viewBackgroundColor"]; + embedsValidationStatus: EmbedsValidationStatus; } | null, ) => { // when exporting, always regenerated to guarantee the latest shape @@ -72,6 +73,7 @@ export class ShapeCache { renderConfig || { isExporting: false, canvasBackgroundColor: COLOR_PALETTE.white, + embedsValidationStatus: null, }, ) as T["type"] extends keyof ElementShapes ? ElementShapes[T["type"]] diff --git a/packages/excalidraw/scene/export.ts b/packages/excalidraw/scene/export.ts index 6220c59da..cc84569a6 100644 --- a/packages/excalidraw/scene/export.ts +++ b/packages/excalidraw/scene/export.ts @@ -266,6 +266,8 @@ export const exportToCanvas = async ( imageCache, renderGrid: false, isExporting: true, + // empty disables embeddable rendering + embedsValidationStatus: new Map(), elementsPendingErasure: new Set(), }, }); @@ -288,6 +290,9 @@ export const exportToSvg = async ( }, files: BinaryFiles | null, opts?: { + /** + * if true, all embeddables passed in will be rendered when possible. + */ renderEmbeddables?: boolean; exportingFrame?: ExcalidrawFrameLikeElement | null; }, @@ -428,14 +433,24 @@ export const exportToSvg = async ( } const rsvg = rough.svg(svgRoot); + + const renderEmbeddables = opts?.renderEmbeddables ?? false; + renderSceneToSvg(elementsForRender, rsvg, svgRoot, files || {}, { offsetX, offsetY, isExporting: true, exportWithDarkMode, - renderEmbeddables: opts?.renderEmbeddables ?? false, + renderEmbeddables, frameRendering, canvasBackgroundColor: viewBackgroundColor, + embedsValidationStatus: renderEmbeddables + ? new Map( + elementsForRender + .filter((element) => isFrameLikeElement(element)) + .map((element) => [element.id, true]), + ) + : new Map(), }); tempScene.destroy(); diff --git a/packages/excalidraw/scene/types.ts b/packages/excalidraw/scene/types.ts index 401ab86d5..57a52fbd4 100644 --- a/packages/excalidraw/scene/types.ts +++ b/packages/excalidraw/scene/types.ts @@ -7,6 +7,7 @@ import { import { AppClassProperties, AppState, + EmbedsValidationStatus, ElementsPendingErasure, InteractiveCanvasAppState, StaticCanvasAppState, @@ -21,6 +22,7 @@ export type StaticCanvasRenderConfig = { /** when exporting the behavior is slightly different (e.g. we can't use CSS filters), and we disable render optimizations for best output */ isExporting: boolean; + embedsValidationStatus: EmbedsValidationStatus; elementsPendingErasure: ElementsPendingErasure; }; @@ -32,6 +34,7 @@ export type SVGRenderConfig = { renderEmbeddables: boolean; frameRendering: AppState["frameRendering"]; canvasBackgroundColor: AppState["viewBackgroundColor"]; + embedsValidationStatus: EmbedsValidationStatus; }; export type InteractiveCanvasRenderConfig = { diff --git a/packages/excalidraw/tests/fixtures/elementFixture.ts b/packages/excalidraw/tests/fixtures/elementFixture.ts index 7f1231a83..ddd7b8b9d 100644 --- a/packages/excalidraw/tests/fixtures/elementFixture.ts +++ b/packages/excalidraw/tests/fixtures/elementFixture.ts @@ -34,7 +34,6 @@ export const rectangleFixture: ExcalidrawElement = { export const embeddableFixture: ExcalidrawElement = { ...elementBase, type: "embeddable", - validated: null, }; export const ellipseFixture: ExcalidrawElement = { ...elementBase, diff --git a/packages/excalidraw/tests/helpers/api.ts b/packages/excalidraw/tests/helpers/api.ts index 2a18805ab..d22d3f221 100644 --- a/packages/excalidraw/tests/helpers/api.ts +++ b/packages/excalidraw/tests/helpers/api.ts @@ -205,7 +205,6 @@ export class API { element = newEmbeddableElement({ type: "embeddable", ...base, - validated: null, }); break; case "iframe": diff --git a/packages/excalidraw/types.ts b/packages/excalidraw/types.ts index 3da06bec4..201a186ba 100644 --- a/packages/excalidraw/types.ts +++ b/packages/excalidraw/types.ts @@ -19,6 +19,7 @@ import { ExcalidrawMagicFrameElement, ExcalidrawFrameLikeElement, ExcalidrawElementType, + ExcalidrawIframeLikeElement, } from "./element/types"; import { Action } from "./actions/types"; import { Point as RoughPoint } from "roughjs/bin/geometry"; @@ -746,4 +747,9 @@ export type Primitive = export type JSONValue = string | number | boolean | null | object; +export type EmbedsValidationStatus = Map< + ExcalidrawIframeLikeElement["id"], + boolean +>; + export type ElementsPendingErasure = Set;