From cd5334659484f1b290e028a674826c02754d8035 Mon Sep 17 00:00:00 2001 From: Dmitry Gozman Date: Wed, 4 May 2022 17:16:24 +0100 Subject: [PATCH] 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. --- packages/playwright-core/src/server/recorder.ts | 7 ++++--- .../src/server/recorder/recorderApp.ts | 10 +++++----- .../src/server/recorder/recorderTypes.ts | 1 + packages/recorder/src/recorder.tsx | 11 +++++++++-- 4 files changed, 19 insertions(+), 10 deletions(-) diff --git a/packages/playwright-core/src/server/recorder.ts b/packages/playwright-core/src/server/recorder.ts index 24a14b3f2b..0e56dbb047 100644 --- a/packages/playwright-core/src/server/recorder.ts +++ b/packages/playwright-core/src/server/recorder.ts @@ -127,7 +127,7 @@ export class Recorder implements InstrumentationListener { this._contextRecorder.on(ContextRecorder.Events.Change, (data: { sources: Source[], primaryFileName: string }) => { this._recorderSources = data.sources; this._pushAllSources(); - this._recorderApp?.setFile(data.primaryFileName); + this._recorderApp?.setFileIfNeeded(data.primaryFileName); }); await this._context.exposeBinding('_playwrightRecorderState', false, source => { @@ -229,7 +229,7 @@ export class Recorder implements InstrumentationListener { const { file, line } = metadata.stack[0]; let source = this._userSources.get(file); 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); } if (line) { @@ -241,7 +241,7 @@ export class Recorder implements InstrumentationListener { } this._pushAllSources(); if (fileToSelect) - this._recorderApp?.setFile(fileToSelect); + this._recorderApp?.setFileIfNeeded(fileToSelect); } private _pushAllSources() { @@ -325,6 +325,7 @@ class ContextRecorder extends EventEmitter { this._recorderSources = []; for (const languageGenerator of orderedLanguages) { const source: Source = { + isRecorded: true, file: languageGenerator.fileName, text: generator.generateText(languageGenerator), language: languageGenerator.highlighter, diff --git a/packages/playwright-core/src/server/recorder/recorderApp.ts b/packages/playwright-core/src/server/recorder/recorderApp.ts index 4f8fa39ba8..4f1f9c232b 100644 --- a/packages/playwright-core/src/server/recorder/recorderApp.ts +++ b/packages/playwright-core/src/server/recorder/recorderApp.ts @@ -28,7 +28,7 @@ import { findChromiumChannel } from '../registry'; declare global { interface Window { - playwrightSetFile: (file: string) => void; + playwrightSetFileIfNeeded: (file: string) => void; playwrightSetMode: (mode: Mode) => void; playwrightSetPaused: (paused: boolean) => void; playwrightSetSources: (sources: Source[]) => void; @@ -42,7 +42,7 @@ export interface IRecorderApp extends EventEmitter { close(): Promise; setPaused(paused: boolean): Promise; setMode(mode: 'none' | 'recording' | 'inspecting'): Promise; - setFile(file: string): Promise; + setFileIfNeeded(file: string): Promise; setSelector(selector: string, focus?: boolean): Promise; updateCallLogs(callLogs: CallLog[]): Promise; bringToFront(): void; @@ -133,9 +133,9 @@ export class RecorderApp extends EventEmitter implements IRecorderApp { }).toString(), true, mode, 'main').catch(() => {}); } - async setFile(file: string): Promise { + async setFileIfNeeded(file: string): Promise { await this._page.mainFrame().evaluateExpression(((file: string) => { - window.playwrightSetFile(file); + window.playwrightSetFileIfNeeded(file); }).toString(), true, file, 'main').catch(() => {}); } @@ -181,7 +181,7 @@ class HeadlessRecorderApp extends EventEmitter implements IRecorderApp { async close(): Promise {} async setPaused(paused: boolean): Promise {} async setMode(mode: 'none' | 'recording' | 'inspecting'): Promise {} - async setFile(file: string): Promise {} + async setFileIfNeeded(file: string): Promise {} async setSelector(selector: string, focus?: boolean): Promise {} async updateCallLogs(callLogs: CallLog[]): Promise {} bringToFront(): void {} diff --git a/packages/playwright-core/src/server/recorder/recorderTypes.ts b/packages/playwright-core/src/server/recorder/recorderTypes.ts index 46bec63b6a..6e3e3931ec 100644 --- a/packages/playwright-core/src/server/recorder/recorderTypes.ts +++ b/packages/playwright-core/src/server/recorder/recorderTypes.ts @@ -52,6 +52,7 @@ export type SourceHighlight = { }; export type Source = { + isRecorded: boolean; file: string; text: string; language: string; diff --git a/packages/recorder/src/recorder.tsx b/packages/recorder/src/recorder.tsx index 17b566958e..d40cf16a56 100644 --- a/packages/recorder/src/recorder.tsx +++ b/packages/recorder/src/recorder.tsx @@ -25,7 +25,7 @@ import './recorder.css'; declare global { interface Window { - playwrightSetFile: (file: string) => void; + playwrightSetFileIfNeeded: (file: string) => void; playwrightSetSelector: (selector: string, focus?: boolean) => void; dispatch(data: any): Promise; } @@ -54,15 +54,22 @@ export const Recorder: React.FC = ({ }; const [f, setFile] = React.useState(); - window.playwrightSetFile = setFile; const file = f || sources[0]?.file; const source = sources.find(s => s.file === file) || { + isRecorded: false, text: '', language: 'javascript', file: '', 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(); React.useLayoutEffect(() => {