ReviewOS

stacks/very-happy-dom

BrowserPage.goto() only changes the URL and never loads the document

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

Summary

BrowserPage.goto() currently updates page.url but does not perform a request, parse the response body, replace the document, return the main-resource Response, or emit navigation lifecycle events.

That makes the Browser API example in the README look successful while leaving the previous/empty document in place. It also blocks the basic Playwright-style flow of navigating to a local app and interacting with the loaded page.

Reproduction

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

let fetchCalls = 0
globalThis.fetch = async () => {
  fetchCalls++
  return new Response('<h1>Loaded</h1>', {
    headers: { 'content-type': 'text/html' },
  })
}

const page = new Browser().newPage()
const response = await page.goto('https://example.test/demo')

console.log(fetchCalls) // 0
console.log(response) // null
console.log(page.url) // https://example.test/demo
console.log(page.mainFrame.document.querySelector('h1')) // null

Confirmed against main at 59d2e7c3 with Bun 1.4.1.

Current implementation

BrowserFrame.goto() only assigns this.url = url and returns null. waitForNavigation(), waitUntilComplete(), goBack(), goForward(), goSteps(), and reload() are also no-ops.

Expected behavior

At minimum, main-frame navigation should:

  1. Resolve and request the target URL.
  2. Populate a fresh document from an HTML response.
  3. Update window.location and history coherently.
  4. Return the main-resource Response (or null for non-network navigations).
  5. Emit the supported domcontentloaded and load page events in a deterministic order.
  6. Make waitForNavigation()/reload() observe the same navigation lifecycle.

Playwright's page.goto() loads the URL and returns the main-resource response: https://playwright.dev/docs/api/class-page#page-goto

If full script/resource loading is intentionally out of scope for the virtual DOM, loading and parsing the main HTML response would still make the documented Browser API usable and the limitation could be documented explicitly.

Why this matters

Without real main-resource navigation, Browser, BrowserContext, and BrowserPage cannot replace even lightweight Playwright browser tests: every caller must fetch HTML and inject page.content manually, navigation waits cannot be used, and redirects/history cannot be tested through the public API.

8 days ago

Implemented on main in 185dba7. Taking the report's own allowance: the main HTML response is loaded and parsed, and the script limitation is documented explicitly rather than papered over.

The reproduction, re-run:

fetchCalls                                        // 1      (was 0)
response                                          // Response (was null)
page.url                                          // https://example.test/demo
page.mainFrame.document.querySelector('h1')       // <h1>Loaded</h1>  (was null)

The six points

  1. Requests the target — through fetch, so page routes and request interception apply to a navigation as to any other request. A route can serve a page, which is tested.
  2. Populates a fresh document, replacing the old one rather than appending.
  3. Location and history stay coherent — a redirect lands the document on the URL fetch actually reached.
  4. Returns the main-resource Response, or null for a non-network target like about:blank.
  5. Emits domcontentloaded then load, walking readyState loading → interactive → complete.
  6. reload() and waitForNavigation() observe the same lifecycle.

History is a real stack: goBack/goForward/goSteps re-render each entry, stepping past either end returns null, and a fresh navigation discards the forward entries.

Two things worth knowing

Fragment markup is given a head/body structure. A browser parsing <h1>Hi</h1> yields <html><head></head><body>…. Assigning the fragment straight to documentElement left head and body null — since 99cf5b2 derives them from its children — so document.body.appendChild(...) would have failed after an ordinary navigation. Caught by a test, not in review.

abort() releases pending navigation waits. It delegates to the window, and also wakes anything awaiting a navigation: an aborted navigation will never arrive. Without that the suite hung, because an existing assertion in browser-api.stress.test.ts starts waitForNavigation(), calls abort(), then awaits it. It resolves rather than rejects, which is that existing contract.

Still out of scope, deliberately

Scripts do not run. External <script src> is not fetched and inline code is not evaluated, so a document arrives parsed but inert — no framework boots, nothing hydrates. There is a test pinning this so it is documented rather than discovered.

That is the line the report anticipated, and it is the same reason the wildloop Playwright specs are not portable: they assert on markup that only exists after hydration. goto now makes the Browser API usable for server-rendered markup, history and redirects; client-rendered assertions still need a real browser.

A non-HTML response also navigates and returns its response, but is not parsed as markup.

18 tests in test/browser-navigation.test.ts. Suite at 1579 passing, lint/typecheck/build clean.

8 days ago

Fixed in 185dba7 (goto performs a real navigation with a history stack), on top of b570adc giving the frame a real Window. 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.