Skip to content

Commit cf0b98c

Browse files
lmac-1stuartc
lmac-1
and
stuartc
authored
Address PR #4110 review feedback: relocate hook and add tests (#4278)
Moves useMonacoSync hook from assets/js/hooks (intended for Phoenix LiveView hooks only) to assets/js/log-viewer where it's co-located with its only consumer. Adds component tests that mock Monaco Editor and verify the race condition fix handles logs arriving before, during, and after editor initialization. Tests confirm the fix works when store updates arrive instantly via WebSocket. Includes monaco-editor mock infrastructure in vitest config to prevent module resolution errors during test runs. Co-authored-by: Stuart Corbishley <corbish@gmail.com>
1 parent ded6a62 commit cf0b98c

File tree

5 files changed

+254
-3
lines changed

5 files changed

+254
-3
lines changed

‎assets/js/log-viewer/component.tsx‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -4,10 +4,10 @@ import { createRoot } from 'react-dom/client';
44
import { useStore } from 'zustand';
55
import { useShallow } from 'zustand/react/shallow';
66

7-
import { useMonacoSync } from '../hooks/useMonacoSync';
87
import { type Monaco, MonacoEditor } from '../monaco';
98

109
import { createLogStore } from './store';
10+
import { useMonacoSync } from './useMonacoSync';
1111

1212
export function mount(
1313
el: HTMLElement,
@@ -85,7 +85,7 @@ const LogViewer = ({
8585
// Define a simple tokenizer for the language
8686
monaco.languages.setMonarchTokensProvider('openFnLogs', {
8787
tokenizer: {
88-
root: [[/^([A-Z\/]{2,4})/, 'logSource']],
88+
root: [[/^([A-Z/]{2,4})/, 'logSource']],
8989
},
9090
});
9191
}

assets/js/hooks/useMonacoSync.ts renamed to assets/js/log-viewer/useMonacoSync.ts

File renamed without changes.

assets/test/_mocks/monaco-editor.ts renamed to assets/test/__mocks__/monaco-editor.ts

File renamed without changes.

‎assets/test/log-viewer/component.test.tsx‎

Lines changed: 251 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,251 @@
1+
/**
2+
* LogViewer Component Tests
3+
*
4+
* Tests the LogViewer component focusing on Monaco Editor
5+
* initialization race conditions where logs arrive via WebSocket
6+
* before Monaco is fully ready.
7+
*
8+
* This addresses the race condition fixed in PR #4110 where logs
9+
* would disappear on browser refresh in the collaborative editor IDE.
10+
*/
11+
12+
import { screen, waitFor } from '@testing-library/react';
13+
import { beforeEach, describe, expect, test, vi } from 'vitest';
14+
15+
import { createLogStore, type LogLine } from '../../js/log-viewer/store';
16+
17+
// Mock @monaco-editor/react to prevent loading issues
18+
vi.mock('@monaco-editor/react', () => ({
19+
default: ({ value }: { value: string }) => (
20+
<div data-testid="monaco-editor" data-value={value}>
21+
{value || 'Loading...'}
22+
</div>
23+
),
24+
}));
25+
26+
// Mock the monaco module that LogViewer imports
27+
vi.mock('../../js/monaco', () => ({
28+
MonacoEditor: ({
29+
value,
30+
loading,
31+
}: {
32+
value: string;
33+
loading?: React.ReactNode;
34+
}) => (
35+
<div data-testid="monaco-editor" data-value={value || ''}>
36+
{loading || value || 'Editor'}
37+
</div>
38+
),
39+
}));
40+
41+
// Import mount after mocks are set up
42+
const { mount } = await import('../../js/log-viewer/component');
43+
44+
describe('LogViewer Component - Monaco Initialization Race Conditions', () => {
45+
let store: ReturnType<typeof createLogStore>;
46+
let container: HTMLElement;
47+
48+
const sampleLogs: LogLine[] = [
49+
{
50+
id: 'log-1',
51+
message: 'Test log message 1',
52+
source: 'RTE',
53+
level: 'info',
54+
step_id: 'step-1',
55+
timestamp: new Date('2025-01-01T00:00:00Z'),
56+
},
57+
{
58+
id: 'log-2',
59+
message: 'Test log message 2',
60+
source: 'RTE',
61+
level: 'info',
62+
step_id: 'step-1',
63+
timestamp: new Date('2025-01-01T00:00:01Z'),
64+
},
65+
];
66+
67+
beforeEach(() => {
68+
vi.clearAllMocks();
69+
store = createLogStore();
70+
container = document.createElement('div');
71+
document.body.appendChild(container);
72+
});
73+
74+
afterEach(() => {
75+
document.body.removeChild(container);
76+
});
77+
78+
test('renders Monaco editor component', async () => {
79+
mount(container, store);
80+
81+
await waitFor(() => {
82+
const editor = container.querySelector('[data-testid="monaco-editor"]');
83+
expect(editor).not.toBeNull();
84+
});
85+
});
86+
87+
test('handles logs that arrive before component mount (race condition)', async () => {
88+
// Step 1: Add logs to store BEFORE mounting component
89+
store.getState().addLogLines(sampleLogs);
90+
const expectedContent = store.getState().formattedLogLines;
91+
92+
// Step 2: Mount component
93+
mount(container, store);
94+
95+
// Step 3: Wait for Monaco to receive the value prop
96+
await waitFor(() => {
97+
const editor = screen.getByTestId('monaco-editor');
98+
const dataValue = editor.getAttribute('data-value');
99+
expect(dataValue).toBe(expectedContent);
100+
});
101+
102+
// Verify both log messages are in the content
103+
expect(expectedContent).toContain('Test log message 1');
104+
expect(expectedContent).toContain('Test log message 2');
105+
});
106+
107+
test('handles logs that arrive immediately after mount', async () => {
108+
// Another race condition: logs arrive right after component mounts
109+
110+
// Step 1: Mount component first
111+
mount(container, store);
112+
113+
// Step 2: Logs arrive immediately (within milliseconds)
114+
store.getState().addLogLines(sampleLogs);
115+
const expectedContent = store.getState().formattedLogLines;
116+
117+
// Step 3: useMonacoSync should handle this and apply the value
118+
await waitFor(() => {
119+
const editor = screen.getByTestId('monaco-editor');
120+
const dataValue = editor.getAttribute('data-value');
121+
expect(dataValue).toBe(expectedContent);
122+
});
123+
});
124+
125+
test('handles multiple rapid log updates before editor ready', async () => {
126+
// Simulate multiple log batches arriving rapidly
127+
128+
mount(container, store);
129+
130+
// Multiple rapid updates
131+
const logs1: LogLine[] = [
132+
{
133+
id: 'log-1',
134+
message: 'First batch',
135+
source: 'RTE',
136+
level: 'info',
137+
step_id: 'step-1',
138+
timestamp: new Date('2025-01-01T00:00:00Z'),
139+
},
140+
];
141+
142+
const logs2: LogLine[] = [
143+
{
144+
id: 'log-2',
145+
message: 'Second batch',
146+
source: 'RTE',
147+
level: 'info',
148+
step_id: 'step-1',
149+
timestamp: new Date('2025-01-01T00:00:01Z'),
150+
},
151+
];
152+
153+
const logs3: LogLine[] = [
154+
{
155+
id: 'log-3',
156+
message: 'Third batch',
157+
source: 'RTE',
158+
level: 'info',
159+
step_id: 'step-1',
160+
timestamp: new Date('2025-01-01T00:00:02Z'),
161+
},
162+
];
163+
164+
store.getState().addLogLines(logs1);
165+
store.getState().addLogLines(logs2);
166+
store.getState().addLogLines(logs3);
167+
168+
const finalContent = store.getState().formattedLogLines;
169+
170+
await waitFor(() => {
171+
const editor = screen.getByTestId('monaco-editor');
172+
const dataValue = editor.getAttribute('data-value');
173+
expect(dataValue).toBe(finalContent);
174+
});
175+
176+
// All three batches should be in the final content
177+
expect(finalContent).toContain('First batch');
178+
expect(finalContent).toContain('Second batch');
179+
expect(finalContent).toContain('Third batch');
180+
});
181+
182+
test('updates when new logs arrive after initial render', async () => {
183+
// Test that subsequent log updates continue to work
184+
185+
mount(container, store);
186+
187+
// First batch of logs
188+
store.getState().addLogLines([sampleLogs[0]]);
189+
190+
await waitFor(() => {
191+
const editor = screen.getByTestId('monaco-editor');
192+
const dataValue = editor.getAttribute('data-value') || '';
193+
expect(dataValue).toContain('Test log message 1');
194+
});
195+
196+
// Second batch arrives
197+
store.getState().addLogLines([sampleLogs[1]]);
198+
199+
await waitFor(() => {
200+
const editor = screen.getByTestId('monaco-editor');
201+
const dataValue = editor.getAttribute('data-value') || '';
202+
expect(dataValue).toContain('Test log message 1');
203+
expect(dataValue).toContain('Test log message 2');
204+
});
205+
});
206+
207+
test('handles log level filtering', async () => {
208+
// Test that log level filtering works with the sync
209+
210+
const debugLog: LogLine = {
211+
id: 'log-debug',
212+
message: 'Debug message',
213+
source: 'RTE',
214+
level: 'debug',
215+
step_id: 'step-1',
216+
timestamp: new Date('2025-01-01T00:00:00Z'),
217+
};
218+
219+
const infoLog: LogLine = {
220+
id: 'log-info',
221+
message: 'Info message',
222+
source: 'RTE',
223+
level: 'info',
224+
step_id: 'step-1',
225+
timestamp: new Date('2025-01-01T00:00:01Z'),
226+
};
227+
228+
// Set to info level (default) - debug should be filtered out
229+
store.getState().setDesiredLogLevel('info');
230+
store.getState().addLogLines([debugLog, infoLog]);
231+
232+
mount(container, store);
233+
234+
await waitFor(() => {
235+
const editor = screen.getByTestId('monaco-editor');
236+
const dataValue = editor.getAttribute('data-value') || '';
237+
expect(dataValue).toContain('Info message');
238+
expect(dataValue).not.toContain('Debug message');
239+
});
240+
241+
// Change to debug level - both should appear
242+
store.getState().setDesiredLogLevel('debug');
243+
244+
await waitFor(() => {
245+
const editor = screen.getByTestId('monaco-editor');
246+
const dataValue = editor.getAttribute('data-value') || '';
247+
expect(dataValue).toContain('Info message');
248+
expect(dataValue).toContain('Debug message');
249+
});
250+
});
251+
});

‎assets/vitest.config.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -46,7 +46,7 @@ export default defineConfig({
4646
// Mock monaco-editor for tests (8MB+ package causes issues)
4747
'monaco-editor': path.resolve(
4848
__dirname,
49-
'./test/_mocks/monaco-editor.ts'
49+
'./test/__mocks__/monaco-editor.ts'
5050
),
5151
},
5252
},

0 commit comments

Comments
 (0)