From c308cc2edd3168a4da673881ed4451043b29caea Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mitja=20Bezen=C5=A1ek?= Date: Fri, 3 May 2024 12:47:58 +0200 Subject: [PATCH] Prevent unnecessary fetching of readonly slugs (#3663) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Prevents unnecessary fetching of readonly slugs. We only need to fetch it if we don't have it yet. There was also a weird issue with `window.location.href` sometimes returning encoded search params and sometimes decoded ones: ![image](https://github.com/tldraw/tldraw/assets/2523721/ca1e36c6-5e86-4e48-9350-c53de32a9f2e) This then caused an additional fetch in the `setInterval` since the [urls did not match](https://github.com/tldraw/tldraw/blob/main/apps/dotcom/src/components/ShareMenu.tsx#L140). ![CleanShot 2024-04-30 at 14 37 12](https://github.com/tldraw/tldraw/assets/2523721/b1c540aa-902a-4574-a8e7-a0507f7dbda2) Resolves https://github.com/tldraw/tldraw/issues/3661 ### Change Type - [ ] `sdk` — Changes the tldraw SDK - [x] `dotcom` — Changes the tldraw.com web app - [ ] `docs` — Changes to the documentation, examples, or templates. - [ ] `vs code` — Changes to the vscode plugin - [ ] `internal` — Does not affect user-facing stuff - [ ] `bugfix` — Bug fix - [ ] `feature` — New feature - [x] `improvement` — Improving existing features - [ ] `chore` — Updating dependencies, other boring stuff - [ ] `galaxy brain` — Architectural changes - [ ] `tests` — Changes to any test code - [ ] `tools` — Changes to infrastructure, CI, internal scripts, debugging tools, etc. - [ ] `dunno` — I don't know ### Test Plan 1. Open a multiplayer room. You should only see one fetch for the readonly slug. 1. Open a local room. 2. Share it. You should only see one fetch for the readonly slug. - [ ] Unit Tests - [ ] End to end tests ### Release Notes - Prevent unnecessary fetching of readonly slugs. --- apps/dotcom/src/components/ShareMenu.tsx | 28 +++++++++++++++--------- apps/dotcom/src/utils/sharing.ts | 15 +++++++++++-- 2 files changed, 31 insertions(+), 12 deletions(-) diff --git a/apps/dotcom/src/components/ShareMenu.tsx b/apps/dotcom/src/components/ShareMenu.tsx index 65a229703..04ac267db 100644 --- a/apps/dotcom/src/components/ShareMenu.tsx +++ b/apps/dotcom/src/components/ShareMenu.tsx @@ -124,16 +124,24 @@ export const ShareMenu = React.memo(function ShareMenu() { }) } - getReadonlyUrl().then((readonlyUrl) => { - if (readonlyUrl && !shareState.readonlyQrCodeDataUrl) { - // fetch the readonly QR code data URL - createQRCodeImageDataString(readonlyUrl).then((dataUrl) => { - if (!cancelled) { - setShareState((s) => ({ ...s, readonlyUrl, readonlyQrCodeDataUrl: dataUrl })) - } - }) - } - }) + if (!shareState.readonlyUrl) { + getReadonlyUrl().then((readonlyUrl) => { + if (readonlyUrl) { + // fetch the readonly QR code data URL + createQRCodeImageDataString(readonlyUrl).then((dataUrl) => { + if (!cancelled) { + setShareState((s) => ({ ...s, readonlyUrl, readonlyQrCodeDataUrl: dataUrl })) + } + }) + } + }) + } else if (!shareState.readonlyQrCodeDataUrl) { + createQRCodeImageDataString(shareState.readonlyUrl).then((dataUrl) => { + if (!cancelled) { + setShareState((s) => ({ ...s, readonlyQrCodeDataUrl: dataUrl })) + } + }) + } const interval = setInterval(() => { const url = window.location.href diff --git a/apps/dotcom/src/utils/sharing.ts b/apps/dotcom/src/utils/sharing.ts index 193fdd9cf..1d2d7c91d 100644 --- a/apps/dotcom/src/utils/sharing.ts +++ b/apps/dotcom/src/utils/sharing.ts @@ -70,7 +70,12 @@ async function getSnapshotLink( return '' } const paramsToUse = getViewportUrlQuery(editor) - const params = paramsToUse ? `?${new URLSearchParams(paramsToUse).toString()}` : '' + // React router has an issue with the search params being encoded, which can cause multiple navigations + // and can also make us believe that the URL has changed when it hasn't. + // https://github.com/tldraw/tldraw/pull/3663#discussion_r1584946080 + const params = paramsToUse + ? decodeURIComponent(`?${new URLSearchParams(paramsToUse).toString()}`) + : '' return new Blob([`${window.location.origin}/${SNAPSHOT_PREFIX}/${response.roomId}${params}`], { type: 'text/plain', }) @@ -134,7 +139,13 @@ export function useSharing(): TLUiOverrides { const query = getViewportUrlQuery(editor) const origin = window.location.origin - const pathname = `/${ROOM_PREFIX}/${response.slug}?${new URLSearchParams(query ?? {}).toString()}` + + // React router has an issue with the search params being encoded, which can cause multiple navigations + // and can also make us believe that the URL has changed when it hasn't. + // https://github.com/tldraw/tldraw/pull/3663#discussion_r1584946080 + const pathname = decodeURIComponent( + `/${ROOM_PREFIX}/${response.slug}?${new URLSearchParams(query ?? {}).toString()}` + ) if (runningInIFrame) { window.open(`${origin}${pathname}`) } else {