fix(codegen): do not reset current source on every recorded action (#13925)

Currently, when I choose "Java" in the sources list and then
click on the page to generate the "click" action, sources reset
to "JavaScript". This is very inconvenient.

This changes the logic to only forcefully change files if either
old or new file is a user file, not a generated one.

Use cases considered:
- run `codegen`, click around, choose different language, click more;
- run script with inspector, pause, click "Record" and record an action;
- same as above, but then continue and see that user source is revealed.
This commit is contained in:
Dmitry Gozman 2022-05-04 17:16:24 +01:00 committed by GitHub
parent 9e0aa67d28
commit cd53346594
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
4 changed files with 19 additions and 10 deletions

View file

@ -127,7 +127,7 @@ export class Recorder implements InstrumentationListener {
this._contextRecorder.on(ContextRecorder.Events.Change, (data: { sources: Source[], primaryFileName: string }) => { this._contextRecorder.on(ContextRecorder.Events.Change, (data: { sources: Source[], primaryFileName: string }) => {
this._recorderSources = data.sources; this._recorderSources = data.sources;
this._pushAllSources(); this._pushAllSources();
this._recorderApp?.setFile(data.primaryFileName); this._recorderApp?.setFileIfNeeded(data.primaryFileName);
}); });
await this._context.exposeBinding('_playwrightRecorderState', false, source => { await this._context.exposeBinding('_playwrightRecorderState', false, source => {
@ -229,7 +229,7 @@ export class Recorder implements InstrumentationListener {
const { file, line } = metadata.stack[0]; const { file, line } = metadata.stack[0];
let source = this._userSources.get(file); let source = this._userSources.get(file);
if (!source) { if (!source) {
source = { file, text: this._readSource(file), highlight: [], language: languageForFile(file) }; source = { isRecorded: false, file, text: this._readSource(file), highlight: [], language: languageForFile(file) };
this._userSources.set(file, source); this._userSources.set(file, source);
} }
if (line) { if (line) {
@ -241,7 +241,7 @@ export class Recorder implements InstrumentationListener {
} }
this._pushAllSources(); this._pushAllSources();
if (fileToSelect) if (fileToSelect)
this._recorderApp?.setFile(fileToSelect); this._recorderApp?.setFileIfNeeded(fileToSelect);
} }
private _pushAllSources() { private _pushAllSources() {
@ -325,6 +325,7 @@ class ContextRecorder extends EventEmitter {
this._recorderSources = []; this._recorderSources = [];
for (const languageGenerator of orderedLanguages) { for (const languageGenerator of orderedLanguages) {
const source: Source = { const source: Source = {
isRecorded: true,
file: languageGenerator.fileName, file: languageGenerator.fileName,
text: generator.generateText(languageGenerator), text: generator.generateText(languageGenerator),
language: languageGenerator.highlighter, language: languageGenerator.highlighter,

View file

@ -28,7 +28,7 @@ import { findChromiumChannel } from '../registry';
declare global { declare global {
interface Window { interface Window {
playwrightSetFile: (file: string) => void; playwrightSetFileIfNeeded: (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;
@ -42,7 +42,7 @@ export interface IRecorderApp extends EventEmitter {
close(): Promise<void>; close(): Promise<void>;
setPaused(paused: boolean): Promise<void>; setPaused(paused: boolean): Promise<void>;
setMode(mode: 'none' | 'recording' | 'inspecting'): Promise<void>; setMode(mode: 'none' | 'recording' | 'inspecting'): Promise<void>;
setFile(file: string): Promise<void>; setFileIfNeeded(file: string): Promise<void>;
setSelector(selector: string, focus?: boolean): Promise<void>; setSelector(selector: string, focus?: boolean): Promise<void>;
updateCallLogs(callLogs: CallLog[]): Promise<void>; updateCallLogs(callLogs: CallLog[]): Promise<void>;
bringToFront(): void; bringToFront(): void;
@ -133,9 +133,9 @@ export class RecorderApp extends EventEmitter implements IRecorderApp {
}).toString(), true, mode, 'main').catch(() => {}); }).toString(), true, mode, 'main').catch(() => {});
} }
async setFile(file: string): Promise<void> { async setFileIfNeeded(file: string): Promise<void> {
await this._page.mainFrame().evaluateExpression(((file: string) => { await this._page.mainFrame().evaluateExpression(((file: string) => {
window.playwrightSetFile(file); window.playwrightSetFileIfNeeded(file);
}).toString(), true, file, 'main').catch(() => {}); }).toString(), true, file, 'main').catch(() => {});
} }
@ -181,7 +181,7 @@ class HeadlessRecorderApp 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: 'none' | 'recording' | 'inspecting'): Promise<void> {} async setMode(mode: 'none' | 'recording' | 'inspecting'): Promise<void> {}
async setFile(file: string): Promise<void> {} async setFileIfNeeded(file: string): Promise<void> {}
async setSelector(selector: string, focus?: boolean): Promise<void> {} async setSelector(selector: string, focus?: boolean): Promise<void> {}
async updateCallLogs(callLogs: CallLog[]): Promise<void> {} async updateCallLogs(callLogs: CallLog[]): Promise<void> {}
bringToFront(): void {} bringToFront(): void {}

View file

@ -52,6 +52,7 @@ export type SourceHighlight = {
}; };
export type Source = { export type Source = {
isRecorded: boolean;
file: string; file: string;
text: string; text: string;
language: string; language: string;

View file

@ -25,7 +25,7 @@ import './recorder.css';
declare global { declare global {
interface Window { interface Window {
playwrightSetFile: (file: string) => void; playwrightSetFileIfNeeded: (file: string) => void;
playwrightSetSelector: (selector: string, focus?: boolean) => void; playwrightSetSelector: (selector: string, focus?: boolean) => void;
dispatch(data: any): Promise<void>; dispatch(data: any): Promise<void>;
} }
@ -54,15 +54,22 @@ export const Recorder: React.FC<RecorderProps> = ({
}; };
const [f, setFile] = React.useState<string | undefined>(); const [f, setFile] = React.useState<string | undefined>();
window.playwrightSetFile = setFile;
const file = f || sources[0]?.file; const file = f || sources[0]?.file;
const source = sources.find(s => s.file === file) || { const source = sources.find(s => s.file === file) || {
isRecorded: false,
text: '', text: '',
language: 'javascript', language: 'javascript',
file: '', file: '',
highlight: [] highlight: []
}; };
window.playwrightSetFileIfNeeded = (value: string) => {
const newSource = sources.find(s => s.file === value);
// Do not forcefully switch between two recorded sources, because
// user did explicitly choose one.
if (newSource && !newSource.isRecorded || !source.isRecorded)
setFile(value);
};
const messagesEndRef = React.createRef<HTMLDivElement>(); const messagesEndRef = React.createRef<HTMLDivElement>();
React.useLayoutEffect(() => { React.useLayoutEffect(() => {