From cabca2d93f4847e748aae8f52ea8b10dadead7d0 Mon Sep 17 00:00:00 2001 From: Andrew Scott Date: Thu, 8 May 2025 14:16:21 -0700 Subject: [PATCH] fix(docs-infra): Correctly support the CLI tutorial type (#61246) There were a myriad of problems when using the type: 'cli' for a tutorial: * Editor component would flicker the non-cli version first since that was the default value. Fixed by not showing anything until type is available * Editor component still rendered a split screen when there was no preview. Fixed * Project initialization was incorrect because the terminal would appear to be available but really didn't work until dependencies were done installing. Fixed by sharing initialization with regular project init and piping the install output to the interactive terminal * Terminals would not work when going from type: 'cli' to another or vice versa. This is because the element instance of a terminal cannot change once it is opened. Fixed by dispoing and recreating terminals when the element changes PR Close #61246 --- .../app/editor/embedded-editor.component.html | 145 +++++++++--------- .../node-runtime-sandbox.service.spec.ts | 46 +----- .../editor/node-runtime-sandbox.service.ts | 101 ++++-------- .../editor/terminal/interactive-terminal.ts | 16 +- .../terminal/terminal-handler.service.ts | 55 +++++-- .../editor/terminal/terminal.component.html | 1 - .../terminal/terminal.component.spec.ts | 61 +++++--- .../app/editor/terminal/terminal.component.ts | 54 +++---- .../features/tutorial/tutorial.component.ts | 5 +- 9 files changed, 225 insertions(+), 259 deletions(-) delete mode 100644 adev/src/app/editor/terminal/terminal.component.html diff --git a/adev/src/app/editor/embedded-editor.component.html b/adev/src/app/editor/embedded-editor.component.html index 3445748862c..8d9ff642d94 100644 --- a/adev/src/app/editor/embedded-editor.component.html +++ b/adev/src/app/editor/embedded-editor.component.html @@ -1,81 +1,84 @@ -
- - - - @if (!displayOnlyTerminal()) { - - } - - - @if (displayOnlyTerminal()) { +@if (this.editorUiState.tutorialType() !== undefined) { +
+ @if (displayOnlyTerminal()) { - } - - - - - @if (!displayOnlyTerminal()) { - - - @if (!displayPreviewInMatTabGroup()) { - - -
-
- Preview -
- @if (!displayPreviewInMatTabGroup()) { - - } -
+ } @else { + + + - } - - - - @if (displayPreviewInMatTabGroup()) { - - - + + + + + @if (!displayPreviewInMatTabGroup()) { + + +
+
+ Preview +
+ @if (!displayPreviewInMatTabGroup()) { + + } +
+ } - - - Console @if (errorsCount()) { - error - - {{ errorsCount() }} - - } - - - - - - - - + + + + @if (displayPreviewInMatTabGroup()) { + + + + } + + + Console + @if (errorsCount()) { + error + + {{ errorsCount() }} + + } + + + + + + + + + + + - } - - -
+ } +
+} diff --git a/adev/src/app/editor/node-runtime-sandbox.service.spec.ts b/adev/src/app/editor/node-runtime-sandbox.service.spec.ts index 9e22e908eab..be0b3442284 100644 --- a/adev/src/app/editor/node-runtime-sandbox.service.spec.ts +++ b/adev/src/app/editor/node-runtime-sandbox.service.spec.ts @@ -30,14 +30,14 @@ describe('NodeRuntimeSandbox', () => { let service: NodeRuntimeSandbox; const fakeTerminalHandler = { - interactiveTerminalInstance: { + interactiveTerminalInstance: signal({ write: (data: string) => {}, onData: (data: string) => {}, breakProcess$: observableOf(), - }, - readonlyTerminalInstance: { + }), + readonlyTerminalInstance: signal({ write: (data: string) => {}, - }, + }), clearTerminals: () => {}, }; @@ -194,16 +194,6 @@ describe('NodeRuntimeSandbox', () => { expect(renameFileSpy).toHaveBeenCalledOnceWith(oldPath, newPath); }); - it('should initialize the Angular CLI based on the tutorial config', async () => { - setValuesToInitializeAngularCLI(); - - const initAngularCliSpy = spyOn(service, 'initAngularCli' as any); - - await service.init(); - - expect(initAngularCliSpy).toHaveBeenCalled(); - }); - it('should initialize a project based on the tutorial config', async () => { service['webContainerPromise'] = Promise.resolve( new FakeWebContainer() as unknown as WebContainer, @@ -217,34 +207,6 @@ describe('NodeRuntimeSandbox', () => { expect(initProjectSpy).toHaveBeenCalled(); }); - it('should cleanup when initializing the Angular CLI if a project was initialized before', async () => { - const cleanupSpy = spyOn(service, 'cleanup' as any); - - setValuesToInitializeProject(); - await service.init(); - - expect(cleanupSpy).not.toHaveBeenCalled(); - - setValuesToInitializeAngularCLI(); - await service.init(); - - expect(cleanupSpy).toHaveBeenCalledOnceWith(); - }); - - it('should cleanup when initializing a project if the Angular CLI was initialized before', async () => { - const cleanupSpy = spyOn(service, 'cleanup' as any); - - setValuesToInitializeAngularCLI(); - await service.init(); - - expect(cleanupSpy).not.toHaveBeenCalled(); - - setValuesToInitializeProject(); - await service.init(); - - expect(cleanupSpy).toHaveBeenCalledOnceWith(); - }); - it("should set the error state when an out of memory message is received from the web container's output", async () => { service['webContainerPromise'] = Promise.resolve( new FakeWebContainer() as unknown as WebContainer, diff --git a/adev/src/app/editor/node-runtime-sandbox.service.ts b/adev/src/app/editor/node-runtime-sandbox.service.ts index f036194ff36..2a98d781371 100644 --- a/adev/src/app/editor/node-runtime-sandbox.service.ts +++ b/adev/src/app/editor/node-runtime-sandbox.service.ts @@ -6,7 +6,7 @@ * found in the LICENSE file at https://angular.dev/license */ -import {DestroyRef, inject, Injectable, signal} from '@angular/core'; +import {DestroyRef, effect, inject, Injectable, signal} from '@angular/core'; import {FileSystemTree, WebContainer, WebContainerProcess} from '@webcontainer/api'; import {BehaviorSubject, filter, map, Subject} from 'rxjs'; @@ -66,6 +66,20 @@ export class NodeRuntimeSandbox { private devServerProcess: WebContainerProcess | undefined; private webContainerPromise: Promise | undefined; + constructor() { + effect(() => { + const terminal = this.terminalHandler.interactiveTerminalInstance(); + terminal.onData((data) => { + this.interactiveShellWriter?.write(data); + }); + + terminal.breakProcess$.subscribe(() => { + // Write CTRL + C into shell to break active process + this.interactiveShellWriter?.write('\x03'); + }); + }); + } + get previewUrl$() { return this._previewUrl$; } @@ -97,11 +111,8 @@ export class NodeRuntimeSandbox { await this.startInteractiveTerminal(webContainer); this.terminalHandler.clearTerminals(); - if (this.embeddedTutorialManager.type() === TutorialType.CLI) { - await this.initAngularCli(); - } else { - await this.initProject(); - } + const startDevServer = this.embeddedTutorialManager.type() !== TutorialType.CLI; + await this.initProject(startDevServer); console.timeEnd('Load time'); } catch (error: any) { @@ -163,7 +174,7 @@ export class NodeRuntimeSandbox { /** * Initialize the WebContainer for an Angular project */ - private async initProject(): Promise { + private async initProject(startDevServer: boolean): Promise { // prevent re-initialization if (this._isProjectInitialized()) return; @@ -185,7 +196,11 @@ export class NodeRuntimeSandbox { if (![PROCESS_EXIT_CODE.SIGTERM, PROCESS_EXIT_CODE.SUCCESS].includes(exitCode)) throw new Error('Installation failed'); - await Promise.all([this.loadTypes(), this.startDevServer()]); + await Promise.all([ + this.loadTypes(), + startDevServer ? this.startDevServer() : Promise.resolve(), + ]); + this.setLoading(LoadingStep.READY); } private handleProjectChanges() { @@ -235,30 +250,6 @@ export class NodeRuntimeSandbox { await Promise.all([this.loadTypes(), this.startDevServer()]); } - /** - * Initialize the WebContainer for the Angular CLI - */ - private async initAngularCli() { - // prevent re-initialization - if (this._isAngularCliInitialized()) return; - - // clean up the sandbox if a project was initialized before so the CLI can - // be initialized without conflicts - if (this._isProjectInitialized()) { - await this.cleanup(); - this.urlToPreview$.next(null); - this._isProjectInitialized.set(false); - } - - this._isAngularCliInitialized.set(true); - - this.setLoading(LoadingStep.INSTALL); - const exitCode = await this.installAngularCli(); - - if (![PROCESS_EXIT_CODE.SIGTERM, PROCESS_EXIT_CODE.SUCCESS].includes(exitCode)) - this.setLoading(LoadingStep.READY); - } - async writeFile(path: string, content: string | Uint8Array): Promise { const webContainer = await this.webContainerPromise!; @@ -309,8 +300,6 @@ export class NodeRuntimeSandbox { // return existing shell process if it's already running if (this.interactiveShellProcess) return this.interactiveShellProcess; - const terminal = this.terminalHandler.interactiveTerminalInstance; - // use WebContainer spawn directly so that the process isn't killed on // cleanup const shellProcess = await webContainer.spawn('bash'); @@ -324,7 +313,7 @@ export class NodeRuntimeSandbox { new WritableStream({ write: (data) => { this.checkForOutOfMemoryError(data.toString()); - terminal.write(data); + this.terminalHandler.interactiveTerminalInstance().write(data); if (data.includes('CREATE') && data.endsWith('\r\n')) { const match = data.match(ngGenerateTerminalOutputRegex); @@ -339,17 +328,7 @@ export class NodeRuntimeSandbox { }), ); - const input = shellProcess.input.getWriter(); - this.interactiveShellWriter = input; - - terminal.onData((data) => { - input.write(data); - }); - - terminal.breakProcess$.subscribe(() => { - // Write CTRL + C into shell to break active process - input.write('\x03'); - }); + this.interactiveShellWriter = shellProcess.input.getWriter(); return shellProcess; } @@ -436,13 +415,17 @@ export class NodeRuntimeSandbox { installProcess.output.pipeTo( new WritableStream({ write: (data) => { - this.terminalHandler.readonlyTerminalInstance.write(data); + this.terminalHandler.readonlyTerminalInstance().write(data); + this.terminalHandler.interactiveTerminalInstance().write(data); }, }), ); // wait for install command to exit - return installProcess.exit; + const code = await installProcess.exit; + // Simulate pressing `Enter` in shell + this.interactiveShellWriter?.write('\x0D'); + return code; } private async loadTypes() { @@ -450,26 +433,6 @@ export class NodeRuntimeSandbox { await this.typingsLoader.retrieveTypeDefinitions(webContainer!); } - private async installAngularCli(): Promise { - // install Angular CLI - const installProcess = await this.spawn(PACKAGE_MANAGER, ['install', '@angular/cli@latest']); - - installProcess.output.pipeTo( - new WritableStream({ - write: (data) => { - this.terminalHandler.interactiveTerminalInstance.write(data); - }, - }), - ); - - const exitCode = await installProcess.exit; - - // Simulate pressing `Enter` in shell - this.interactiveShellWriter?.write('\x0D'); - - return exitCode; - } - private async startDevServer(): Promise { const webContainer = await this.webContainerPromise!; @@ -492,7 +455,7 @@ export class NodeRuntimeSandbox { this.devServerProcess.output.pipeTo( new WritableStream({ write: (data) => { - this.terminalHandler.readonlyTerminalInstance.write(data); + this.terminalHandler.readonlyTerminalInstance().write(data); if (this.checkForOutOfMemoryError(data.toString())) { reject(new Error(data.toString())); diff --git a/adev/src/app/editor/terminal/interactive-terminal.ts b/adev/src/app/editor/terminal/interactive-terminal.ts index 7a97964d9eb..33ed288cf61 100644 --- a/adev/src/app/editor/terminal/interactive-terminal.ts +++ b/adev/src/app/editor/terminal/interactive-terminal.ts @@ -6,12 +6,9 @@ * found in the LICENSE file at https://angular.dev/license */ -import {inject} from '@angular/core'; import {Subject} from 'rxjs'; import {Terminal} from '@xterm/xterm'; -import {WINDOW} from '@angular/docs'; - import {CommandValidator} from './command-validator.service'; export const NOT_VALID_COMMAND_MSG = 'Angular Documentation - Not allowed command!'; @@ -25,15 +22,15 @@ export const ALLOWED_KEYS: Array = [ ]; export class InteractiveTerminal extends Terminal { - private readonly window = inject(WINDOW); - private readonly commandValidator = inject(CommandValidator); - private readonly breakProcess = new Subject(); // Using this stream, the webcontainer shell can break current process. breakProcess$ = this.breakProcess.asObservable(); - constructor() { + constructor( + readonly window: Window, + readonly commandValidator: CommandValidator, + ) { super({convertEol: true, disableStdin: false}); // bypass command validation if sudo=true is present in the query string @@ -46,6 +43,11 @@ export class InteractiveTerminal extends Terminal { this.breakProcess.next(); } + override dispose(): void { + super.dispose(); + this.breakProcess.complete(); + } + // Method validate if provided command by user is on the list of the allowed commands. // If so, then command is executed, otherwise error message is displayed in the terminal. private handleCommandExecution(): void { diff --git a/adev/src/app/editor/terminal/terminal-handler.service.ts b/adev/src/app/editor/terminal/terminal-handler.service.ts index 20f7392dd96..2ae133803c7 100644 --- a/adev/src/app/editor/terminal/terminal-handler.service.ts +++ b/adev/src/app/editor/terminal/terminal-handler.service.ts @@ -6,10 +6,12 @@ * found in the LICENSE file at https://angular.dev/license */ -import {Injectable} from '@angular/core'; +import {computed, inject, Injectable, signal, Signal, untracked} from '@angular/core'; import {Terminal} from '@xterm/xterm'; import {FitAddon} from '@xterm/addon-fit'; import {InteractiveTerminal} from './interactive-terminal'; +import {WINDOW} from '@angular/docs'; +import {CommandValidator} from './command-validator.service'; export enum TerminalType { READONLY, @@ -18,47 +20,66 @@ export enum TerminalType { @Injectable({providedIn: 'root'}) export class TerminalHandler { + private readonly window = inject(WINDOW); + private readonly commandValidator = inject(CommandValidator); + private terminals = { // Passing a theme with CSS custom properties colors does not work // Because colors are parsed // See https://github.com/xtermjs/xterm.js/blob/854e2736f66ca3e5d3ab5a7b65bf3fd6fba8b707/src/browser/services/ThemeService.ts#L125 - [TerminalType.READONLY]: { + [TerminalType.READONLY]: signal({ instance: new Terminal({convertEol: true, disableStdin: true}), fitAddon: new FitAddon(), - }, - [TerminalType.INTERACTIVE]: { - instance: new InteractiveTerminal(), + }), + [TerminalType.INTERACTIVE]: signal({ + instance: new InteractiveTerminal(this.window, this.commandValidator), fitAddon: new FitAddon(), - }, - } as const; + }), + }; constructor() { // Load fitAddon for each terminal instance - for (const {instance, fitAddon} of Object.values(this.terminals)) { + for (const val of Object.values(this.terminals)) { + const {instance, fitAddon} = untracked(val); instance.loadAddon(fitAddon); } } - get readonlyTerminalInstance(): Terminal { - return this.terminals[TerminalType.READONLY].instance; + get readonlyTerminalInstance(): Signal { + return computed(() => this.terminals[TerminalType.READONLY]().instance); } - get interactiveTerminalInstance(): InteractiveTerminal { - return this.terminals[TerminalType.INTERACTIVE].instance; + get interactiveTerminalInstance(): Signal { + return computed(() => this.terminals[TerminalType.INTERACTIVE]().instance); } registerTerminal(type: TerminalType, element: HTMLElement): void { - const terminal = this.terminals[type]; - this.mapTerminalToElement(terminal.instance, terminal.fitAddon, element); + let {instance, fitAddon} = untracked(this.terminals[type]); + if (instance.element && element !== instance.element) { + instance.dispose(); + fitAddon = new FitAddon(); + if (type === TerminalType.READONLY) { + instance = new Terminal({convertEol: true, disableStdin: true}); + this.terminals[type].set({instance, fitAddon}); + } else { + const newInstance = new InteractiveTerminal(this.window, this.commandValidator); + instance = newInstance; + this.terminals[type].set({instance: newInstance, fitAddon}); + } + instance.loadAddon(fitAddon); + } + this.mapTerminalToElement(instance, fitAddon, element); } resizeToFitParent(type: TerminalType): void { - this.terminals[type]?.fitAddon.fit(); + untracked(this.terminals[type])?.fitAddon.fit(); } clearTerminals() { - this.terminals[TerminalType.READONLY].instance.clear(); - this.terminals[TerminalType.INTERACTIVE].instance.clear(); + untracked(() => { + this.terminals[TerminalType.READONLY]().instance.clear(); + this.terminals[TerminalType.INTERACTIVE]().instance.clear(); + }); } private mapTerminalToElement(terminal: Terminal, fitAddon: FitAddon, element: HTMLElement): void { diff --git a/adev/src/app/editor/terminal/terminal.component.html b/adev/src/app/editor/terminal/terminal.component.html deleted file mode 100644 index cfb942dc222..00000000000 --- a/adev/src/app/editor/terminal/terminal.component.html +++ /dev/null @@ -1 +0,0 @@ -
diff --git a/adev/src/app/editor/terminal/terminal.component.spec.ts b/adev/src/app/editor/terminal/terminal.component.spec.ts index f24cae34e3e..09efec9fd0c 100644 --- a/adev/src/app/editor/terminal/terminal.component.spec.ts +++ b/adev/src/app/editor/terminal/terminal.component.spec.ts @@ -7,44 +7,63 @@ */ import {ComponentFixture, TestBed} from '@angular/core/testing'; -import {By} from '@angular/platform-browser'; +import {DOCUMENT, inject} from '@angular/core'; import {Terminal} from './terminal.component'; -import {TerminalHandler, TerminalType} from './terminal-handler.service'; -import {FakeEventTarget, WINDOW} from '@angular/docs'; +import {TerminalType, TerminalHandler} from './terminal-handler.service'; +import {WINDOW} from '@angular/docs'; describe('Terminal', () => { - let component: Terminal; let fixture: ComponentFixture; - let terminalHandlerSpy: jasmine.SpyObj; - - const fakeWindow = new FakeEventTarget(); - beforeEach(async () => { - terminalHandlerSpy = jasmine.createSpyObj('TerminalHandler', [ - 'registerTerminal', - 'resizeToFitParent', - ]); - - await TestBed.configureTestingModule({ + TestBed.configureTestingModule({ imports: [Terminal], providers: [ - {provide: TerminalHandler, useValue: terminalHandlerSpy}, { provide: WINDOW, - useValue: fakeWindow, + useFactory: () => inject(DOCUMENT).defaultView, }, ], }); fixture = TestBed.createComponent(Terminal); - component = fixture.componentInstance; - component.type = TerminalType.READONLY; - fixture.detectChanges(); + fixture.componentRef.setInput('type', TerminalType.READONLY); + await fixture.whenStable(); }); - it('should create', () => { - expect(component).toBeTruthy(); + it('should write to terminal', async () => { + await expectWriteStringToTerminal('test string'); }); + + it('should write to terminal when using new instance', async () => { + await expectWriteStringToTerminal('first value'); + fixture.destroy(); + + fixture = TestBed.createComponent(Terminal); + fixture.componentRef.setInput('type', TerminalType.READONLY); + await fixture.whenStable(); + + await expectWriteStringToTerminal('second value'); + }); + + async function expectWriteStringToTerminal(v: string) { + TestBed.inject(TerminalHandler).readonlyTerminalInstance().write(v); + await expectAsync(until(() => fixture.nativeElement.innerHTML.indexOf(v) > -1)).toBeResolved(); + } }); + +async function until(fn: () => T): Promise { + const timeout = 100; + const start = performance.now(); + while (true) { + const result = fn(); + if (result) { + return result; + } + if (performance.now() - start > timeout) { + throw new Error(`condition not satisfied within ${timeout}ms.`); + } + await new Promise((r) => setTimeout(r, 1)); + } +} diff --git a/adev/src/app/editor/terminal/terminal.component.ts b/adev/src/app/editor/terminal/terminal.component.ts index 43ee4f11fc2..fd6c7b806ed 100644 --- a/adev/src/app/editor/terminal/terminal.component.ts +++ b/adev/src/app/editor/terminal/terminal.component.ts @@ -7,62 +7,56 @@ */ import { - AfterViewInit, ChangeDetectionStrategy, Component, - DestroyRef, ElementRef, - Input, + input, ViewEncapsulation, + afterNextRender, inject, viewChild, + DestroyRef, } from '@angular/core'; - -import {debounceTime} from 'rxjs/operators'; -import {TerminalHandler, TerminalType} from './terminal-handler.service'; -import {takeUntilDestroyed} from '@angular/core/rxjs-interop'; import {Subject} from 'rxjs'; +import {takeUntilDestroyed} from '@angular/core/rxjs-interop'; +import {debounceTime} from 'rxjs/operators'; + +import {TerminalHandler, TerminalType} from './terminal-handler.service'; @Component({ selector: 'docs-tutorial-terminal', - templateUrl: './terminal.component.html', + template: '
', styleUrls: ['./terminal.component.scss'], changeDetection: ChangeDetectionStrategy.OnPush, - imports: [], // ViewEncapsulation is disabled to allow Xterm.js's styles to be applied // to the terminal element. encapsulation: ViewEncapsulation.None, }) -export class Terminal implements AfterViewInit { - @Input({required: true}) type!: TerminalType; - readonly terminalElementRef = viewChild.required>('terminalOutput'); +export class Terminal { + readonly type = input.required(); + private readonly terminalElementRef = + viewChild.required>('terminalOutput'); private readonly destroyRef = inject(DestroyRef); private readonly terminalHandler = inject(TerminalHandler); - private readonly resize$ = new Subject(); - - ngAfterViewInit() { - this.terminalHandler.registerTerminal(this.type, this.terminalElementRef().nativeElement); - - this.setResizeObserver(); - - this.resize$.pipe(debounceTime(50), takeUntilDestroyed(this.destroyRef)).subscribe(() => { - this.handleResize(); + constructor() { + afterNextRender({ + read: () => { + this.terminalHandler.registerTerminal(this.type(), this.terminalElementRef().nativeElement); + this.setResizeObserver(); + }, }); } private setResizeObserver(): void { - const resizeObserver = new ResizeObserver((_) => { - this.resize$.next(); - }); + const resize = new Subject(); + resize + .pipe(debounceTime(50), takeUntilDestroyed(this.destroyRef)) + .subscribe(() => void this.terminalHandler.resizeToFitParent(this.type())); + const resizeObserver = new ResizeObserver(() => void resize.next()); resizeObserver.observe(this.terminalElementRef().nativeElement); - - this.destroyRef.onDestroy(() => resizeObserver.disconnect()); - } - - private handleResize(): void { - this.terminalHandler.resizeToFitParent(this.type); + this.destroyRef.onDestroy(() => void resizeObserver.disconnect()); } } diff --git a/adev/src/app/features/tutorial/tutorial.component.ts b/adev/src/app/features/tutorial/tutorial.component.ts index 710791fdb00..9ec7c6cb191 100644 --- a/adev/src/app/features/tutorial/tutorial.component.ts +++ b/adev/src/app/features/tutorial/tutorial.component.ts @@ -195,7 +195,10 @@ export default class Tutorial { if (routeData.type === TutorialType.LOCAL) { this.setLocalTutorialData(routeData); - } else if (routeData.type === TutorialType.EDITOR && this.isBrowser) { + } else if ( + (routeData.type === TutorialType.EDITOR || routeData.type === TutorialType.CLI) && + this.isBrowser + ) { await this.setEditorTutorialData( tutorialNavigationItem.path.replace(`${PagePrefix.TUTORIALS}/`, ''), );