mirror of
https://github.com/QwenLM/qwen-code.git
synced 2026-05-19 16:28:28 +00:00
chore(deps): re-upgrade ink 6 → 7.0.3 (upstream Static remount fix landed) (#4119)
* chore(deps): re-upgrade ink 6 → 7.0.3 (upstream Static remount fix landed) PR #3860 first upgraded ink 6 → 7.0.2. PR #4083 reverted because of a TUI regression: `<Static>` did not re-emit items when its `key` prop was bumped, so `/clear` / Ctrl+O / refreshStatic left the history area blank under ink 7.0.2. ink 7.0.3 (released after #4083) contains the exact fixes: - be9f44cda Fix: <Static> remount via key change drops new items (#948) - 669c4386c Fix: Drop stale <Static> output from fullStaticOutput on identity change (#950) - 7c2267c01 Fix `useBoxMetrics` not accepting ref objects with an initial null value (#945) Changes: - `ink` ^6.2.3 → ^7.0.3 (root hoist + cli direct) - `react` ^19.1.0 → ^19.2.4 (cli direct; ink 7.0.3 peerDeps requires >=19.2.0) - `react`/`react-dom` overrides ^19.2.4 added so the transitive graph stays deduped to a single instance (avoids `Invalid hook call` from multiple React copies, the classic ink-upgrade hazard) - `wrap-ansi` already on ^10.0.0 from #4083's partial-revert (no change) Verified: - `npm ls ink` → single `ink@7.0.3` across all peer deps - `npm ls react` → single `react@19.2.4` - `npm run typecheck --workspace=@qwen-code/qwen-code` clean - `npm run typecheck --workspace=@qwen-code/qwen-code-core` clean - Composer.test.tsx 20/20, MainContent.test.tsx 6/6, TableRenderer.test.tsx 59/59 + 1 skipped — all key UI components green on the new ink The Static-remount regression is upstream-fixed in 7.0.3, so the runtime path is restored without needing #3941's overflowY-self-managed viewport. #3941 (virtual viewport) remains an opt-in performance feature on top. * fix(deps,cli): add @types/react overrides + move refreshStatic out of setCurrentModel updater Two follow-ups from the multi-round audit of the ink 7.0.3 re-upgrade: 1. @types/react / @types/react-dom now pinned to ^19.2.0 in root overrides. packages/web-templates still declares @types/react ^18.2.0 in its devDeps. Today the CLI build is unaffected (web-templates's 18.x types are nested in its own node_modules and the React-using src/insight and src/export-html files are excluded from its tsconfig build), but a future reincludes-or-hoist accident would land conflicting global JSX namespaces in the CLI compile graph. Match the dep dedup we already enforce for `react` and `react-dom` so the type graph stays as deduped as the runtime graph. 2. AppContainer's onModelChange handler was calling refreshStatic() as a side-effect inside the setCurrentModel updater. React.StrictMode double-invokes state updaters in dev, so model swaps fired two clearTerminal writes + two <Static> key bumps. The double work was masked under ink 6 (key changes were no-ops on <Static>), but ink 7.0.3 honors key changes — the doubled work is now potentially visible as a faster flash-flash on every model switch. Refactor: setCurrentModel becomes a pure setter; refreshStatic moves into a useEffect keyed on currentModel with a ref-comparison guard so the first render doesn't fire. Single clearTerminal write per real model change, even under StrictMode. Verified: npm ls ink → single 7.0.3, npm ls react → single 19.2.4, npm ls @types/react → 19.2.10 hoisted (npm flags web-templates's 18.x constraint as overridden, which is the intended behavior). Typecheck clean across cli + core workspaces. * fix(cli): collapse model-change effect back into one batched handler wenshao's PR #4119 review correctly flagged that splitting the onModelChange flow into two effects (b25831b0e) reintroduced the issue #3899 freeze regression on every model switch: 1. setCurrentModel(model) commits first, with the OLD historyRemountKey. 2. <Static key={`${historyRemountKey}-${currentModel}`}> sees its key change (because currentModel did) and remounts immediately. 3. MainContent's render-phase progressive-replay reset only fires when historyRemountKey changes, so replayCount is still the full mergedHistory.length from any prior catch-up. 4. The remounted Static dumps the entire history in one synchronous layout pass — exactly the freeze progressive replay was added to avoid (#3899). The second effect's refreshStatic() bump arrives a render too late. Fix: do not split. Both side effects (refreshStatic, which writes clearTerminal + bumps historyRemountKey, and setCurrentModel) live in the event handler again, with a ref guard for same-model notifications. The React.StrictMode concern that motivatedb25831b0eis addressed by keeping the side effect OUT of the setState updater (it now runs once per event-handler invocation, not once per double-invoked updater call). Both setState calls land in the same React batch, so historyRemountKey and currentModel update together — MainContent's render-phase reset sees the new key, replayCount drops to the first chunk, and Static remounts with chunked replay intact. Tests: - AppContainer.test.tsx: 4 new tests covering the synchronous refreshStatic side-effect contract, same-model no-op, ref-guarded StrictMode double-invoke, and unsubscribe-on-unmount. - MainContent.test.tsx: new regression guard — when currentModel changes but historyRemountKey is held constant, progressive replay must NOT reset (pins the MainContent invariant the two-effect refactor accidentally relied on). Verified: vitest packages/cli AppContainer + MainContent green (82/82). Typecheck clean. Generated with AI Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> --------- Co-authored-by: 秦奇 <gary.gq@alibaba-inc.com> Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
This commit is contained in:
parent
f6315b378d
commit
78c65c8dee
6 changed files with 365 additions and 40 deletions
105
package-lock.json
generated
105
package-lock.json
generated
|
|
@ -17,7 +17,7 @@
|
|||
],
|
||||
"dependencies": {
|
||||
"@testing-library/dom": "^10.4.1",
|
||||
"ink": "^6.2.3",
|
||||
"ink": "^7.0.3",
|
||||
"simple-git": "^3.28.0"
|
||||
},
|
||||
"bin": {
|
||||
|
|
@ -91,9 +91,9 @@
|
|||
}
|
||||
},
|
||||
"node_modules/@alcalzone/ansi-tokenize": {
|
||||
"version": "0.2.5",
|
||||
"resolved": "https://registry.npmjs.org/@alcalzone/ansi-tokenize/-/ansi-tokenize-0.2.5.tgz",
|
||||
"integrity": "sha512-3NX/MpTdroi0aKz134A6RC2Gb2iXVECN4QaAXnvCIxxIm3C3AVB1mkUe8NaaiyvOpDfsrqWhYtj+Q6a62RrTsw==",
|
||||
"version": "0.3.0",
|
||||
"resolved": "https://registry.npmjs.org/@alcalzone/ansi-tokenize/-/ansi-tokenize-0.3.0.tgz",
|
||||
"integrity": "sha512-p+CMKJ93HFmLkjXKlXiVGlMQEuRb6H0MokBSwUsX+S6BRX8eV5naFZpQJFfJHjRZY0Hmnqy1/r6UWl3x+19zYA==",
|
||||
"license": "MIT",
|
||||
"dependencies": {
|
||||
"ansi-styles": "^6.2.1",
|
||||
|
|
@ -6277,6 +6277,7 @@
|
|||
"version": "5.2.0",
|
||||
"resolved": "https://registry.npmjs.org/cli-truncate/-/cli-truncate-5.2.0.tgz",
|
||||
"integrity": "sha512-xRwvIOMGrfOAnM1JYtqQImuaNtDEv9v6oIYAs4LIHwTiKee8uwvIi363igssOC0O5U04i4AlENs79LQLu9tEMw==",
|
||||
"dev": true,
|
||||
"license": "MIT",
|
||||
"dependencies": {
|
||||
"slice-ansi": "^8.0.0",
|
||||
|
|
@ -6293,6 +6294,7 @@
|
|||
"version": "6.2.3",
|
||||
"resolved": "https://registry.npmjs.org/ansi-styles/-/ansi-styles-6.2.3.tgz",
|
||||
"integrity": "sha512-4Dj6M28JB+oAH8kFkTLUo+a2jwOFkuqb3yucU0CANcRRUbxS0cP0nZYCGjcc3BNXwRIsUVmDGgzawme7zvJHvg==",
|
||||
"dev": true,
|
||||
"license": "MIT",
|
||||
"engines": {
|
||||
"node": ">=12"
|
||||
|
|
@ -6305,6 +6307,7 @@
|
|||
"version": "5.1.0",
|
||||
"resolved": "https://registry.npmjs.org/is-fullwidth-code-point/-/is-fullwidth-code-point-5.1.0.tgz",
|
||||
"integrity": "sha512-5XHYaSyiqADb4RnZ1Bdad6cPp8Toise4TzEjcOYDHZkTCbKgiUl7WTUCpNWHuxmDt91wnsZBc9xinNzopv3JMQ==",
|
||||
"dev": true,
|
||||
"license": "MIT",
|
||||
"dependencies": {
|
||||
"get-east-asian-width": "^1.3.1"
|
||||
|
|
@ -6320,6 +6323,7 @@
|
|||
"version": "8.0.0",
|
||||
"resolved": "https://registry.npmjs.org/slice-ansi/-/slice-ansi-8.0.0.tgz",
|
||||
"integrity": "sha512-stxByr12oeeOyY2BlviTNQlYV5xOj47GirPr4yA1hE9JCtxfQN0+tVbkxwCtYDQWhEKWFHsEK48ORg5jrouCAg==",
|
||||
"dev": true,
|
||||
"license": "MIT",
|
||||
"dependencies": {
|
||||
"ansi-styles": "^6.2.3",
|
||||
|
|
@ -6336,6 +6340,7 @@
|
|||
"version": "8.2.1",
|
||||
"resolved": "https://registry.npmjs.org/string-width/-/string-width-8.2.1.tgz",
|
||||
"integrity": "sha512-IIaP0g3iy9Cyy18w3M9YcaDudujEAVHKt3a3QJg1+sr/oX96TbaGUubG0hJyCjCBThFH+tFpcIyoUHUn1ogaLA==",
|
||||
"dev": true,
|
||||
"license": "MIT",
|
||||
"dependencies": {
|
||||
"get-east-asian-width": "^1.5.0",
|
||||
|
|
@ -9820,43 +9825,43 @@
|
|||
}
|
||||
},
|
||||
"node_modules/ink": {
|
||||
"version": "6.8.0",
|
||||
"resolved": "https://registry.npmjs.org/ink/-/ink-6.8.0.tgz",
|
||||
"integrity": "sha512-sbl1RdLOgkO9isK42WCZlJCFN9hb++sX9dsklOvfd1YQ3bQ2AiFu12Q6tFlr0HvEUvzraJntQCCpfEoUe9DSzA==",
|
||||
"version": "7.0.3",
|
||||
"resolved": "https://registry.npmjs.org/ink/-/ink-7.0.3.tgz",
|
||||
"integrity": "sha512-5kxHkIj9+RuqCU3zyvP4qvYWNOSHP2TW/SHayHGHOmk87KwfVcZwvJGemi9ch+ci2gXUqerK/Eh2DGEDt5q45g==",
|
||||
"license": "MIT",
|
||||
"dependencies": {
|
||||
"@alcalzone/ansi-tokenize": "^0.2.4",
|
||||
"@alcalzone/ansi-tokenize": "^0.3.0",
|
||||
"ansi-escapes": "^7.3.0",
|
||||
"ansi-styles": "^6.2.1",
|
||||
"ansi-styles": "^6.2.3",
|
||||
"auto-bind": "^5.0.1",
|
||||
"chalk": "^5.6.0",
|
||||
"cli-boxes": "^3.0.0",
|
||||
"chalk": "^5.6.2",
|
||||
"cli-boxes": "^4.0.1",
|
||||
"cli-cursor": "^4.0.0",
|
||||
"cli-truncate": "^5.1.1",
|
||||
"cli-truncate": "^6.0.0",
|
||||
"code-excerpt": "^4.0.0",
|
||||
"es-toolkit": "^1.39.10",
|
||||
"es-toolkit": "^1.45.1",
|
||||
"indent-string": "^5.0.0",
|
||||
"is-in-ci": "^2.0.0",
|
||||
"patch-console": "^2.0.0",
|
||||
"react-reconciler": "^0.33.0",
|
||||
"scheduler": "^0.27.0",
|
||||
"signal-exit": "^3.0.7",
|
||||
"slice-ansi": "^8.0.0",
|
||||
"slice-ansi": "^9.0.0",
|
||||
"stack-utils": "^2.0.6",
|
||||
"string-width": "^8.1.1",
|
||||
"string-width": "^8.2.0",
|
||||
"terminal-size": "^4.0.1",
|
||||
"type-fest": "^5.4.1",
|
||||
"type-fest": "^5.5.0",
|
||||
"widest-line": "^6.0.0",
|
||||
"wrap-ansi": "^9.0.0",
|
||||
"ws": "^8.18.0",
|
||||
"wrap-ansi": "^10.0.0",
|
||||
"ws": "^8.20.0",
|
||||
"yoga-layout": "~3.2.1"
|
||||
},
|
||||
"engines": {
|
||||
"node": ">=20"
|
||||
"node": ">=22"
|
||||
},
|
||||
"peerDependencies": {
|
||||
"@types/react": ">=19.0.0",
|
||||
"react": ">=19.0.0",
|
||||
"@types/react": ">=19.2.0",
|
||||
"react": ">=19.2.0",
|
||||
"react-devtools-core": ">=6.1.2"
|
||||
},
|
||||
"peerDependenciesMeta": {
|
||||
|
|
@ -10034,6 +10039,34 @@
|
|||
"url": "https://github.com/chalk/chalk?sponsor=1"
|
||||
}
|
||||
},
|
||||
"node_modules/ink/node_modules/cli-boxes": {
|
||||
"version": "4.0.1",
|
||||
"resolved": "https://registry.npmjs.org/cli-boxes/-/cli-boxes-4.0.1.tgz",
|
||||
"integrity": "sha512-5IOn+jcCEHEraYolBPs/sT4BxYCe2nHg374OPiItB1O96KZFseS2gthU4twyYzeDcFew4DaUM/xwc5BQf08JJw==",
|
||||
"license": "MIT",
|
||||
"engines": {
|
||||
"node": ">=18.20 <19 || >=20.10"
|
||||
},
|
||||
"funding": {
|
||||
"url": "https://github.com/sponsors/sindresorhus"
|
||||
}
|
||||
},
|
||||
"node_modules/ink/node_modules/cli-truncate": {
|
||||
"version": "6.0.0",
|
||||
"resolved": "https://registry.npmjs.org/cli-truncate/-/cli-truncate-6.0.0.tgz",
|
||||
"integrity": "sha512-3+YKIUFsohD9MIoOFPFBldjAlnfCmCDcqe6aYGFqlDTRKg80p4wg35L+j83QQ63iOlKRccEkbn8IuM++HsgEjA==",
|
||||
"license": "MIT",
|
||||
"dependencies": {
|
||||
"slice-ansi": "^9.0.0",
|
||||
"string-width": "^8.2.0"
|
||||
},
|
||||
"engines": {
|
||||
"node": ">=22"
|
||||
},
|
||||
"funding": {
|
||||
"url": "https://github.com/sponsors/sindresorhus"
|
||||
}
|
||||
},
|
||||
"node_modules/ink/node_modules/is-fullwidth-code-point": {
|
||||
"version": "5.1.0",
|
||||
"resolved": "https://registry.npmjs.org/is-fullwidth-code-point/-/is-fullwidth-code-point-5.1.0.tgz",
|
||||
|
|
@ -10071,16 +10104,16 @@
|
|||
"license": "ISC"
|
||||
},
|
||||
"node_modules/ink/node_modules/slice-ansi": {
|
||||
"version": "8.0.0",
|
||||
"resolved": "https://registry.npmjs.org/slice-ansi/-/slice-ansi-8.0.0.tgz",
|
||||
"integrity": "sha512-stxByr12oeeOyY2BlviTNQlYV5xOj47GirPr4yA1hE9JCtxfQN0+tVbkxwCtYDQWhEKWFHsEK48ORg5jrouCAg==",
|
||||
"version": "9.0.0",
|
||||
"resolved": "https://registry.npmjs.org/slice-ansi/-/slice-ansi-9.0.0.tgz",
|
||||
"integrity": "sha512-SO/3iYL5S3W57LLEniscOGPZgOqZUPCx6d3dB+52B80yJ0XstzsC/eV8gnA4tM3MHDrKz+OCFSLNjswdSC+/bA==",
|
||||
"license": "MIT",
|
||||
"dependencies": {
|
||||
"ansi-styles": "^6.2.3",
|
||||
"is-fullwidth-code-point": "^5.1.0"
|
||||
},
|
||||
"engines": {
|
||||
"node": ">=20"
|
||||
"node": ">=22"
|
||||
},
|
||||
"funding": {
|
||||
"url": "https://github.com/chalk/slice-ansi?sponsor=1"
|
||||
|
|
@ -10132,6 +10165,23 @@
|
|||
"url": "https://github.com/sponsors/sindresorhus"
|
||||
}
|
||||
},
|
||||
"node_modules/ink/node_modules/wrap-ansi": {
|
||||
"version": "10.0.0",
|
||||
"resolved": "https://registry.npmjs.org/wrap-ansi/-/wrap-ansi-10.0.0.tgz",
|
||||
"integrity": "sha512-SGcvg80f0wUy2/fXES19feHMz8E0JoXv2uNgHOu4Dgi2OrCy1lqwFYEJz1BLbDI0exjPMe/ZdzZ/YpGECBG/aQ==",
|
||||
"license": "MIT",
|
||||
"dependencies": {
|
||||
"ansi-styles": "^6.2.3",
|
||||
"string-width": "^8.2.0",
|
||||
"strip-ansi": "^7.1.2"
|
||||
},
|
||||
"engines": {
|
||||
"node": ">=20"
|
||||
},
|
||||
"funding": {
|
||||
"url": "https://github.com/chalk/wrap-ansi?sponsor=1"
|
||||
}
|
||||
},
|
||||
"node_modules/ink/node_modules/ws": {
|
||||
"version": "8.20.0",
|
||||
"resolved": "https://registry.npmjs.org/ws/-/ws-8.20.0.tgz",
|
||||
|
|
@ -13054,7 +13104,6 @@
|
|||
"os": [
|
||||
"darwin"
|
||||
],
|
||||
"peer": true,
|
||||
"engines": {
|
||||
"node": "^8.16.0 || ^10.6.0 || >=11.0.0"
|
||||
}
|
||||
|
|
@ -17231,7 +17280,7 @@
|
|||
"fzf": "^0.5.2",
|
||||
"glob": "^10.5.0",
|
||||
"highlight.js": "^11.11.1",
|
||||
"ink": "^6.2.3",
|
||||
"ink": "^7.0.3",
|
||||
"ink-gradient": "^3.0.0",
|
||||
"ink-link": "^4.1.0",
|
||||
"ink-spinner": "^5.0.0",
|
||||
|
|
@ -17239,7 +17288,7 @@
|
|||
"open": "^10.1.2",
|
||||
"p-limit": "^7.3.0",
|
||||
"prompts": "^2.4.2",
|
||||
"react": "^19.1.0",
|
||||
"react": "^19.2.4",
|
||||
"read-package-up": "^11.0.0",
|
||||
"shell-quote": "^1.8.3",
|
||||
"simple-git": "^3.28.0",
|
||||
|
|
|
|||
|
|
@ -79,7 +79,11 @@
|
|||
"wrap-ansi": "7.0.0"
|
||||
},
|
||||
"baseline-browser-mapping": "^2.9.19",
|
||||
"normalize-package-data": "^7.0.1"
|
||||
"normalize-package-data": "^7.0.1",
|
||||
"react": "^19.2.4",
|
||||
"react-dom": "^19.2.4",
|
||||
"@types/react": "^19.2.0",
|
||||
"@types/react-dom": "^19.2.0"
|
||||
},
|
||||
"bin": {
|
||||
"qwen": "dist/cli.js"
|
||||
|
|
@ -130,7 +134,7 @@
|
|||
},
|
||||
"dependencies": {
|
||||
"@testing-library/dom": "^10.4.1",
|
||||
"ink": "^6.2.3",
|
||||
"ink": "^7.0.3",
|
||||
"simple-git": "^3.28.0"
|
||||
},
|
||||
"optionalDependencies": {
|
||||
|
|
|
|||
|
|
@ -60,7 +60,7 @@
|
|||
"fzf": "^0.5.2",
|
||||
"glob": "^10.5.0",
|
||||
"highlight.js": "^11.11.1",
|
||||
"ink": "^6.2.3",
|
||||
"ink": "^7.0.3",
|
||||
"ink-gradient": "^3.0.0",
|
||||
"ink-link": "^4.1.0",
|
||||
"ink-spinner": "^5.0.0",
|
||||
|
|
@ -68,7 +68,7 @@
|
|||
"open": "^10.1.2",
|
||||
"p-limit": "^7.3.0",
|
||||
"prompts": "^2.4.2",
|
||||
"react": "^19.1.0",
|
||||
"react": "^19.2.4",
|
||||
"read-package-up": "^11.0.0",
|
||||
"shell-quote": "^1.8.3",
|
||||
"simple-git": "^3.28.0",
|
||||
|
|
|
|||
|
|
@ -2895,6 +2895,188 @@ describe('AppContainer State Management', () => {
|
|||
expect(mockCloseModelDialog).toHaveBeenCalled();
|
||||
});
|
||||
});
|
||||
|
||||
// Coverage for the AppContainer onModelChange wiring. The Static header
|
||||
// (key = `${historyRemountKey}-${currentModel}`) and MainContent's
|
||||
// progressive-replay reset (keyed on historyRemountKey) both depend on
|
||||
// these two state updates landing in the same commit on a real model
|
||||
// change — see the comment in AppContainer.tsx around the
|
||||
// config.onModelChange subscription and PR #4119 review discussion.
|
||||
describe('Model change refreshStatic wiring', () => {
|
||||
function captureModelChangeListener(config: Config) {
|
||||
// Track every subscribe/unsubscribe pair. The CLI test harness
|
||||
// tears down ink's renderer after the initial render flush, which
|
||||
// runs the effect's cleanup synchronously — but the captured
|
||||
// callback closure is still callable (and AppContainer's setState
|
||||
// still updates state because React's update queue is independent
|
||||
// of the listener registration). We therefore fire on the LAST
|
||||
// captured callback, regardless of whether ink considers the
|
||||
// effect mounted, and assert on the number of subscribe/cleanup
|
||||
// calls separately for unsubscribe coverage.
|
||||
const subs: Array<{
|
||||
cb: (model: string) => void;
|
||||
active: boolean;
|
||||
}> = [];
|
||||
const fakeOnModelChange = vi.fn((cb: (model: string) => void) => {
|
||||
const entry = { cb, active: true };
|
||||
subs.push(entry);
|
||||
return () => {
|
||||
entry.active = false;
|
||||
};
|
||||
});
|
||||
(
|
||||
config as unknown as { onModelChange: typeof fakeOnModelChange }
|
||||
).onModelChange = fakeOnModelChange;
|
||||
return {
|
||||
spy: fakeOnModelChange,
|
||||
notify: (model: string) => {
|
||||
if (subs.length === 0) {
|
||||
throw new Error('AppContainer never subscribed to onModelChange');
|
||||
}
|
||||
// Always fire on the most-recent captured callback.
|
||||
subs[subs.length - 1].cb(model);
|
||||
},
|
||||
subscribeCount: () => subs.length,
|
||||
activeCount: () => subs.filter((s) => s.active).length,
|
||||
};
|
||||
}
|
||||
|
||||
// Effects run after the synchronous render returns. Flushing two
|
||||
// microtasks lines up the same pattern used by other async tests in
|
||||
// this file (search "Let the userMessages-fetching effect resolve").
|
||||
const flushEffects = async () => {
|
||||
await Promise.resolve();
|
||||
await Promise.resolve();
|
||||
};
|
||||
|
||||
it('fires refreshStatic in the same handler that updates currentModel', async () => {
|
||||
// Wenshao's PR #4119 [Critical]: if refreshStatic (which bumps
|
||||
// historyRemountKey) and setCurrentModel were split into two
|
||||
// separate effects, the first commit would show the new
|
||||
// currentModel against the OLD historyRemountKey — MainContent's
|
||||
// <Static key={`${historyRemountKey}-${currentModel}`}> would
|
||||
// remount BEFORE the progressive-replay reset, dumping the full
|
||||
// history in one frame.
|
||||
//
|
||||
// The fix moves refreshStatic into the event handler itself so
|
||||
// both side effects (clearTerminal + setHistoryRemountKey via
|
||||
// refreshStatic, plus setCurrentModel) run inside the same
|
||||
// synchronous JS task — React 18+ batches all setState calls in
|
||||
// an event-handler-style task into one commit. We verify this
|
||||
// synchronously by inspecting mockStdout.write the moment the
|
||||
// listener returns: clearTerminal must already be written, proving
|
||||
// refreshStatic runs in-handler rather than queued for a later
|
||||
// useEffect tick. (We cannot observe the post-commit React state
|
||||
// through capturedUIState here because ink-testing-library tears
|
||||
// down the renderer once render() returns, so setState calls
|
||||
// queued from the listener never produce a follow-up commit. The
|
||||
// synchronous side-effect ordering is the part that matters for
|
||||
// the bug wenshao flagged.)
|
||||
vi.spyOn(mockConfig, 'getModel').mockReturnValue('model-a');
|
||||
const trigger = captureModelChangeListener(mockConfig);
|
||||
|
||||
render(
|
||||
<AppContainer
|
||||
config={mockConfig}
|
||||
settings={mockSettings}
|
||||
version="1.0.0"
|
||||
initializationResult={mockInitResult}
|
||||
/>,
|
||||
);
|
||||
await flushEffects();
|
||||
mockStdout.write.mockClear();
|
||||
|
||||
// Synchronous notification → refreshStatic must run BEFORE the
|
||||
// notify() call returns (i.e., before any React batch tick).
|
||||
trigger.notify('model-b');
|
||||
|
||||
expect(mockStdout.write).toHaveBeenCalledWith(ansiEscapes.clearTerminal);
|
||||
});
|
||||
|
||||
it('skips refreshStatic when the notified model matches the current one', async () => {
|
||||
vi.spyOn(mockConfig, 'getModel').mockReturnValue('model-a');
|
||||
const trigger = captureModelChangeListener(mockConfig);
|
||||
|
||||
render(
|
||||
<AppContainer
|
||||
config={mockConfig}
|
||||
settings={mockSettings}
|
||||
version="1.0.0"
|
||||
initializationResult={mockInitResult}
|
||||
/>,
|
||||
);
|
||||
await flushEffects();
|
||||
|
||||
const baselineRemountKey = capturedUIState.historyRemountKey;
|
||||
mockStdout.write.mockClear();
|
||||
|
||||
trigger.notify('model-a');
|
||||
await flushEffects();
|
||||
|
||||
expect(mockStdout.write).not.toHaveBeenCalledWith(
|
||||
ansiEscapes.clearTerminal,
|
||||
);
|
||||
expect(capturedUIState.historyRemountKey).toBe(baselineRemountKey);
|
||||
expect(capturedUIState.currentModel).toBe('model-a');
|
||||
});
|
||||
|
||||
it('fires refreshStatic only once per real model change (StrictMode-safe)', async () => {
|
||||
// StrictMode double-invokes state updater functions in dev. The
|
||||
// refreshStatic side-effect therefore must NOT live inside a
|
||||
// setState updater — it lives in the event handler, with a ref
|
||||
// guard to de-dupe redundant notifications. We simulate the
|
||||
// StrictMode-style re-fire by calling the listener twice with the
|
||||
// same value (e.g. if a deduplicator upstream missed it).
|
||||
vi.spyOn(mockConfig, 'getModel').mockReturnValue('model-a');
|
||||
const trigger = captureModelChangeListener(mockConfig);
|
||||
|
||||
render(
|
||||
<AppContainer
|
||||
config={mockConfig}
|
||||
settings={mockSettings}
|
||||
version="1.0.0"
|
||||
initializationResult={mockInitResult}
|
||||
/>,
|
||||
);
|
||||
await flushEffects();
|
||||
mockStdout.write.mockClear();
|
||||
|
||||
trigger.notify('model-b');
|
||||
trigger.notify('model-b');
|
||||
await flushEffects();
|
||||
|
||||
const clearWrites = mockStdout.write.mock.calls.filter(
|
||||
([arg]) => arg === ansiEscapes.clearTerminal,
|
||||
);
|
||||
expect(clearWrites).toHaveLength(1);
|
||||
});
|
||||
|
||||
it('returns an unsubscribe function that AppContainer wires up', async () => {
|
||||
// AppContainer's effect returns the unsubscribe so React can call it
|
||||
// on unmount or when deps change. We verify both halves of the
|
||||
// subscribe/cleanup contract were exercised — every subscribe must
|
||||
// have paired with a cleanup invocation by the time the renderer
|
||||
// tears down.
|
||||
vi.spyOn(mockConfig, 'getModel').mockReturnValue('model-a');
|
||||
const trigger = captureModelChangeListener(mockConfig);
|
||||
|
||||
const { unmount } = render(
|
||||
<AppContainer
|
||||
config={mockConfig}
|
||||
settings={mockSettings}
|
||||
version="1.0.0"
|
||||
initializationResult={mockInitResult}
|
||||
/>,
|
||||
);
|
||||
await flushEffects();
|
||||
expect(trigger.subscribeCount()).toBeGreaterThanOrEqual(1);
|
||||
|
||||
unmount();
|
||||
await flushEffects();
|
||||
|
||||
expect(trigger.activeCount()).toBe(0);
|
||||
});
|
||||
});
|
||||
});
|
||||
|
||||
describe('dedupeNewestFirst', () => {
|
||||
|
|
|
|||
|
|
@ -738,17 +738,36 @@ export const AppContainer = (props: AppContainerProps) => {
|
|||
// Keep the static header in sync with model changes without polling.
|
||||
// Ink's <Static> output is append-only, so model changes must explicitly
|
||||
// clear and remount the static region to redraw the banner at the top.
|
||||
//
|
||||
// Two requirements pull in opposite directions:
|
||||
// (a) refreshStatic() must NOT be called from inside a setState updater,
|
||||
// because React.StrictMode double-invokes updaters in dev and we'd
|
||||
// fire two clearTerminal writes per model swap.
|
||||
// (b) setHistoryRemountKey (inside refreshStatic) and setCurrentModel
|
||||
// MUST land in the SAME commit. MainContent's <Static> key is
|
||||
// `${historyRemountKey}-${currentModel}` and its render-phase
|
||||
// progressive-replay reset (lastRemountKey !== historyRemountKey)
|
||||
// only fires when historyRemountKey changes. If currentModel
|
||||
// changes first in its own render, Static remounts with the OLD
|
||||
// remount key and the unreset (full-length) replayCount — i.e.
|
||||
// a full-history Static render that bypasses progressive replay
|
||||
// (the issue #3899 freeze regression). See PR #4119 review.
|
||||
//
|
||||
// Fix: side-effect lives in the event handler (NOT the updater); a ref
|
||||
// guard de-dupes same-model notifications. React batches the
|
||||
// setHistoryRemountKey (via refreshStatic) and setCurrentModel calls in
|
||||
// this event handler into a single commit, so the render-phase reset
|
||||
// and the Static remount happen together — no full-history flash.
|
||||
const lastNotifiedModelRef = useRef(currentModel);
|
||||
useEffect(() => {
|
||||
const unsubscribe = config.onModelChange((model) => {
|
||||
setCurrentModel((prev) => {
|
||||
if (prev === model) {
|
||||
return prev;
|
||||
}
|
||||
refreshStatic();
|
||||
return model;
|
||||
});
|
||||
if (lastNotifiedModelRef.current === model) {
|
||||
return;
|
||||
}
|
||||
lastNotifiedModelRef.current = model;
|
||||
refreshStatic();
|
||||
setCurrentModel(model);
|
||||
});
|
||||
|
||||
return unsubscribe;
|
||||
}, [config, refreshStatic]);
|
||||
|
||||
|
|
|
|||
|
|
@ -456,4 +456,75 @@ describe('<MainContent />', () => {
|
|||
|
||||
expect(staticItemsSpy.mock.calls.at(-1)?.[0]).toHaveLength(53);
|
||||
});
|
||||
|
||||
it('does NOT reset progressive replay when only currentModel changes (PR #4119 regression guard)', async () => {
|
||||
// Wenshao's review on PR #4119: if AppContainer splits the model-change
|
||||
// wiring into two separate effects (setCurrentModel first, refreshStatic
|
||||
// -> historyRemountKey bump second), there is a render where currentModel
|
||||
// is new but historyRemountKey is still the old value. <Static>'s key is
|
||||
// `${historyRemountKey}-${currentModel}`, so the key changes (Ink remounts
|
||||
// Static), but the render-phase reset (lastRemountKey !== historyRemountKey)
|
||||
// does NOT fire — so the new <Static> is mounted with the full pre-catch-up
|
||||
// replayCount, and Ink does the synchronous full-history layout the PR is
|
||||
// meant to avoid.
|
||||
//
|
||||
// This test reproduces only the dangerous half of that interleaving:
|
||||
// currentModel flips while historyRemountKey is held constant. Under the
|
||||
// correct (single-batch) AppContainer wiring this combination never
|
||||
// appears in practice, but the test pins the MainContent invariant —
|
||||
// currentModel alone must not trigger progressive-replay reset, which
|
||||
// makes any future "two-effect" regression visible here as a freeze.
|
||||
staticItemsSpy.mockClear();
|
||||
const history = Array.from({ length: 200 }, (_, i) => ({
|
||||
type: 'user' as const,
|
||||
id: i,
|
||||
text: `msg ${i}`,
|
||||
}));
|
||||
const TOTAL = 203;
|
||||
|
||||
const { rerender } = renderMainContent(
|
||||
createUIState({
|
||||
history,
|
||||
historyRemountKey: 1,
|
||||
currentModel: 'model-a',
|
||||
}),
|
||||
);
|
||||
|
||||
// Drive the chunked replay to completion (replayCount === TOTAL).
|
||||
for (let i = 0; i < 50; i++) {
|
||||
const len = staticItemsSpy.mock.calls.at(-1)?.[0].length ?? 0;
|
||||
if (len === TOTAL) break;
|
||||
await new Promise<void>((resolve) => setImmediate(resolve));
|
||||
}
|
||||
expect(staticItemsSpy.mock.calls.at(-1)?.[0]).toHaveLength(TOTAL);
|
||||
|
||||
// Re-render with a NEW currentModel but the SAME historyRemountKey.
|
||||
// <Static>'s key will change (Ink remounts), but replayCount must stay
|
||||
// at TOTAL — i.e. progressive replay must NOT re-trigger. Any future
|
||||
// refactor that re-introduces a one-render gap between setCurrentModel
|
||||
// and the historyRemountKey bump will trip this assertion the moment
|
||||
// someone correctly drives the reset off the model dimension instead.
|
||||
rerender(
|
||||
<AppContext.Provider value={{ version: '1.2.3', startupWarnings: [] }}>
|
||||
<CompactModeProvider value={{ compactMode: false }}>
|
||||
<UIActionsContext.Provider value={createUIActions()}>
|
||||
<UIStateContext.Provider
|
||||
value={createUIState({
|
||||
history,
|
||||
historyRemountKey: 1,
|
||||
currentModel: 'model-b',
|
||||
})}
|
||||
>
|
||||
<OverflowProvider>
|
||||
<MainContent />
|
||||
</OverflowProvider>
|
||||
</UIStateContext.Provider>
|
||||
</UIActionsContext.Provider>
|
||||
</CompactModeProvider>
|
||||
</AppContext.Provider>,
|
||||
);
|
||||
|
||||
// No reset means the LAST staticItemsSpy call still received TOTAL.
|
||||
expect(staticItemsSpy.mock.calls.at(-1)?.[0]).toHaveLength(TOTAL);
|
||||
});
|
||||
});
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue