JS formatting popover: format errors are silently dropped if user closes popover before async format completes #78

New Issue

2026-05-06T05:01:06Z

spikerj commented

2026-05-06 05:01:06 +00:00

Problem

When the user clicks Format Now inside the formatting options popover and then closes the popover (clicks outside / backdrop / Esc) before the async formatter.formatModelInPlace(...) call resolves, any Prettier parse error is silently dropped:

// js-formatting-options-button.component.ts
showFormatError(message: string): void {
  if (!this.isOpen()) return;
  this.panelRef()?.showError(message);
}

The most common case where this matters is exactly the high-error-probability path: user clicks Format on syntactically broken code, immediately closes the popover thinking nothing’s happening, and the parse error never surfaces. From the user’s perspective: “clicked Format, nothing happened.”

Repro

Open the JS playground.
Paste obviously broken code (e.g. function foo( {).
Open the formatting options popover, click Format Now, and within the same beat click outside the popover to dismiss it.
Observe nothing changes; no toast, no chip update, no console message.

Suggested fixes (any of)

Keep the popover open until the async format resolves. Add an isFormatting signal to the panel; bind the close handler to refuse while truthy. Simplest UX.
Route format errors through the toolbar status chip in addition to the panel. The chip is always visible, so even a closed popover doesn’t lose the error. Requires adding a one-shot error display path on the chip’s JsDebuggerToolbarComponent.
Cheap diagnostic: console.warn(message) in the closed-popover branch instead of silent return. Worse UX but at least makes the failure visible to a learner who opens devtools.

Owner pointers

libraries/tools/src/components/js-formatting-options/js-formatting-options-button.component.ts:153-156 (showFormatError)
libraries/tools/src/components/javascript-runner/javascript-runner.ts:199-207 (onFormatNowRequested)
libraries/tools/src/components/js-debugger-toolbar/js-debugger-toolbar.component.ts:54-56 (reportFormatError plumbing)

## Problem When the user clicks **Format Now** inside the formatting options popover and then closes the popover (clicks outside / backdrop / Esc) before the async `formatter.formatModelInPlace(...)` call resolves, any Prettier parse error is silently dropped: ```ts // js-formatting-options-button.component.ts showFormatError(message: string): void { if (!this.isOpen()) return; this.panelRef()?.showError(message); } ``` The most common case where this matters is exactly the high-error-probability path: user clicks Format on syntactically broken code, immediately closes the popover thinking nothing’s happening, and the parse error never surfaces. From the user’s perspective: “clicked Format, nothing happened.” ## Repro 1. Open the JS playground. 2. Paste obviously broken code (e.g. `function foo( {`). 3. Open the formatting options popover, click **Format Now**, and within the same beat click outside the popover to dismiss it. 4. Observe nothing changes; no toast, no chip update, no console message. ## Suggested fixes (any of) 1. **Keep the popover open until the async format resolves.** Add an `isFormatting` signal to the panel; bind the close handler to refuse while truthy. Simplest UX. 2. **Route format errors through the toolbar status chip in addition to the panel.** The chip is always visible, so even a closed popover doesn’t lose the error. Requires adding a one-shot error display path on the chip’s `JsDebuggerToolbarComponent`. 3. **Cheap diagnostic**: `console.warn(message)` in the closed-popover branch instead of silent return. Worse UX but at least makes the failure visible to a learner who opens devtools. ## Owner pointers - `libraries/tools/src/components/js-formatting-options/js-formatting-options-button.component.ts:153-156` (`showFormatError`) - `libraries/tools/src/components/javascript-runner/javascript-runner.ts:199-207` (`onFormatNowRequested`) - `libraries/tools/src/components/js-debugger-toolbar/js-debugger-toolbar.component.ts:54-56` (`reportFormatError` plumbing) ## Related Found during code review of the Prettier integration + JS formatting options panel feature wave.

spikerj commented

2026-06-15 15:12:54 +00:00

Fixed in spikersoft-angular PR #54. showFormatError() no longer early-returns when the popover is closed — it queues the message, re-opens the popover, and flushes the error into the panel via a constructor effect() once the overlay mounts (the panel viewChild resolves one CD cycle after isOpen flips). When the popover is still open the error shows immediately as before. Added a spec covering reopen-on-closed, flush-on-mount, immediate-show-when-open, and single-consume. Will close once PR #54 merges.

Fixed in `spikersoft-angular` PR #54. `showFormatError()` no longer early-returns when the popover is closed — it queues the message, re-opens the popover, and flushes the error into the panel via a constructor `effect()` once the overlay mounts (the panel `viewChild` resolves one CD cycle after `isOpen` flips). When the popover is still open the error shows immediately as before. Added a spec covering reopen-on-closed, flush-on-mount, immediate-show-when-open, and single-consume. Will close once PR #54 merges.

spikerj commented

2026-06-15 15:33:32 +00:00