[pull] main from tldraw:main#578
Merged
Merged
Conversation
Closes #8920. In order to stop multi-tab editing from feeling sluggish, this PR keeps a room at the full network sync rate whenever it has more than one session. Today a tab drops into "solo" mode — which throttles network sync to 1fps — whenever there are no *other users* present. But each tab, window, or device the same user opens is its own session, so two tabs of your own were both treated as single-player and both throttled, leaving edits to propagate between your own tabs with up to ~1s of lag. The fix is to count *all* other sessions in the room, not just other users'. Every session pushes its presence once on connect (ungated by presence mode), and the server never echoes a session its own record, so the store already holds one `instance_presence` record per other session — including your own other tabs and devices. An empty set means we're genuinely the only session and can throttle to solo; anything else (another user, or just another tab of our own) keeps us at full rate. ### Relationship to #8920 / #8949 This resolves #8920 by targeting the symptom users actually feel — multi-tab edit lag — rather than its original framing of sharing a single WebSocket to cut connection count. That heavier approach was explored in #8949 (a leader/presenter/follower layer, ~850 lines of new code) and closed in favor of this ~9-line change. Note this does **not** reduce the WebSocket count: each tab keeps its own socket, as today. If reducing connections per user becomes a priority later, #8949 remains the reference for that work. Because detection is server-mediated (via `instance_presence`) rather than a `BroadcastChannel`, it also works across **separate devices**, not just tabs in the same browser. ### Trade-off worth knowing With multiple of your own sessions in full mode, each pushes live presence. The editor renders one cursor per remote user (`CollaboratorsManager` dedupes by `userId`, newest activity wins), so collaborators see a single cursor that "teleports" to whichever of your windows you last touched — rather than two cursors. This is already today's behavior when you have multiple tabs open alongside other collaborators; this PR just extends full mode to the no-other-users case. Avoiding the teleport entirely is what the presenter role in #8949 buys. One caveat: detection relies on presence being enabled (the default). A consumer that disables presence pushes no `instance_presence` record, so its multi-tab sessions would still throttle to solo. ### Per-session presence is preserved end-to-end Worth flagging for future work: the collapse to one cursor per user happens *only* in the editor's view layer — not in the protocol or the store. The server assigns every session (each tab, window, or device) its own `presenceId` and broadcasts a distinct `instance_presence` record per session, and the client store keeps all of them (one per other session, including the user's own other tabs). It's `CollaboratorsManager.getCollaborators()` that groups those by `userId` and keeps only the most-recently-active record per user — which is why collaborators see one cursor rather than several. So showing a cursor *per session* — multiple live cursors for one user, one per tab/device — is already supported by the backend and the store. It would only take a change to that editor-side derivation (keying by the presence record instead of by `userId`), with no protocol or server change. This PR doesn't do that, but it doesn't foreclose it either. ### Change type - [x] `improvement` ### Test plan 1. Open a room in a single tab with no other users. Confirm it's in solo mode (network sync throttled). 2. Open the same room in a second tab as the same user. Confirm both tabs now sync at full rate — an edit in one appears in the other promptly, not ~1s later. 3. Open the same room on a second device as the same user. Confirm the same full-rate behavior (validates server-mediated, cross-device detection). 4. Close the second tab/device. Confirm the remaining lone session returns to solo throttling. 5. Confirm existing multi-user behavior is unchanged: with another user present, the room is at full rate. - [ ] Unit tests - [ ] End to end tests ### Release notes - Opening the same multiplayer room in multiple tabs, windows, or devices now keeps changes syncing at full speed between them, instead of throttling to once per second. ### Code changes | Section | LOC change | | --------- | ---------- | | Core code | +9 / -5 | Of the 9 added lines, 7 are an explanatory comment; the behavioral change is dropping the `userId` filter on the presence query and simplifying the `presenceMode` branch.
…ed to `dev-template` script (#9021) In order to test templates from another device on the local network, this PR lets `yarn dev-template <name>` forward any extra arguments to the template's vite dev server. For example, `yarn dev-template vite --host` exposes the dev server on the LAN. lazy applies passthrough args (`--`) to *every* matched task, so forwarding `--host` through the combined `lazy run dev` would also hand it to the `packages/tldraw` (chokidar) and `apps/bemo-worker` (tsx) dev scripts, which would break. To scope the args correctly, the shared deps keep running through lazy while vite runs directly in the template directory with the extra args. Because lazy installs no signal handlers of its own, the deps run in their own process group (`set -m`) and cleanup signals the whole group on exit, so lazy and the watchers it spawns are torn down instead of orphaned. With no extra args, the original single `lazy run dev` fast path is unchanged. ### Change type - [x] `other` ### Test plan 1. Run `yarn dev-template vite` — behaves as before (template, tldraw, and bemo-worker all start). 2. Run `yarn dev-template vite --host` — vite prints a Network URL; open it from another device on the LAN. 3. Quit with Ctrl-C and confirm no orphaned `vite`, `chokidar`, or `tsx` processes remain (`ps ax | grep -E 'chokidar|tsx'`). ### Code changes | Section | LOC change | | -------------- | ---------- | | Config/tooling | +23 / -1 |
In order to let the server templates build and run on current Node.js, this PR bumps `better-sqlite3` from `^11.8.1` to `^12.10.0` in the two server templates. The `11.10.0` native addon cannot compile against Node 26's V8 (it uses removed APIs like `Object::GetPrototype`, `Context::GetIsolate`, and `PropertyCallbackInfo::This`) and ships no Node 26 prebuilt binary, so `yarn install` fails with a `node-gyp` build error. `better-sqlite3@12` ships prebuilt binaries for Node 24/25/26 and builds cleanly. The only breaking change across the 11 → 12 line is dropping EOL Node.js 18 (`engines` is now `20.x || 22.x || 23.x || 24.x || 25.x || 26.x`), which is already below this repo's `node: ^20.0.0` requirement. There are no JavaScript API changes, and the templates only use basic `prepare`/`run`/`get`/`exec`, so no code changes are needed. ### Change type - [x] `other` (chore — template dependency bump) ### Test plan 1. With Node 24/26 active, run `yarn dev-template simple-server-example`. 2. Run `yarn dev-template socketio-server-example`. 3. Confirm the server starts and persists rooms via better-sqlite3 without a build error. ### Code changes | Section | LOC change | | -------------- | ---------- | | Templates | +2 / -2 | | Config/tooling | +6 / -6 | ### Release notes - Bump `better-sqlite3` to v12 in the server templates so they install and run on Node 24+ (including Node 26).
…ient (#8792) In order to remove bookkeeping that has no downstream consumer, this PR drops `useCallback` wrappers from event handlers in 8 components across `apps/docs` and `apps/dotcom/client`. In every case the consumer component is not wrapped in `React.memo`, the callback is not used as a dependency in any other hook, and there is no other reason for the cached reference to matter — so the `useCallback` was pure overhead. Opening as a draft because I am unsure about whether there is something I cannot see! ### Motivation While reviewing `apps/docs/components/content/copy-button.tsx`, the `useCallback` on `handleClick` looked unnecessary. The consumer (`Button` in `apps/docs/components/common/button.tsx`) is not wrapped in `React.memo`, the callback was never used as a hook dep, and `Button` itself creates a fresh inline arrow function each render and forwards _that_ to the real DOM `<button>` — so any reference stability from the parent's `useCallback` was discarded one component down. Investigating the same pattern across `apps/docs` and `apps/dotcom/client` surfaced 7 more files. Related: https://react.dev/learn/you-might-not-need-an-effect ### What changed Replaced `useCallback` wrappers with plain function declarations (and dropped the now-unused `useCallback` imports): | File | useCallbacks removed | | ---- | -------------------- | | `apps/docs/components/content/copy-button.tsx` | 1 | | `apps/docs/components/docs/copy-markdown-button.tsx` | 1 | | `apps/docs/components/docs/docs-feedback-widget.tsx` | 3 | | `apps/dotcom/client/.../TlaSidebarCreateFileButton.tsx` | 1 | | `apps/dotcom/client/.../TlaFileShareMenu/Tabs/TlaAnonCopyLinkTab.tsx` | 1 | | `apps/dotcom/client/.../TlaFileShareMenu/Tabs/TlaExportTab.tsx` | 6 | | `apps/dotcom/client/.../dialogs/TlaDeleteFileDialog.tsx` | 1 | | `apps/dotcom/client/.../TlaSidebar/components/TlaSidebarFileLink.tsx` | 1 | **Skipped:** `apps/docs/components/search/FullPageSearch.tsx`. Its `handleKeyDown` is passed to a third-party Ariakit `Combobox`. I did not want to remove memoization without verifying Ariakit's internals. ### Reasoning For each file I verified three things: 1. The consumer component is not wrapped in `React.memo`. Confirmed for `Button`, `TldrawUiButton` (`forwardRef` only, not `memo`), `TlaShareMenuCopyButton`, `TlaMenuSwitch`, `TlaMenuSelect`, and `TlaButton` (`forwardRef` only, not `memo`). 2. The callback is not referenced as a dependency in any other hook in the same file. 3. Every handler is a local `const` inside its component body — no cross-file consumption. Without a memoized child or another hook depending on the function reference, `useCallback` adds bookkeeping cost (deps comparison, cached-function retention) without any downstream benefit. The deps arrays exist because `eslint-plugin-react-hooks/exhaustive-deps` requires them once `useCallback` is in use — not because the callback relies on stale-capture semantics. Every closure here reads values that should always be fresh, so removing `useCallback` is, if anything, slightly safer (no risk of a missing dep producing a stale closure later). ### Verification - `yarn typecheck` from repo root: passes. - `yarn lint-current` on changed files: 0 warnings, 0 errors. - `yarn test run` in `apps/dotcom/client`: 133 tests pass. - `yarn test run` in `apps/docs`: 10 tests pass. - Manual smoke testing of affected UI surfaces: not done. ### Open questions for reviewers 1. Is there a team convention I'm missing where `useCallback` is preferred prophylactically (e.g. in case a child is later memoized)? If so, this PR contradicts that convention. 2. Are any of the affected dotcom components (`TlaButton`, `TldrawUiButton`, etc.) intended to be wrapped in `React.memo` in the future? If so, leaving the `useCallback`s in might be the right call. 3. Should `FullPageSearch.tsx` be cleaned up too? I left it because the consumer is third-party. 4. The cleanup is consistent with React's official guidance ("you don't need useCallback unless..."), but I want to confirm that matches this codebase's preference before extending the audit. ### Proposition If the reasoning holds, I'd like to extend a similar audit/cleanup pass to other parts of the repo where the pattern recurs. I'd want sign-off on the philosophy first. ### Change type - [x] `other` ### Test plan 1. In docs, navigate to a page with a code block, click the copy button — text should copy and the icon should briefly change to a checkmark. 2. In docs, on any docs page click the feedback widget thumbs up/down, then submit feedback in the textarea — the success state should appear. 3. In tldraw.com, sign in and click the sidebar "Create file" button — it should create a new file and open inline rename. 4. In tldraw.com, open the share menu on a file, switch to the Export tab, toggle padding/background, change theme/format, click "Export image" — should produce an image download. 5. In tldraw.com (signed out), open the share menu and click "Copy link" — clipboard should contain a deep link. 6. In tldraw.com, right-click a file in the sidebar and choose Delete — the confirm dialog should fire and the file should be deleted/forgotten. 7. In tldraw.com, hover the guest badge on a shared file in the sidebar — clicking the tooltip should navigate to the file. - [ ] Unit tests - [ ] End to end tests ### Code changes | Section | LOC change | | --------------- | ---------- | | Documentation | +29 / -32 | | Apps | +41 / -55 |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
4 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
See Commits and Changes for more details.
Created by
pull[bot] (v2.0.0-alpha.4)
Can you help keep this open source service alive? 💖 Please sponsor : )