From 8d4164452ae472741395354ecc266396b72417d4 Mon Sep 17 00:00:00 2001 From: imccyu Date: Thu, 6 Aug 2026 21:08:20 +0800 Subject: [PATCH] fix(ui): drop stale react-dom deps and address onboarding review knip: ui-settings no longer imports react-dom (the portal moved into OnboardingSurface), so the react-dom peer/dev dependencies and @types/react-dom go away with the usage. Review follow-ups: the Agent Note's Testing section now records the held-join reload regression scenario this PR adds (both languages, pairing re-recorded); the e2e hold gates EVERY settings.describe issued before release instead of only the first, so a future boot-time join consumer cannot silently collapse the widened window; the sampler's persistence across later navigations is documented and the vacuity wording softened to what the hold actually buys (timing independence). --- ...rding-step-owned-takeover-chrome.i18n.yaml | 4 +-- ...6-onboarding-step-owned-takeover-chrome.md | 2 +- ...nboarding-step-owned-takeover-chrome.zh.md | 2 +- .../tests/onboarding-deepseek-config.e2e.ts | 27 ++++++++++++------- packages/client/ui-settings/package.json | 7 ++--- pnpm-lock.yaml | 6 ----- 6 files changed, 24 insertions(+), 24 deletions(-) diff --git a/.agents/notes/implemented/bug-fix/2026-08-06-onboarding-step-owned-takeover-chrome.i18n.yaml b/.agents/notes/implemented/bug-fix/2026-08-06-onboarding-step-owned-takeover-chrome.i18n.yaml index 0f1ef9f696..a08d6323dd 100644 --- a/.agents/notes/implemented/bug-fix/2026-08-06-onboarding-step-owned-takeover-chrome.i18n.yaml +++ b/.agents/notes/implemented/bug-fix/2026-08-06-onboarding-step-owned-takeover-chrome.i18n.yaml @@ -2,5 +2,5 @@ # side as of the last confirmed-consistent state. Both languages carry equal authority; # after editing either side, bring the other along and re-record with: # pnpm run verify-translation-pairing --write .agents/notes/implemented/bug-fix/2026-08-06-onboarding-step-owned-takeover-chrome.md -2026-08-06-onboarding-step-owned-takeover-chrome.md: 6f0b1fa5df95a7b82daaa50c147eb44f1c99e26c -2026-08-06-onboarding-step-owned-takeover-chrome.zh.md: 37bbb47f6becf311d4a968bd47b48cce40051952 +2026-08-06-onboarding-step-owned-takeover-chrome.md: 4b3bbbc03c4494359297ae6e54bcc9a74c387e80 +2026-08-06-onboarding-step-owned-takeover-chrome.zh.md: 548285d285939325a26df3b8d70c43fe952813a9 diff --git a/.agents/notes/implemented/bug-fix/2026-08-06-onboarding-step-owned-takeover-chrome.md b/.agents/notes/implemented/bug-fix/2026-08-06-onboarding-step-owned-takeover-chrome.md index 6f0b1fa5df..4b3bbbc03c 100644 --- a/.agents/notes/implemented/bug-fix/2026-08-06-onboarding-step-owned-takeover-chrome.md +++ b/.agents/notes/implemented/bug-fix/2026-08-06-onboarding-step-owned-takeover-chrome.md @@ -32,4 +32,4 @@ A future step that registers without wrapping its visible content in `Onboarding ## Testing -`packages/client/ui-primitives/tests/onboarding-surface.spec.tsx` pins the primitive: body portal around the content, mask/stage class presence, `#root` inert held for exactly the mount lifetime, and the no-`#root` composition. `packages/client/ui-settings/tests/settings-root.spec.tsx` pins the inverted shell contract: no takeover chrome and no inert while a mounted step renders nothing. The existing step specs (`ui-settings-general`, `ui-models`) and the assembled `apps/web/tests/onboarding-deepseek-config.e2e.ts` scenario continue to pass unchanged — the mask selector and geometry pins survive because the stylesheet moved verbatim. +`packages/client/ui-primitives/tests/onboarding-surface.spec.tsx` pins the primitive: body portal around the content, mask/stage class presence, `#root` inert held for exactly the mount lifetime, and the no-`#root` composition. `packages/client/ui-settings/tests/settings-root.spec.tsx` pins the inverted shell contract: no takeover chrome and no inert while a mounted step renders nothing. `apps/web/tests/onboarding-deepseek-config.e2e.ts` gains the defect's assembled regression pin: a configured world reloads while every `settings.describe` response is held open at the browser's network boundary — widening the steps' deciding window from loopback-invisible to hundreds of milliseconds, which is what keeps the assertions non-vacuous — and an 8 ms in-page sampler proves the takeover chrome never mounts and `#root` never turns inert. The file's existing scenarios and the step specs (`ui-settings-general`, `ui-models`) pass unchanged — the mask selector and geometry pins survive because the stylesheet moved verbatim. diff --git a/.agents/notes/implemented/bug-fix/2026-08-06-onboarding-step-owned-takeover-chrome.zh.md b/.agents/notes/implemented/bug-fix/2026-08-06-onboarding-step-owned-takeover-chrome.zh.md index 37bbb47f6b..548285d285 100644 --- a/.agents/notes/implemented/bug-fix/2026-08-06-onboarding-step-owned-takeover-chrome.zh.md +++ b/.agents/notes/implemented/bug-fix/2026-08-06-onboarding-step-owned-takeover-chrome.zh.md @@ -32,4 +32,4 @@ ## 测试 -`packages/client/ui-primitives/tests/onboarding-surface.spec.tsx` 钉住原语行为：内容外的 body portal、遮罩／展示层类名存在、`#root` 的 `inert` 恰好持续挂载生命周期，以及无 `#root` 的组合。`packages/client/ui-settings/tests/settings-root.spec.tsx` 钉住反转后的外壳契约：已挂载步骤什么都不渲染时，无接管界面框架、无 inert。既有的步骤 spec（`ui-settings-general`、`ui-models`）与整装的 `apps/web/tests/onboarding-deepseek-config.e2e.ts` 场景原样通过——样式表逐字迁移，遮罩选择器与几何钉子得以幸存。 +`packages/client/ui-primitives/tests/onboarding-surface.spec.tsx` 钉住原语行为：内容外的 body portal、遮罩／展示层类名存在、`#root` 的 `inert` 恰好持续挂载生命周期，以及无 `#root` 的组合。`packages/client/ui-settings/tests/settings-root.spec.tsx` 钉住反转后的外壳契约：已挂载步骤什么都不渲染时，无接管界面框架、无 inert。`apps/web/tests/onboarding-deepseek-config.e2e.ts` 新增本缺陷的整装回归钉：已配置世界刷新页面，同时在浏览器网络边界扣住所有 `settings.describe` 响应——把步骤的判定窗口从 loopback 下不可见拉宽到数百毫秒，这正是断言保持非空洞的关键——页内 8ms 采样器证明接管界面框架从未挂载、`#root` 从未变为 inert。该文件的既有场景与步骤 spec（`ui-settings-general`、`ui-models`）原样通过——样式表逐字迁移，遮罩选择器与几何钉子得以幸存。 diff --git a/apps/web/tests/onboarding-deepseek-config.e2e.ts b/apps/web/tests/onboarding-deepseek-config.e2e.ts index a5cb2d1cb0..85eb76e3e3 100644 --- a/apps/web/tests/onboarding-deepseek-config.e2e.ts +++ b/apps/web/tests/onboarding-deepseek-config.e2e.ts @@ -167,9 +167,14 @@ describe.skipIf(MODE === 'record')('web e2e: first-run DeepSeek credential setup // (welcome acknowledged, credential configured), yet each must LOAD its // private join before it can decide not to show. The chrome lives inside // the step (OnboardingSurface), so the deciding window paints and blocks - // nothing. Holding the join's settings.describe response widens that - // window from loopback-invisible to hundreds of milliseconds — without - // the hold, the assertions below would pass vacuously. + // nothing. Holding settings.describe widens that window from loopback + // RTT scale to a deterministic hundreds of milliseconds, removing all + // timing dependence from the sampler assertions below. + // + // The sampler init script persists across this shared page's later + // navigations (init scripts re-run per navigation); that stays harmless + // because no later scenario in this file legitimately shows the + // takeover, and only this test reads __takeoverSightings. await page.addInitScript(() => { const sightings: string[] = [] ;(window as unknown as { __takeoverSightings: string[] }).__takeoverSightings = sightings @@ -180,13 +185,17 @@ describe.skipIf(MODE === 'record')('web e2e: first-run DeepSeek credential setup if (document.getElementById('root')?.inert === true) sightings.push('inert') }, 8) }) - let releaseDescribe = (): void => {} - const held = new Promise((resolve) => { releaseDescribe = resolve }) - let gated = false + // EVERY settings.describe issued before the release is held — not just + // the first — so the pin cannot silently collapse back to loopback + // timing if a second boot-time consumer of the join ever appears. + let released = false + const heldRoutes: Array<() => void> = [] + const releaseDescribe = (): void => { + released = true + for (const resolve of heldRoutes.splice(0)) resolve() + } await page.route('**/api/settings.describe', async (route) => { - if (gated) { await route.continue(); return } - gated = true - await held + if (!released) await new Promise((resolve) => { heldRoutes.push(resolve) }) await route.continue() }) const warningsBefore = tripwire.warnings.length diff --git a/packages/client/ui-settings/package.json b/packages/client/ui-settings/package.json index 6fa2fdc8cb..02f1c74fbc 100644 --- a/packages/client/ui-settings/package.json +++ b/packages/client/ui-settings/package.json @@ -43,8 +43,7 @@ "@deepseek-ai/dsh-client-ui-slots": "^0.0.1", "@deepseek-ai/dsh-invariants": "^0.0.1", "cordis": "^4.0.0-rc.7", - "react": "^18.2.0", - "react-dom": "^18.2.0" + "react": "^18.2.0" }, "devDependencies": { "@deepseek-ai/dsh-client-locale": "workspace:^", @@ -53,11 +52,9 @@ "@deepseek-ai/dsh-client-ui-sidebar": "workspace:^", "@deepseek-ai/dsh-client-ui-slots": "workspace:^", "@deepseek-ai/dsh-invariants": "workspace:^", - "@types/react-dom": "~18.3.0", "@types/react": "~18.3.1", "cordis": "^4.0.0-rc.7", - "react": "^18.2.0", - "react-dom": "^18.2.0" + "react": "^18.2.0" }, "files": [ "lib/index.js", diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index aaaed3c423..83d8e0a0fe 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -1922,18 +1922,12 @@ importers: '@types/react': specifier: ~18.3.1 version: 18.3.31 - '@types/react-dom': - specifier: ~18.3.0 - version: 18.3.7(@types/react@18.3.31) cordis: specifier: ^4.0.0-rc.7 version: link:../../../vendor/cordis react: specifier: ^18.2.0 version: 18.3.1 - react-dom: - specifier: ^18.2.0 - version: 18.3.1(react@18.3.1) packages/client/ui-settings-general: dependencies: