Repository navigation
fix(desktop): dedupe error UI + wire iframe error listener #14
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from 2 commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,28 @@ | ||
| import { describe, expect, it } from 'vitest'; | ||
| import { readIframeErrorMessage } from './App'; | ||
|
|
||
| describe('readIframeErrorMessage', () => { | ||
| it('ignores iframe errors while a new generation is in progress', () => { | ||
| const data = { | ||
| __codesign: true, | ||
| type: 'IFRAME_ERROR', | ||
| kind: 'error', | ||
| message: 'old preview failed', | ||
| timestamp: Date.now(), | ||
| }; | ||
|
|
||
| expect(readIframeErrorMessage(true, data)).toBeNull(); | ||
| }); | ||
|
|
||
| it('returns the iframe error message when generation is idle', () => { | ||
| const data = { | ||
| __codesign: true, | ||
| type: 'IFRAME_ERROR', | ||
| kind: 'error', | ||
| message: 'current preview failed', | ||
| timestamp: Date.now(), | ||
| }; | ||
|
|
||
| expect(readIframeErrorMessage(false, data)).toBe('current preview failed'); | ||
| }); | ||
| }); |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,4 +1,5 @@ | ||
| import { useT } from '@open-codesign/i18n'; | ||
| import { isIframeErrorMessage } from '@open-codesign/runtime'; | ||
| import { useEffect, useMemo, useState } from 'react'; | ||
| import { CommandPalette } from './components/CommandPalette'; | ||
| import { PreviewPane } from './components/PreviewPane'; | ||
|
|
@@ -10,6 +11,11 @@ import { useKeyboard } from './hooks/useKeyboard'; | |
| import { Onboarding } from './onboarding'; | ||
| import { useCodesignStore } from './store'; | ||
|
|
||
| export function readIframeErrorMessage(isGenerating: boolean, data: unknown): string | null { | ||
| if (isGenerating || !isIframeErrorMessage(data)) return null; | ||
| return data.message; | ||
| } | ||
|
|
||
| export function App() { | ||
| const t = useT(); | ||
| const config = useCodesignStore((s) => s.config); | ||
|
|
@@ -30,6 +36,17 @@ export function App() { | |
| void loadConfig(); | ||
| }, [loadConfig]); | ||
|
|
||
| useEffect(() => { | ||
| const handler = (e: MessageEvent) => { | ||
| const state = useCodesignStore.getState(); | ||
| const message = readIframeErrorMessage(state.isGenerating, e.data); | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 这里少了对
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 这里直接消费 // App.tsx
// remove this listener and keep PreviewPane's onMessage handler only |
||
| if (message === null) return; | ||
| state.pushIframeError(message); | ||
| }; | ||
| window.addEventListener('message', handler); | ||
| return () => window.removeEventListener('message', handler); | ||
| }, []); | ||
|
|
||
| function submit(): void { | ||
| const trimmed = prompt.trim(); | ||
| if (!trimmed || isGenerating) return; | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,97 @@ | ||
| import type { OnboardingState } from '@open-codesign/shared'; | ||
| import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; | ||
| import { useCodesignStore } from './store'; | ||
|
|
||
| const READY_CONFIG: OnboardingState = { | ||
| hasKey: true, | ||
| provider: 'anthropic', | ||
| modelPrimary: 'claude-sonnet-4-6', | ||
| modelFast: 'claude-haiku-3', | ||
| baseUrl: null, | ||
| designSystem: null, | ||
| }; | ||
|
|
||
| const initialState = useCodesignStore.getState(); | ||
|
|
||
| function resetStore() { | ||
| useCodesignStore.setState({ | ||
| ...initialState, | ||
| messages: [], | ||
| previewHtml: null, | ||
| isGenerating: false, | ||
| errorMessage: null, | ||
| lastError: null, | ||
| config: READY_CONFIG, | ||
| configLoaded: true, | ||
| toastMessage: null, | ||
| iframeErrors: [], | ||
| toasts: [], | ||
| }); | ||
| } | ||
|
|
||
| beforeEach(() => { | ||
| resetStore(); | ||
| }); | ||
|
|
||
| afterEach(() => { | ||
| vi.unstubAllGlobals(); | ||
| }); | ||
|
|
||
| describe('useCodesignStore iframe error handling', () => { | ||
| it('clears stale iframe errors when starting a new generation', async () => { | ||
| let resolveGenerate: ((value: unknown) => void) | undefined; | ||
| const generate = vi.fn( | ||
| () => | ||
| new Promise((resolve) => { | ||
| resolveGenerate = resolve; | ||
| }), | ||
| ); | ||
|
|
||
| vi.stubGlobal('window', { | ||
| codesign: { | ||
| generate, | ||
| }, | ||
| }); | ||
|
|
||
| useCodesignStore.setState({ iframeErrors: ['old iframe error'] }); | ||
|
|
||
| const sendPromise = useCodesignStore.getState().sendPrompt({ prompt: 'make a landing page' }); | ||
|
|
||
| expect(useCodesignStore.getState().iframeErrors).toEqual([]); | ||
| expect(useCodesignStore.getState().isGenerating).toBe(true); | ||
|
|
||
| resolveGenerate?.({ | ||
| artifacts: [{ content: '<html></html>' }], | ||
| message: 'Done.', | ||
| }); | ||
| await sendPromise; | ||
|
|
||
| expect(generate).toHaveBeenCalledOnce(); | ||
| }); | ||
|
|
||
| it('deduplicates consecutive identical iframe errors', () => { | ||
| const { pushIframeError } = useCodesignStore.getState(); | ||
|
|
||
| pushIframeError('first'); | ||
| pushIframeError('first'); // duplicate — should be skipped | ||
| pushIframeError('second'); | ||
| pushIframeError('second'); // duplicate — should be skipped | ||
| pushIframeError('third'); | ||
|
|
||
| expect(useCodesignStore.getState().iframeErrors).toEqual(['first', 'second', 'third']); | ||
| }); | ||
|
|
||
| it('caps iframeErrors at 50 entries and drops the oldest when exceeded', () => { | ||
| const { pushIframeError } = useCodesignStore.getState(); | ||
|
|
||
| for (let i = 0; i < 55; i++) { | ||
| pushIframeError(`error-${i}`); | ||
| } | ||
|
|
||
| const errors = useCodesignStore.getState().iframeErrors; | ||
| expect(errors).toHaveLength(50); | ||
| // oldest (0-4) should have been shifted out; newest (5-54) remain | ||
| expect(errors[0]).toBe('error-5'); | ||
| expect(errors[49]).toBe('error-54'); | ||
| }); | ||
| }); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
readIframeErrorMessage()现在只返回data.message,把kind/source/lineno都丢掉了。当前 runtime 合同仍然会提供这些字段,所以这会把原本可定位的error: boom (foo.js:12)退化成裸 message,CanvasErrorBar的排障价值明显下降。建议在这里保留最少的上下文格式化。