[pull] main from tldraw:main#605
Merged
Merged
Conversation
### Change type - [ ] `bugfix` - [x] `improvement` - [ ] `feature` - [ ] `api` - [ ] `other` ### Release notes - ui: removes title attribute for menu items which were extraneous
In order to make the workspace member-role selects (and other `TlaMenuSelect` menus) usable on iOS, this PR fixes the wrapper that was swallowing the trigger click. Radix Select opens its dropdown on `click` for touch and pen input, but only on `pointerdown` for mouse. The `TlaMenuSelect` wrapper called `stopPropagation()` in the capture phase, so the click was stopped before it could descend to the trigger — harmless on desktop (already opened on `pointerdown`) but on iOS it meant the menu never opened and the control looked dead. Moving the guard to the bubble phase lets the trigger handle the click first, then still stops the click from reaching ancestor handlers (e.g. a background dismiss). This also tightens the menu select trigger height from 36px to 24px. ### Change type - [x] `bugfix` ### Test plan 1. On an iOS device or Simulator, open the workspace settings dialog. 2. Tap a member's role select. 3. Confirm the dropdown opens and options can be selected. 4. Confirm desktop mouse behaviour is unchanged. ### Release notes - Fix workspace settings selects not opening on iOS. ### Code changes | Section | LOC change | | -------------- | ---------- | | Apps | +7 / -2 |
Follow-up to #9226. That PR added per-script teardown so `yarn dev-app` stops leaving orphaned processes and Docker containers behind. It helped, but node/worker processes could still linger after quitting (and switching branches), which is what motivated this fix. ### Two problems **1. Only zero-cache reaped its descendant tree.** The other scripts signalled their **direct** child only — and each of those is either a wrapper or a process that forks its own grandchildren: | Script | Direct child | What leaked | | --- | --- | --- | | `client/scripts/dev-app.ts` | `vite` | vite's esbuild/optimizer helpers | | `sync-worker/dev.ts` | `yarn run -T tsx …` wrapper | the tsx → node → wrangler → workerd subtree | | `internal/scripts/workers/dev.ts` | `wrangler` / esbuild wrapper | workerd, esbuild watcher | `SIGKILL` is uncatchable and does not cascade, so killing the parent just orphaned the grandchildren — they reparent to `launchd` and keep holding the fixed dev ports. **2. The reap ran in the `exit` handler — too late.** In the dominant Ctrl+C path the direct child exits *before* the `exit` handler runs, and its children have already reparented to pid 1, so a `pgrep -P` walk from us finds nothing. I verified this with a minimal harness reproducing the exact topology: ``` [signal] descendantPids(parent) = [<child>, <grandchild>] <- tree still alive [exit] descendantPids(parent) = [] <- what the old reap saw >>> grandchild LEAKED (reparented to pid 1) ``` Reaping the **live** tree on the signal path instead cleans it up. ### What changed - Factor zero-cache's descendant-tree walk into a shared `killProcessTree` helper (`internal/scripts/lib/kill-tree.ts`). - Have every dev orchestrator reap the tree on the **signal/shutdown path** while it is still alive, killing deepest-first so nothing reparents out from under us. This is the pattern zero-cache already used, now applied consistently across `dev-app`, `sync-worker`, and the workers dev script. - `killProcessTree` remains as the synchronous `exit` backstop. It only uses `spawnSync`, which runs to completion in an `exit` handler — unlike the async `spawn` in zero-cache's `composeDown()`, which stays on the `shutdown()` path only. This also reconciles the previously contradictory zero-cache `exit`-handler comment. - Removed now-redundant tracking (`MiniflareMonitor.dispose()`, zero-cache's `children` array) in favour of the single tree reap. ### Validation - Minimal harness reproducing the orchestrator → wrapper → grandchild topology: confirmed the old `exit`-handler reap is a no-op (grandchild leaks) and the live-tree signal-path reap cleans it up. - Added a regression test (`kill-tree.test.ts`) that spawns a real three-level process tree and asserts the descendants are reaped while the root survives. Stable across repeated runs. - `yarn workspace dotcom test run scripts/` (6 tests) passes; `yarn typecheck` and lint clean. ### Test plan 1. `yarn dev-app`, confirm the stack starts (client 3000, sync-worker 8787, zero 4848, workers 9339, migrations 7654, Docker 6432/6543). 2. Ctrl+C, then confirm no stray `vite`/`workerd`/`wrangler`/zero processes remain (`lsof -nP -iTCP:3000,8787,4848,9339 -sTCP:LISTEN`) and the next `yarn dev-app` starts without a port-in-use error. - [x] Unit tests
In order to quiet the valid peer-dependency warnings emitted during `yarn install`, this PR fixes the three cases where one of our own workspaces was genuinely missing or under-pinning a required dependency. The remaining install warnings (React 19 / Vite 8 peer-range lag in `next-mdx-remote-client`, `react-helmet-async`, `@clerk/clerk-react`, `vite-plugin-vue-devtools`, plus `mcp-app`'s unused `ai`/`rollup` peers and `mermaid`'s library-level `react-dom` peer) are upstream conservatism or transitively satisfied, and are intentionally left as-is. What changed: - Bump `oxlint-tsgolint` `^0.18.0` → `^0.23.0` so it satisfies `oxlint@1.66.0`'s `>=0.22.1` request. - Add `vitest` to `internal/config`, the non-optional peer of `vitest-canvas-mock`. - Add `expect-webdriverio` and its follow-on peer `@wdio/logger` to `apps/examples`, the non-optional peers of `@wdio/globals` (the rest of the wdio device-test stack was already present). ### Change type - [x] `other` ### Test plan 1. Run `yarn install`. 2. Confirm the `oxlint-tsgolint`, `config`/`vitest`, and `examples`/`expect-webdriverio` warnings no longer appear in post-resolution validation. ### Code changes | Section | LOC change | | -------------- | ---------- | | Apps | +2 / -0 | | Config/tooling | +62 / -35 | --------- Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
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.
2 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 : )