ReviewOS

stacks/very-happy-dom

BrowserPage.evaluate() runs strings outside the frame and drops arguments

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

Summary

BrowserPage.evaluate() does not currently provide Playwright-like page-context evaluation:

  • String expressions run through host eval(), where document and window are not the frame's globals.
  • Function arguments are not accepted or forwarded.
  • A function that expects an argument silently receives undefined.

Reproduction

import { Browser } from 'very-happy-dom'

const page = new Browser().newPage()
page.content = '<head><title>Page title</title></head><body></body>'

page.evaluate('document.title')
// ReferenceError: document is not defined

;(page.evaluate as any)((value: number) => value + 1, 41)
// NaN (the argument is discarded)

Confirmed against main at 59d2e7c3 with Bun 1.4.1.

Current implementation

BrowserFrame.evaluate() creates a wrapper with window and document only for function input and invokes the function with no arguments. String input uses direct eval(code), so it executes in the module/host realm instead of the frame realm.

Expected behavior

Support a signature equivalent to:

evaluate<R, Arg>(fn: string | ((arg: Arg) => R), arg?: Arg): Promise<R>

Both forms should resolve window, document, location, and other exposed globals from the target frame. Function arguments and return values should cross the boundary predictably, with clear errors for unsupported values.

Playwright documents page-context execution and serializable arguments here: https://playwright.dev/docs/evaluating

Acceptance cases

expect(await page.evaluate('document.title')).toBe('Page title')
expect(await page.evaluate(value => value + 1, 41)).toBe(42)
expect(await page.evaluate(() => window.location.href)).toBe(page.url)

Frame evaluation should use the selected frame's globals rather than the process globals.

Why this matters

Evaluation is the escape hatch for browser automation. Without page globals and arguments, callers cannot inspect application state or execute common Playwright migration patterns, even when the virtual DOM already implements the relevant browser API.

8 days ago

Fixed on main in 40162fb.

Both reproductions from the report:

page.evaluate('document.title')        // "Page title"  (was: ReferenceError)
page.evaluate(value => value + 1, 41)  // 42            (was: NaN)

Both forms now run through new Function(..., 'with (window) { ... }') — the same mechanism the jsdom script runner already used — so bare document, localStorage, navigator, innerWidth and the rest resolve against the frame. arg is forwarded, BrowserPage.evaluate passes it on, and a string that is itself a function expression is invoked with it, matching Playwright.

One decision worth flagging

The report suggested evaluate<R, Arg>(...): Promise<R>. I kept the value returned directly rather than promise-wrapped, because that composes with await in both directions while the reverse does not:

  • await evaluate(async () => 7) still yields 7
  • evaluate('1 + 1') === 2 keeps working — there is an existing assertion of exactly that shape in browser-api.stress.test.ts

Returning a promise would have broken the synchronous form for no gain. Happy to revisit if Playwright signature parity matters more than not breaking callers.

One limitation now pinned by a test

globalThis inside a page function is still the host's, not the frame's — with (window) only affects bare-name resolution. Playwright runs in real page context where the two agree, so code reaching for globalThis.document will not see the frame. There is an explicit test asserting this, so it is documented behaviour rather than a latent surprise.

Closing this properly probably wants that limitation either accepted or tracked separately, so I have left the issue open for you.

16 tests in test/browser-page-evaluate.test.ts; 8 fail against the previous implementation. Full suite 1411 passing, lint/typecheck/build clean.

8 days ago

Fixed in 40162fb (evaluate runs in the frame realm and forwards its argument), on top of b570adc. Released in v0.1.11, so this is now available to consumers rather than only on main. The scope notes I left earlier on this issue still stand — read those for what was deliberately left out.

Sign in to comment on this issue.