ReviewOS

stacks/very-happy-dom

confirm() always returns false and there is no page.on('dialog'), so confirm-gated actions are untestable

#1612
Closed glennmichael123 opened this 8 days ago ·
8 days ago

Problem

alert, confirm and prompt are hardcoded stubs (Window.ts:965):

alert(_message?: string): void {}

confirm(_message?: string): boolean {
  return false
}

prompt(_message?: string, _default?: string): string | null {
  return null
}

and there is no dialog event to override them:

page.on('dialog', ...)   // 'dialog' is not in PageEventType

PageEventType is 'console' | 'request' | 'response' | 'error' | 'load' | 'domcontentloaded', so this is a type error rather than a silent no-op — which is better than #1602's failure mode, but still leaves no way through.

Why it matters

confirm() returning a hardcoded false means a confirm-gated action can only ever be cancelled:

deleteButton.addEventListener('click', () => {
  if (!confirm('Delete this trail?')) return
  removeTrail()     // unreachable in a test, always
})

The test clicks the button, nothing happens, and the assertion fails with no hint that a dialog was involved. The "delete" half of every destructive flow is untestable, and destructive flows are the ones most worth a test.

prompt() returning null has the same shape. alert() is the benign one — a no-op is close enough to Playwright's auto-dismiss — except that there is no way to assert an alert happened, which is the other reason to care.

Playwright's model fits cleanly: dialogs auto-dismiss unless a handler is registered, so existing behaviour is the default and a test opts in.

Proposed scope

  • Add dialog to PageEventType, emitted with a Dialog carrying type ('alert' | 'confirm' | 'prompt' | 'beforeunload'), message, defaultValue, and accept(promptText?) / dismiss().
  • Route Window.alert / confirm / prompt through it. The return value is whatever the handler chose: accept() gives true / the prompt text, dismiss() gives false / null.
  • Default with no handler: dismiss, which keeps today's false / null and is what Playwright does.
  • These are synchronous DOM APIs, so the handler has to be able to answer synchronously. An async handler cannot be awaited from inside confirm() — that needs to either be rejected outright or documented as resolving after the return, and quietly returning the default while an async handler is still running would be the worst of the three.
  • beforeunload only if navigation already fires it; otherwise leave it out rather than advertise it.

Acceptance

  • With page.on('dialog', d => d.accept()), the confirm-gated handler above reaches removeTrail().
  • With d.accept('Ridge Loop'), prompt() returns that string.
  • With no handler, confirm() still returns false and prompt() still returns null, asserted, so the default is unchanged.
  • The event reports the message, so a test can assert which dialog appeared.
  • An async handler either throws or has documented, tested behaviour — not a silent default.

Found while assessing whether wildloop's Playwright specs could port here.

Sign in to comment on this issue.