Skip to content

fix: [#2374] classic <script> top-level var/function declarations must reach the global object - #2246

Open
marciomazza wants to merge 1 commit into
capricorn86:masterfrom
marciomazza:fix/classic-script-global-declarations
Open

fix: [#2374] classic <script> top-level var/function declarations must reach the global object#2246
marciomazza wants to merge 1 commit into
capricorn86:masterfrom
marciomazza:fix/classic-script-global-declarations

Conversation

@marciomazza

@marciomazza marciomazza commented Jul 18, 2026

Copy link
Copy Markdown

Description

Resolves #2374

happy-dom wrapped every classic script body in (function anonymous($happy_dom) { ... }) before running it, trapping top-level "var" and function declarations inside that function's own scope instead of letting them become properties of the global object, as real browsers do (and as the ECMAScript spec requires for classic, non-module scripts). Only module scripts are supposed to get their own scope.

Example of the bug:

const script = document.createElement('script');
script.text = 'var foo = 123;';
document.body.appendChild(script);

console.log(window.foo); // happy-dom: undefined — real browsers: 123

I ran into this while loading a real-world library (htmx) as a classic <script src>: the library's own top-level var htmx = ... never reached window, even though the exact same file works fine in an actual browser.

In this fix, JavaScriptCompiler no longer wraps the code in a function. $happy_dom (needed for dynamicImport()/dispatchError()) is now bound as a non-enumerable property on the window instead of a function parameter — it stays installed for the window's lifetime because async continuations of dynamicImport() reference it after the synchronous top level finishes. execute() runs the code directly through evaluateScript() so top-level declarations land on the global object. Adjacent behavior change: a syntax error now throws from execute() instead of compile() (e.g. a top-level returnSyntaxError), which is more spec-correct; both callers handle it.

AI

I used Claude Code to write this, over multiple iterations. I guided and reviewed
it myself at every step.

Before submitting the PR, please make sure you do the following:

  • Read the contributing guidelines.
  • It's really useful if your PR references an issue where it is discussed ahead of time.
  • Please check Allow edits by maintainers to make review process faster. Note that this option is not available for repositories that are owned by Github organizations.

Tests

  • Make sure to add tests for your changes. Run your test in a real browser to make sure that the test tests what a real browser would do (e.g. by running the code in the browser console). (Not verified in an actual browser console — behavior confirmed against the WHATWG/ECMAScript spec for classic-script top-level declarations, which is well established browser behavior.)
  • Run the tests with npm test locally to make sure that all tests pass before submitting the PR. (Full suite passes locally. Also ran tsc --noEmit and eslint --max-warnings 0 clean.)

Title

  • The title of the pull request should be in the format of "type: [#issue] description". The type can be feat, fix, chore or BREAKING CHANGE. The issue is optional and can be omitted if the pull request does not relate to an issue.
  • The title should be concise and descriptive. The title will be used when generating release notes. Make sure that the title is easily understood by users of the library.

@marciomazza
marciomazza requested a review from capricorn86 as a code owner July 18, 2026 18:12
@marciomazza
marciomazza force-pushed the fix/classic-script-global-declarations branch from b02b316 to 50c4e33 Compare July 18, 2026 19:51
@marciomazza

Copy link
Copy Markdown
Author

This PR's CI failure isn't caused by this diff — it's a known flaky test on master: the same cache-revalidation flake as SyncFetch.test.ts (stale content returned instead of the revalidated value), here in Fetch.test.ts, unrelated to what's changed here. Fixed in #2266.

Note there's a second unrelated flake still on master too — a ServerRendererBrowser.test.ts debug-trace race, fixed in #2270 — so a rebuild/rerun here could hit that one instead before both land. No action needed on this PR's own diff either way.

@marciomazza
marciomazza force-pushed the fix/classic-script-global-declarations branch 3 times, most recently from 458e397 to afaebf0 Compare September 4, 2026 19:36
@marciomazza marciomazza changed the title fix: Classic <script> top-level var/function declarations must reach the global object fix: [#2374] classic <script> top-level var/function declarations must reach the global object Sep 7, 2026
Removes the anonymous function wrapper around evaluated script code, so top-level var/function declarations attach to window (follows the spec).
Adds a test covering the behavior and fixes existing tests that hardcoded the old wrapped source.
@marciomazza
marciomazza force-pushed the fix/classic-script-global-declarations branch from afaebf0 to c6e6d4f Compare September 8, 2026 08:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

classic <script> top-level var / function declarations don't reach window

1 participant