fix(recorder): address the react race condition (#32628)

This commit is contained in:
Pavel Feldman 2024-09-16 13:47:13 -07:00 committed by GitHub
parent ce06a81aa6
commit 92c6408b94
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
7 changed files with 27 additions and 37 deletions

View file

@ -132,10 +132,9 @@ export class Recorder implements InstrumentationListener, IRecorder {
this._context.instrumentation.removeListener(this); this._context.instrumentation.removeListener(this);
this._recorderApp?.close().catch(() => {}); this._recorderApp?.close().catch(() => {});
}); });
this._contextRecorder.on(ContextRecorder.Events.Change, (data: { sources: Source[], primaryFileName: string }) => { this._contextRecorder.on(ContextRecorder.Events.Change, (data: { sources: Source[] }) => {
this._recorderSources = data.sources; this._recorderSources = data.sources;
this._pushAllSources(); this._pushAllSources();
this._recorderApp?.setFileIfNeeded(data.primaryFileName);
}); });
await this._context.exposeBinding('__pw_recorderState', false, source => { await this._context.exposeBinding('__pw_recorderState', false, source => {
@ -294,7 +293,7 @@ export class Recorder implements InstrumentationListener, IRecorder {
} }
this._pushAllSources(); this._pushAllSources();
if (fileToSelect) if (fileToSelect)
this._recorderApp?.setFileIfNeeded(fileToSelect); this._recorderApp?.setFile(fileToSelect);
} }
private _pushAllSources() { private _pushAllSources() {

View file

@ -97,10 +97,7 @@ export class ContextRecorder extends EventEmitter {
if (languageGenerator === this._orderedLanguages[0]) if (languageGenerator === this._orderedLanguages[0])
this._throttledOutputFile?.setContent(source.text); this._throttledOutputFile?.setContent(source.text);
} }
this.emit(ContextRecorder.Events.Change, { this.emit(ContextRecorder.Events.Change, { sources: this._recorderSources });
sources: this._recorderSources,
primaryFileName: this._orderedLanguages[0].id
});
}); });
context.on(BrowserContext.Events.BeforeClose, () => { context.on(BrowserContext.Events.BeforeClose, () => {
this._throttledOutputFile?.flush(); this._throttledOutputFile?.flush();

View file

@ -30,7 +30,7 @@ import type { IRecorder, IRecorderApp, IRecorderAppFactory } from './recorderFro
declare global { declare global {
interface Window { interface Window {
playwrightSetFileIfNeeded: (file: string) => void; playwrightSetFile: (file: string) => void;
playwrightSetMode: (mode: Mode) => void; playwrightSetMode: (mode: Mode) => void;
playwrightSetPaused: (paused: boolean) => void; playwrightSetPaused: (paused: boolean) => void;
playwrightSetSources: (sources: Source[]) => void; playwrightSetSources: (sources: Source[]) => void;
@ -46,7 +46,7 @@ export class EmptyRecorderApp extends EventEmitter implements IRecorderApp {
async close(): Promise<void> {} async close(): Promise<void> {}
async setPaused(paused: boolean): Promise<void> {} async setPaused(paused: boolean): Promise<void> {}
async setMode(mode: Mode): Promise<void> {} async setMode(mode: Mode): Promise<void> {}
async setFileIfNeeded(file: string): Promise<void> {} async setFile(file: string): Promise<void> {}
async setSelector(selector: string, userGesture?: boolean): Promise<void> {} async setSelector(selector: string, userGesture?: boolean): Promise<void> {}
async updateCallLogs(callLogs: CallLog[]): Promise<void> {} async updateCallLogs(callLogs: CallLog[]): Promise<void> {}
async setSources(sources: Source[]): Promise<void> {} async setSources(sources: Source[]): Promise<void> {}
@ -144,9 +144,9 @@ export class RecorderApp extends EventEmitter implements IRecorderApp {
}).toString(), { isFunction: true }, mode).catch(() => {}); }).toString(), { isFunction: true }, mode).catch(() => {});
} }
async setFileIfNeeded(file: string): Promise<void> { async setFile(file: string): Promise<void> {
await this._page.mainFrame().evaluateExpression(((file: string) => { await this._page.mainFrame().evaluateExpression(((file: string) => {
window.playwrightSetFileIfNeeded(file); window.playwrightSetFile(file);
}).toString(), { isFunction: true }, file).catch(() => {}); }).toString(), { isFunction: true }, file).catch(() => {});
} }

View file

@ -26,7 +26,7 @@ export interface IRecorderApp extends EventEmitter {
close(): Promise<void>; close(): Promise<void>;
setPaused(paused: boolean): Promise<void>; setPaused(paused: boolean): Promise<void>;
setMode(mode: Mode): Promise<void>; setMode(mode: Mode): Promise<void>;
setFileIfNeeded(file: string): Promise<void>; setFile(file: string): Promise<void>;
setSelector(selector: string, userGesture?: boolean): Promise<void>; setSelector(selector: string, userGesture?: boolean): Promise<void>;
updateCallLogs(callLogs: CallLog[]): Promise<void>; updateCallLogs(callLogs: CallLog[]): Promise<void>;
setSources(sources: Source[]): Promise<void>; setSources(sources: Source[]): Promise<void>;

View file

@ -55,7 +55,7 @@ export class RecorderInTraceViewer extends EventEmitter implements IRecorderApp
this._transport.sendEvent?.('setMode', { mode }); this._transport.sendEvent?.('setMode', { mode });
} }
async setFileIfNeeded(file: string): Promise<void> { async setFile(file: string): Promise<void> {
this._transport.sendEvent?.('setFileIfNeeded', { file }); this._transport.sendEvent?.('setFileIfNeeded', { file });
} }

View file

@ -27,14 +27,6 @@ import { asLocator } from '@isomorphic/locatorGenerators';
import { toggleTheme } from '@web/theme'; import { toggleTheme } from '@web/theme';
import { copy } from '@web/uiUtils'; import { copy } from '@web/uiUtils';
declare global {
interface Window {
playwrightSetFileIfNeeded: (file: string) => void;
playwrightSetSelector: (selector: string, focus?: boolean) => void;
dispatch(data: any): Promise<void>;
}
}
export interface RecorderProps { export interface RecorderProps {
sources: Source[], sources: Source[],
paused: boolean, paused: boolean,
@ -56,14 +48,22 @@ export const Recorder: React.FC<RecorderProps> = ({
setFileId(sources[0].id); setFileId(sources[0].id);
}, [fileId, sources]); }, [fileId, sources]);
const source: Source = sources.find(s => s.id === fileId) || { const source = React.useMemo(() => {
id: 'default', if (fileId) {
isRecorded: false, const source = sources.find(s => s.id === fileId);
text: '', if (source)
language: 'javascript', return source;
label: '', }
highlight: [] const source: Source = {
}; id: 'default',
isRecorded: false,
text: '',
language: 'javascript',
label: '',
highlight: []
};
return source;
}, [sources, fileId]);
const [locator, setLocator] = React.useState(''); const [locator, setLocator] = React.useState('');
window.playwrightSetSelector = (selector: string, focus?: boolean) => { window.playwrightSetSelector = (selector: string, focus?: boolean) => {
@ -73,13 +73,7 @@ export const Recorder: React.FC<RecorderProps> = ({
setLocator(asLocator(language, selector)); setLocator(asLocator(language, selector));
}; };
window.playwrightSetFileIfNeeded = (value: string) => { window.playwrightSetFile = setFileId;
const newSource = sources.find(s => s.id === value);
// Do not forcefully switch between two recorded sources, because
// user did explicitly choose one.
if (newSource && !newSource.isRecorded || !source.isRecorded)
setFileId(value);
};
const messagesEndRef = React.useRef<HTMLDivElement>(null); const messagesEndRef = React.useRef<HTMLDivElement>(null);
React.useLayoutEffect(() => { React.useLayoutEffect(() => {

View file

@ -96,7 +96,7 @@ declare global {
playwrightSetSources: (sources: Source[]) => void; playwrightSetSources: (sources: Source[]) => void;
playwrightSetOverlayVisible: (visible: boolean) => void; playwrightSetOverlayVisible: (visible: boolean) => void;
playwrightUpdateLogs: (callLogs: CallLog[]) => void; playwrightUpdateLogs: (callLogs: CallLog[]) => void;
playwrightSetFileIfNeeded: (file: string) => void; playwrightSetFile: (file: string) => void;
playwrightSetSelector: (selector: string, focus?: boolean) => void; playwrightSetSelector: (selector: string, focus?: boolean) => void;
playwrightSourcesEchoForTest: Source[]; playwrightSourcesEchoForTest: Source[];
dispatch(data: any): Promise<void>; dispatch(data: any): Promise<void>;