Skip to content

Commit

Permalink
chore: improve code editing experience
Browse files Browse the repository at this point in the history 
- code keeps same formatting
- when changing to another language
  - if code is valid, changes replicate to the other languages
  - if it has an error, code is reset back to a valid state
  • Loading branch information
@ruifigueira
ruifigueira committed Dec 7, 2024
1 parent af34ae2 commit 39d99a4
Show file tree
Hide file tree
Showing 2 changed files with 194 additions and 58 deletions.
165 changes: 107 additions & 58 deletions src/server/recorder/crxRecorderApp.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -54,11 +54,11 @@ export class CrxRecorderApp extends EventEmitter implements IRecorderApp {
readonly _recorder: Recorder;
private _player: CrxPlayer;
private _filename?: string;
private _code?: string;
private _sources?: Source[];
private _mode: Mode = 'none';
private _window?: RecorderWindow;
private _codeLoadDebounceTimeout: NodeJS.Timeout | undefined;
private _editedCode?: EditedCode;
private _recordedActions: ActionInContextWithLocation[] = [];

constructor(recorder: Recorder, player: CrxPlayer) {
super();
Expand Down Expand Up @@ -114,11 +114,11 @@ export class CrxRecorderApp extends EventEmitter implements IRecorderApp {
}

async setMode(mode: Mode) {
if (['none', 'standby'].includes(mode)) {
if (!this._recorder._isRecording())
this._player.pause().catch(() => {});
} else {
else
this._player.stop().catch(() => {});
}

if (this._mode !== mode) {
this._mode = mode;
this.emit('modeChanged', { mode });
Expand All @@ -134,10 +134,11 @@ export class CrxRecorderApp extends EventEmitter implements IRecorderApp {
}

async setSources(sources: Source[]) {
// hack to prevent recorder from opening files
this._sources = sources.filter(s => s.isRecorded);
this._sendMessage({ type: 'recorder', method: 'setSources', sources: this._sources });
this._setCode(this._sources.find(s => s.id === 'playwright-test')?.text);
sources = sources
// hack to prevent recorder from opening files
.filter(s => s.isRecorded)
.map(s => this._editedCode?.decorate(s) ?? s);
this._sendMessage({ type: 'recorder', method: 'setSources', sources });
}

async elementPicked(elementInfo: ElementInfo, userGesture?: boolean) {
Expand All @@ -157,59 +158,39 @@ export class CrxRecorderApp extends EventEmitter implements IRecorderApp {
}

async setActions(actions: ActionInContext[], sources: Source[]) {
this._sendMessage({ type: 'recorder', method: 'setActions', actions, sources });
}

load(code: string) {
try {
if (this._codeLoadDebounceTimeout) {
clearTimeout(this._codeLoadDebounceTimeout);
this._codeLoadDebounceTimeout = undefined;
}
this._code = code;
const [{ actions, options }] = parse(code);
const { deviceName, contextOptions } = { deviceName: '', contextOptions: {}, ...options };
this._recorder.loadScript({ actions, deviceName, contextOptions, text: code });
return { actions, code };
} catch (error) {
// syntax error / parsing error
const line = error.loc.line ?? error.loc.start.line ?? code.split('\n').length;
const highlight: SourceHighlight[] = [{ line, type: 'error', message: error.message }];
this._recorder.loadScript({ actions: [], deviceName: '', contextOptions: {}, text: code, highlight });
return { actions: [] as ActionInContextWithLocation[], code, error };
}
this._recordedActions = Array.from(actions);
this._sources = Array.from(sources);
if (this._recorder._isRecording())
this._updateCode(null);
}

private _setCode(code?: string) {
if (this._code === code)
private _updateCode(code: string | null) {
if (this._editedCode?.code === code)
return;

this._code = code;
this._editedCode?.stopLoad();
this._editedCode = undefined;

if (this._codeLoadDebounceTimeout)
clearTimeout(this._codeLoadDebounceTimeout);
if (!code || this._recorder._isRecording())
return;

if (!code || !['inspecting', 'standby'].includes(this._mode)) {
this._codeLoadDebounceTimeout = undefined;
} else {
this._codeLoadDebounceTimeout = setTimeout(() => {
this._codeLoadDebounceTimeout = undefined;
if (this._code)
this.load(this._code);
}, 500);
}
this._editedCode = new EditedCode(this._recorder, code);
}

private _onMessage({ type, event, params }: RecorderEventData) {
if (type === 'recorderEvent') {
switch (event) {
case 'fileChanged':
if (this._code)
this.load(this._code);
this._filename = params.file;
if (this._editedCode?.hasErrors()) {
this._updateCode(null);
// force editor sources to refresh
if (this._sources)
this.setSources(this._sources);
}
break;
case 'codeChanged':
this._setCode(params.code);
this._updateCode(params.code);
break;
case 'resume':
case 'step':
Expand Down Expand Up @@ -237,21 +218,21 @@ export class CrxRecorderApp extends EventEmitter implements IRecorderApp {
}

private _getActions(): ActionInContextWithLocation[] {
if (!this._code)
return [];

// this will indirectly refresh sources
const { actions, error } = this.load(this._code);
if (error)
return [];

if (!this._filename || this._filename === 'playwright-test')
return actions;
if (this._editedCode) {
// this will indirectly refresh sources
this._editedCode.load();
let actions = this._editedCode.actions();

if (!this._filename || this._filename === 'playwright-test')
return actions;
}

const source = this._sources?.find(s => s.id === this._filename);
if (!source)
return [];

const actions = this._editedCode && !this._editedCode.hasErrors() ? this._editedCode.actions() : this._recordedActions;

const { header } = source;
const languageGenerator = [...languageSet()].find(l => l.id === this._filename)!;
// we generate actions here to have a one-to-one mapping between actions and text
Expand All @@ -263,6 +244,74 @@ export class CrxRecorderApp extends EventEmitter implements IRecorderApp {
return numLines(header) + numLines(actionTexts.slice(0, index).filter(Boolean).join('\n')) + 1;
}

return actions.map((action, index) => ({ ...action, location: { file: this._filename!, line: sourceLine(index), column: 1 } }));
return actions.map((action, index) => ({
...action,
location: {
file: this._filename!,
line: sourceLine(index),
column: 1
}
}));
}
}

class EditedCode {
readonly code: string;
private _recorder: Recorder;
private _actions: ActionInContextWithLocation[] = [];
private _highlight: SourceHighlight[] = [];
private _codeLoadDebounceTimeout: NodeJS.Timeout | undefined;

constructor(recorder: Recorder, code: string) {
this.code = code;
this._recorder = recorder;
this._codeLoadDebounceTimeout = setTimeout(this.load.bind(this), 500);
}

actions() {
return Array.from(this._actions);
}

hasErrors() {
return this._highlight?.length > 0;
}

hasLoaded() {
return !this._codeLoadDebounceTimeout;
}

decorate(source: Source) {
if (source.id !== 'playwright-test')
return;

return {
...source,
highlight: this.hasLoaded() && this.hasErrors() ? this._highlight : source.highlight,
text: this.code,
};
}

stopLoad() {
clearTimeout(this._codeLoadDebounceTimeout);
this._codeLoadDebounceTimeout = undefined;
}

load() {
if (this.hasLoaded())
return;

this.stopLoad();
try {
const [{ actions, options }] = parse(this.code);
this._actions = actions;
const { deviceName, contextOptions } = { deviceName: '', contextOptions: {}, ...options };
this._recorder.loadScript({ actions, deviceName, contextOptions, text: this.code });
} catch (error) {
this._actions = [];
// syntax error / parsing error
const line = error.loc.line ?? error.loc.start.line ?? this.code.split('\n').length;
this._highlight = [{ line, type: 'error', message: error.message }];
this._recorder.loadScript({ actions: this._actions, deviceName: '', contextOptions: {}, text: this.code, highlight: this._highlight });
}
}
}
87 changes: 87 additions & 0 deletions tests/crx/recorder-edit.spec.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,10 @@ async function editCode(recorderPage: Page, code: string) {
await editor.fill(code);
}

async function getCode(recorderPage: Page) {
return await recorderPage.locator('.CodeMirror').evaluate(elem => (elem as any).CodeMirror.getValue());
}

function editorLine(recorderPage: Page, linenumber: number) {
return recorderPage.locator('.CodeMirror-code > div')
.filter({ has: recorderPage.locator('.CodeMirror-linenumber', { hasText: String(linenumber) }) })
Expand Down Expand Up @@ -131,3 +135,86 @@ test('test', async ({ page }) => {

await expect(page.locator('textarea')).toHaveValue('modified test');
});

test('should reset code with errors if file changed', async ({ page, recordAction, attachRecorder, baseURL }) => {
const recorderPage = await attachRecorder(page);

await recordAction(() => page.goto(`${baseURL}/input/textarea.html`));

await recorderPage.getByTitle('Record').click();

// introduce an error in the code
await editCode(recorderPage, `import { test, expect } from '@playwright/test';
test('test', async ({ page }) => {
await page.errorFunction('${baseURL}/input/textarea.html');
});`);

// ensure it's visible
await expect(recorderPage.locator('.source-line-error-widget')).toBeVisible();

// change to python
await recorderPage.locator('.source-chooser').selectOption('python-pytest');

// should not display the error anymore
await expect(recorderPage.locator('.source-line-error-widget')).toBeHidden();

// back to javascript
await recorderPage.locator('.source-chooser').selectOption('playwright-test');

// error was reverted
await expect(editorLine(recorderPage, 4)).toHaveText(` await page.goto('${baseURL}/input/textarea.html');`);
});

test('should keep playwright test formatting', async ({ page, attachRecorder, baseURL }) => {
const recorderPage = await attachRecorder(page);
await recorderPage.getByTitle('Record').click();

const reformattedCode = `import { test, expect } from '@playwright/test';
test('test', async ({ page }) => {
await page.goto(
'${baseURL}/input/textarea.html'
)
await page.locator('textarea')
.fill('modified test');
});`;

await editCode(recorderPage, reformattedCode);

// ensure code is loaded
await recorderPage.waitForTimeout(1000);

// ensure there's no errors
await expect(recorderPage.locator('.source-line-error-widget')).toBeHidden();

// ensure code was not reformatted
expect(await getCode(recorderPage)).toBe(reformattedCode);

// change to python
await recorderPage.locator('.source-chooser').selectOption('python-pytest');

// ensure edited code is formatted the default python way
expect(await getCode(recorderPage)).toBe(`import re
from playwright.sync_api import Page, expect
def test_example(page: Page) -> None:
page.goto("${baseURL}/input/textarea.html")
page.locator("textarea").fill("modified test")
`);

// back to javascript
await recorderPage.locator('.source-chooser').selectOption('playwright-test');

// ensure code keeps same formatting
expect(await getCode(recorderPage)).toBe(reformattedCode);

await recorderPage.getByTitle('Step Over (F10)').click();
await expect(recorderPage.locator('.source-line-paused .CodeMirror-line')).toHaveText(` await page.goto(`);

await recorderPage.getByTitle('Step Over (F10)').click();
await expect(recorderPage.locator('.source-line-paused .CodeMirror-line')).toHaveText(` await page.locator('textarea')`);
});

0 comments on commit 39d99a4

Please sign in to comment.