From e09aab2ed4089b2e62f9f44600e9498da74f83f4 Mon Sep 17 00:00:00 2001
From: Eric Pheterson <epheterson@gmail.com>
Date: Mon, 7 Sep 2026 19:44:48 -0700
Subject: [PATCH 1/3] Carry a script's top-level declarations across the wombat
 block

The rewriter wraps a script in a block so wombat can shadow window,
document and the rest. A block is a scope, so every top-level const, let
and class in that script becomes block-scoped with it and stops being
visible to any other script on the page. The script still runs, so
nothing reports an error; the page is simply wrong.

This ports wabac.js's approach (parseGlobals in src/rewrite/jsrewriter.ts),
keeping its names and its output byte for byte:

  * let is declared before the block and its keyword removed inside, so
    the assignment writes the outer binding
  * const and class are handed out through self.___WB_const_<name> and
    re-declared after the block, and the carrier deleted
  * a name that shadows one of the wombat globals is left to the script
    and that global is dropped from the wrapper
  * a top-level document.write() still gets its document.close()

Parsing is behind rewriting/js_ast.py so the parser is one import to
change. tree-sitter rather than a pure-Python parser because the input is
whatever the live web served: esprima is ES2017 and refuses optional
chaining, class fields and for-await, all ordinary in shipped code, and a
parse failure silently restores the bug. A script that still cannot be
parsed is wrapped exactly as before rather than corrupted.

Two existing fixtures encoded the old behaviour and are updated. One of
them, a script declaring 'var self', was emitting 'let self' twice in the
same block: a SyntaxError that killed the whole script.

Fixes #329
---
 pyproject.toml                        |   7 ++
 src/zimscraperlib/rewriting/js.py     |  91 +++++++++++++++-
 src/zimscraperlib/rewriting/js_ast.py | 146 ++++++++++++++++++++++++++
 tests/rewriting/test_js_globals.py    | 140 ++++++++++++++++++++++++
 tests/rewriting/test_js_rewriting.py  |  61 ++++++++---
 5 files changed, 428 insertions(+), 17 deletions(-)
 create mode 100644 src/zimscraperlib/rewriting/js_ast.py
 create mode 100644 tests/rewriting/test_js_globals.py

diff --git a/pyproject.toml b/pyproject.toml
index 06a731d..cc8f341 100644
--- a/pyproject.toml
+++ b/pyproject.toml
@@ -34,6 +34,13 @@ dependencies = [
   "piexif==1.1.3", # this dep is a nightmare in terms of release management, better pinned just like in optimize-images anyway
   "idna>=2.5,<4.0",
   "xxhash>=2.0,<4.0",
+  # Parsing JavaScript well enough to know which names a script declares at its
+  # top level (see rewriting/js_ast.py). tree-sitter rather than a pure-Python
+  # parser because the input is whatever the live web served: esprima is ES2017
+  # and refuses optional chaining, class fields and `for await`, all ordinary in
+  # shipped code, and a parse failure here silently restores the bug this fixes.
+  "tree-sitter>=0.23,<1.0",
+  "tree-sitter-javascript>=0.23,<1.0",
   "types-xxhash>=2.0,<4.0",
 ]
 dynamic = ["authors", "classifiers", "keywords", "license", "version", "urls"]
diff --git a/src/zimscraperlib/rewriting/js.py b/src/zimscraperlib/rewriting/js.py
index 2743fde..171fb4e 100644
--- a/src/zimscraperlib/rewriting/js.py
+++ b/src/zimscraperlib/rewriting/js.py
@@ -21,6 +21,7 @@
 from collections.abc import Callable, Iterable
 from typing import Any, Literal
 
+from zimscraperlib.rewriting.js_ast import parse_top_level
 from zimscraperlib.rewriting.rx_replacer import (
     RxRewriter,
     TransformationAction,
@@ -348,13 +349,97 @@ def rewrite(self, text: str | bytes, opts: dict[str, Any] | None = None) -> str:
         if opts.get("inline", False):
             new_text = new_text.replace("\n", " ")
 
-        # This is not totally correctly handling globals,
-        # see https://github.com/openzim/python-scraperlib/issues/329
         if wrap_globals:
-            new_text = self.first_buff + new_text + self.last_buff
+            new_text = self._wrap(new_text, GLOBAL_OVERRIDES)
+            if opts.get("inline", False):
+                new_text = new_text.replace("\n", " ")
 
         return new_text
 
+    def _wrap(self, new_text: str, overrides: list[str]) -> str:
+        """Put the script inside the wombat block, and put its globals back.
+
+        The block is a scope, so `const`, `let` and `class` declared at the top
+        level of the script stop being reachable from any other script on the
+        page — which is how a page that declares its data in one <script> and
+        reads it from another comes out broken but silent (#329).
+
+        So the declarations are carried across the block boundary, exactly as
+        wabac.js does it:
+
+          * `let x` is declared before the block and the keyword removed
+            inside it, so the assignment inside writes the outer binding
+          * `const x` and `class X` cannot be split that way, so their value
+            is handed out through `self.___WB_const_x` and re-declared as a
+            const after the block, and the carrier deleted
+          * a name that shadows one of the wombat globals is left alone, and
+            that global is dropped from the wrapper instead
+          * a top-level `document.write()` gets its `document.close()`
+
+        If the script cannot be parsed, none of this happens and the wrapper is
+        exactly what it was before: a script Zimi cannot read is still a script
+        it must not corrupt."""
+        first_buff = self.first_buff
+        last_buff = self.last_buff
+        pre_scope_globals = ""
+        in_scope_globals = ""
+        post_scope_globals = ""
+
+        parsed = parse_top_level(new_text) if new_text else None
+        if parsed is not None:
+            names: list[tuple[str, str]] = []
+            exclude_overrides: set[str] = set()
+            let_offsets: list[int] = []
+            last_start = -1
+            for decl in parsed.declarations:
+                if decl.name in overrides:
+                    exclude_overrides.add(decl.name)
+                    continue
+                if decl.kind == "class":
+                    names.append((decl.name, "const"))
+                elif decl.kind in ("const", "let"):
+                    names.append((decl.name, decl.kind))
+                    if decl.kind == "let" and last_start != decl.start:
+                        let_offsets.insert(0, decl.start)
+                        last_start = decl.start
+
+            if exclude_overrides:
+                first_buff = self._init_local_declaration(
+                    [name for name in overrides if name not in exclude_overrides]
+                )
+            if parsed.has_document_write:
+                last_buff = ";document.close();" + self.last_buff
+
+            # Offsets are byte offsets into the source, and descending, so each
+            # removal leaves the ones still to come valid.
+            data = new_text.encode("utf-8")
+            for offset in let_offsets:
+                data = data[:offset] + data[offset + len("let") :]
+            new_text = data.decode("utf-8", errors="replace")
+
+            for name, kind in names:
+                if kind == "const":
+                    varname = f"self.___WB_const_{name}"
+                    in_scope_globals += f"{varname} = {name};\n"
+                    post_scope_globals += (
+                        f"{kind} {name} = {varname}; delete {varname};\n"
+                    )
+                else:
+                    pre_scope_globals += f"let {name};\n"
+            if in_scope_globals:
+                in_scope_globals = "\n;" + in_scope_globals
+            if post_scope_globals:
+                post_scope_globals = "\n" + post_scope_globals
+
+        return (
+            pre_scope_globals
+            + first_buff
+            + new_text
+            + in_scope_globals
+            + last_buff
+            + post_scope_globals
+        )
+
     def _get_esm_import_rule(self) -> TransformationRule:
         # Capture plain local values instead of closing over `self`: a closure that
         # references `self` here would end up stored in `self.rules`, creating a
diff --git a/src/zimscraperlib/rewriting/js_ast.py b/src/zimscraperlib/rewriting/js_ast.py
new file mode 100644
index 0000000..f0f29e5
--- /dev/null
+++ b/src/zimscraperlib/rewriting/js_ast.py
@@ -0,0 +1,146 @@
+"""The little bit of JavaScript parsing the JS rewriter needs.
+
+`js.py` wraps a script in a block so wombat can shadow `window`, `document`
+and friends. A block is a scope, so every top-level `const`, `let` and `class`
+in the script becomes block-scoped too, and stops being visible to any other
+script on the page. wabac.js solves this by parsing the script and hoisting
+those names back out; this module is the parsing half of that, kept behind one
+function so the choice of parser is one import to change.
+
+Only the top level matters. Nothing nested can leak a global, so this never
+walks into a function body, and it answers four questions:
+
+  * which `const` / `let` / `class` names the script declares at the top level
+  * where each `let` statement starts, so the keyword can be removed
+  * which of the names shadow a global the wrapper is about to declare
+  * whether the script calls `document.write()` at the top level
+
+Why tree-sitter and not a pure-Python parser: the scripts this runs on are
+whatever the live web served. `esprima` (the obvious pure-Python choice) is
+ES2017 and refuses optional chaining, class fields and `for await`, all of
+which are ordinary in shipped code today; tree-sitter parses them, and is
+error-tolerant besides, so a script it cannot fully understand still yields
+the declarations it could read rather than an exception.
+"""
+
+from __future__ import annotations
+
+import functools
+from dataclasses import dataclass
+
+__all__ = ["Declaration", "TopLevel", "parse_top_level", "parser_available"]
+
+
+@dataclass(frozen=True)
+class Declaration:
+    """One name a script declares at its top level."""
+
+    name: str
+    kind: str  # "const" | "let" | "var" | "class"
+    start: int  # byte offset of the statement that declares it
+
+
+@dataclass(frozen=True)
+class TopLevel:
+    declarations: list[Declaration]
+    has_document_write: bool
+
+
+@functools.lru_cache(maxsize=1)
+def _parser():
+    """The parser, built once. None when tree-sitter is not installed, which
+    is not an error: the rewriter falls back to its unparsed behaviour."""
+    try:
+        import tree_sitter_javascript
+        from tree_sitter import Language, Parser
+
+        return Parser(Language(tree_sitter_javascript.language()))
+    except Exception:  # noqa: BLE001 - any import or ABI trouble means no parser
+        return None
+
+
+def parser_available() -> bool:
+    return _parser() is not None
+
+
+def _text(node, source: bytes) -> str:
+    return source[node.start_byte : node.end_byte].decode("utf-8", errors="replace")
+
+
+def _identifiers(node, source: bytes) -> list[str]:
+    """The plain identifiers a declarator binds.
+
+    Destructuring (`const {a, b} = x`) is deliberately skipped, exactly as
+    wabac.js skips anything whose id is not an Identifier: hoisting a
+    destructured binding would mean rebuilding the pattern, and the names it
+    binds are rare enough at the top level of a script to be worth leaving
+    alone rather than getting subtly wrong."""
+    names = []
+    for child in node.named_children:
+        if child.type == "variable_declarator":
+            first = child.child_by_field_name("name")
+            if first is not None and first.type == "identifier":
+                names.append(_text(first, source))
+    return names
+
+
+def _is_document_write(node, source: bytes) -> bool:
+    if node.type != "expression_statement":
+        return False
+    call = node.named_children[0] if node.named_children else None
+    if call is None or call.type != "call_expression":
+        return False
+    callee = call.child_by_field_name("function")
+    if callee is None or callee.type != "member_expression":
+        return False
+    obj = callee.child_by_field_name("object")
+    prop = callee.child_by_field_name("property")
+    return (
+        obj is not None
+        and prop is not None
+        and obj.type == "identifier"
+        and prop.type == "property_identifier"
+        and _text(obj, source) == "document"
+        and _text(prop, source) == "write"
+    )
+
+
+def parse_top_level(text: str) -> TopLevel | None:
+    """Read a script's top-level declarations, or None when it cannot be read.
+
+    None means "no opinion" and the caller must fall back to leaving the
+    script alone, which is what happened before this existed."""
+    parser = _parser()
+    if parser is None:
+        return None
+    source = text.encode("utf-8")
+    try:
+        tree = parser.parse(source)
+    except Exception:  # noqa: BLE001 - a parser crash must not fail a scrape
+        return None
+    root = tree.root_node
+    if root is None:
+        return None
+
+    declarations: list[Declaration] = []
+    has_document_write = False
+    for node in root.named_children:
+        if node.type == "lexical_declaration":
+            # `const` / `let`: the keyword is the first token of the statement.
+            kind = _text(node.children[0], source) if node.children else ""
+            if kind in ("const", "let"):
+                for name in _identifiers(node, source):
+                    declarations.append(Declaration(name, kind, node.start_byte))
+        elif node.type == "variable_declaration":
+            for name in _identifiers(node, source):
+                declarations.append(Declaration(name, "var", node.start_byte))
+        elif node.type == "class_declaration":
+            named = node.child_by_field_name("name")
+            if named is not None:
+                declarations.append(
+                    Declaration(_text(named, source), "class", node.start_byte)
+                )
+        elif not has_document_write and _is_document_write(node, source):
+            has_document_write = True
+
+    return TopLevel(declarations=declarations, has_document_write=has_document_write)
diff --git a/tests/rewriting/test_js_globals.py b/tests/rewriting/test_js_globals.py
new file mode 100644
index 0000000..757b773
--- /dev/null
+++ b/tests/rewriting/test_js_globals.py
@@ -0,0 +1,140 @@
+"""Top-level declarations survive the wombat block (#329).
+
+The rewriter wraps a script in `{ ... }` so wombat can shadow `window` and
+friends. A block is a scope, so a `const`, `let` or `class` declared at the top
+level of that script stops being visible to every other script on the page —
+silently, because the script itself still runs.
+
+These cases are ported from wabac.js's own suite (test/rewriteJS.ts), which is
+the reference implementation this module tracks, with its expected strings kept
+verbatim so a divergence shows up as a test failure rather than as a subtly
+different ZIM.
+"""
+
+from collections.abc import Callable
+
+import pytest
+
+from zimscraperlib.rewriting.js import JsRewriter
+from zimscraperlib.rewriting.js_ast import parser_available
+from zimscraperlib.rewriting.url_rewriting import ArticleUrlRewriter
+
+pytestmark = pytest.mark.skipif(
+    not parser_available(), reason="no JavaScript parser installed"
+)
+
+
+@pytest.fixture
+def js_rewriter(
+    simple_url_rewriter_gen: Callable[[str], ArticleUrlRewriter],
+) -> JsRewriter:
+    return JsRewriter(
+        url_rewriter=simple_url_rewriter_gen("http://www.example.com"),
+        base_href=None,
+        notify_js_module=None,
+    )
+
+
+def test_a_const_is_readable_after_the_block(js_rewriter: JsRewriter):
+    # The bug that started #329: nerdfonts.com declares its glyph table as a
+    # top-level const in one inline script and reads it from another.
+    out = js_rewriter.rewrite("const glyphs = {a: 1};\nwindow.x = 1;")
+    assert ";self.___WB_const_glyphs = glyphs;\n" in out
+    assert (
+        "const glyphs = self.___WB_const_glyphs; delete self.___WB_const_glyphs;" in out
+    )
+    assert out.index("self.___WB_const_glyphs = glyphs") < out.index("\n\n}")
+    assert out.index("\n\n}") < out.index("const glyphs = self.___WB_const_glyphs")
+
+
+def test_a_let_is_declared_before_the_block_and_assigned_inside(
+    js_rewriter: JsRewriter,
+):
+    out = js_rewriter.rewrite("let counter = 4;\nwindow.x = 1;")
+    assert out.startswith("let counter;\n")
+    # The keyword is removed where it stood, so the assignment inside the block
+    # writes the binding declared outside it.
+    assert "\n counter = 4;" in out
+    assert "let counter = 4" not in out
+
+
+def test_a_class_travels_as_a_const(js_rewriter: JsRewriter):
+    out = js_rewriter.rewrite("class Thing {}\nwindow.x = 1;")
+    assert ";self.___WB_const_Thing = Thing;\n" in out
+    assert "const Thing = self.___WB_const_Thing; delete self.___WB_const_Thing;" in out
+
+
+def test_var_is_left_alone(js_rewriter: JsRewriter):
+    # `var` is function-scoped, so the block never captured it and there is
+    # nothing to carry.
+    out = js_rewriter.rewrite("var legacy = 3;\nwindow.x = 1;")
+    assert "___WB_const_legacy" not in out
+    assert "var legacy = 3;" in out
+
+
+def test_a_name_that_shadows_a_wombat_global_wins(js_rewriter: JsRewriter):
+    # The script declares its own `location`; the wrapper must not declare one
+    # too, or the script's declaration is a redeclaration in the same scope.
+    out = js_rewriter.rewrite("const location = 'here';\nwindow.x = 1;")
+    assert 'let location = _____WB$wombat$assign$function_____("location");' not in out
+    assert 'let window = _____WB$wombat$assign$function_____("window");' in out
+    assert "___WB_const_location" not in out
+
+
+def test_a_top_level_document_write_gets_its_close(js_rewriter: JsRewriter):
+    out = js_rewriter.rewrite("document.write(x);")
+    assert ";document.close();" in out
+
+
+def test_the_whole_wabac_fixture(js_rewriter: JsRewriter):
+    # wabac.js's own combined case, expected output copied from its suite.
+    out = js_rewriter.rewrite("""
+  class A {}
+  const B = 5;
+  let C = 4;
+  var D = 3;
+
+  location = "http://example.com/2\"""")
+    assert out.startswith("let C;\n")
+    assert "  class A {}\n  const B = 5;\n   C = 4;\n  var D = 3;" in out
+    assert ";self.___WB_const_A = A;\nself.___WB_const_B = B;\n" in out
+    assert (
+        "const A = self.___WB_const_A; delete self.___WB_const_A;\n"
+        "const B = self.___WB_const_B; delete self.___WB_const_B;\n"
+    ) in out
+
+
+def test_nothing_is_carried_out_of_a_nested_scope(js_rewriter: JsRewriter):
+    # Only the top level can leak a global; a const inside a function is the
+    # function's business and must be left exactly as written.
+    out = js_rewriter.rewrite("function f() { const inner = 1; }\nwindow.x = 1;")
+    assert "___WB_const_inner" not in out
+    assert "const inner = 1;" in out
+
+
+def test_modern_syntax_is_still_parsed(js_rewriter: JsRewriter):
+    # Optional chaining, nullish coalescing and class fields are ordinary in
+    # shipped code; a parser that refuses them would silently fall back to the
+    # broken behaviour on exactly the pages most likely to need this.
+    out = js_rewriter.rewrite(
+        "const config = window?.settings ?? {};\nclass P { #id = 1; static all = []; }"
+    )
+    # Only the first carried name takes the leading ";".
+    assert ";self.___WB_const_config = config;\n" in out
+    assert "self.___WB_const_P = P;\n" in out
+
+
+def test_an_unparsable_script_is_wrapped_but_not_altered(js_rewriter: JsRewriter):
+    # A script Zimi cannot read is still a script it must not corrupt.
+    out = js_rewriter.rewrite("window.x = 1; const = = =;")
+    assert "___WB_const_" not in out
+    assert "const = = =;" in out
+
+
+def test_a_script_with_no_globals_is_unchanged_around_the_block(
+    js_rewriter: JsRewriter,
+):
+    out = js_rewriter.rewrite("window.x = 1;")
+    assert not out.startswith("let ")
+    assert "___WB_const_" not in out
+    assert out.endswith("\n\n}")
diff --git a/tests/rewriting/test_js_rewriting.py b/tests/rewriting/test_js_rewriting.py
index ab65e81..2212342 100644
--- a/tests/rewriting/test_js_rewriting.py
+++ b/tests/rewriting/test_js_rewriting.py
@@ -141,18 +141,43 @@ def __init__(
         input_: str | bytes,
         expected: str | bytes | None = None,
         article_url: str = "https://kiwix.org",
+        *,
+        without: tuple[str, ...] = (),
+        before: str = "",
+        after: str = "",
     ) -> None:
+        """``without`` names globals the script declares itself, so the wrapper
+        must not declare them too; ``before`` and ``after`` are what the
+        hoisting of the script's own top-level declarations adds around the
+        block (see #329)."""
         super().__init__(input_=input_, expected=expected, article_url=article_url)
-        self.expected = self.wrap_script(self.expected_str)
+        self.expected = (
+            before + self.wrap_script(self.expected_str, without=without) + after
+        )
 
     @staticmethod
-    def wrap_script(text: str) -> str:
+    def wrap_script(text: str, without: tuple[str, ...] = ()) -> str:
         """
         A small wrapper to help generate the expected content.
 
         JsRewriter must add this local definition around all js code (when we access on
         of the local varibles)
         """
+        decls = "".join(
+            f'let {name} = _____WB$wombat$assign$function_____("{name}");\n'
+            for name in (
+                "window",
+                "globalThis",
+                "self",
+                "document",
+                "location",
+                "top",
+                "parent",
+                "frames",
+                "opener",
+            )
+            if name not in without
+        )
         return (
             "var _____WB$wombat$assign$function_____ = function(name) {return (self."
             "_wb_wombat && self._wb_wombat.local_init && self._wb_wombat.local_init"
@@ -160,15 +185,7 @@ def wrap_script(text: str) -> str:
             "if (!self.__WB_pmw) { self.__WB_pmw = function(obj) { this.__WB_source ="
             " obj; return this; } }\n"
             "{\n"
-            'let window = _____WB$wombat$assign$function_____("window");\n'
-            'let globalThis = _____WB$wombat$assign$function_____("globalThis");\n'
-            'let self = _____WB$wombat$assign$function_____("self");\n'
-            'let document = _____WB$wombat$assign$function_____("document");\n'
-            'let location = _____WB$wombat$assign$function_____("location");\n'
-            'let top = _____WB$wombat$assign$function_____("top");\n'
-            'let parent = _____WB$wombat$assign$function_____("parent");\n'
-            'let frames = _____WB$wombat$assign$function_____("frames");\n'
-            'let opener = _____WB$wombat$assign$function_____("opener");\n'
+            f"{decls}"
             "let arguments;\n"
             "\n"
             f"{text}"
@@ -218,6 +235,9 @@ def wrap_script(text: str) -> str:
             expected="if (self.foo) { console.log('blah') }",
         ),
         WrappedTestContent(input_="window.x = 5", expected="window.x = 5"),
+        # A script's own top-level declarations survive the block (#329):
+        # `let` is declared before it and assigned inside, `const` and `class`
+        # are handed out through a carrier and re-declared after it.
         WrappedTestContent(
             input_="""
             class A {}
@@ -229,12 +249,18 @@ class A {}
             expected="""
             class A {}
             const B = 5;
-            let C = 4;
+             C = 4;
             var D = 3;
 
             location = ((self.__WB_check_loc && """
             "self.__WB_check_loc(location, [])) || {}).maybeHref "
-            """= "http://example.com/2" """,
+            """= "http://example.com/2" """
+            "\n;self.___WB_const_A = A;\nself.___WB_const_B = B;\n",
+            before="let C;\n",
+            after=(
+                "\nconst A = self.___WB_const_A; delete self.___WB_const_A;\n"
+                "const B = self.___WB_const_B; delete self.___WB_const_B;\n"
+            ),
         ),
         # Ensure these *don't* get rewritten
         WrappedTestContent(
@@ -257,7 +283,14 @@ class A {}
                 " _____WB$wombat$check$this$function_____(this).location; }"
             ),
         ),
-        WrappedTestContent(input_=" var    self  ", expected=" let    self  "),
+        # The script declares its own `self`, so the wrapper must not declare
+        # one as well: before #329 was fixed this produced `let self` twice in
+        # the same block, which is a SyntaxError and killed the whole script.
+        WrappedTestContent(
+            input_=" var    self  ",
+            expected=" let    self  ",
+            without=("self",),
+        ),
     ]
 )
 def rewrite_wrapped_content(request: pytest.FixtureRequest):

From bc2b0561f1902a1b5eb7db062d7567a2eb87c568 Mon Sep 17 00:00:00 2001
From: Eric Pheterson <epheterson@gmail.com>
Date: Mon, 7 Sep 2026 21:59:32 -0700
Subject: [PATCH 2/3] Port the rest of wabac.js's globals tests, and harden the
 walk

Adds the remaining cases from wabac.js's test/rewriteJS.ts that exercise
this branch: several declarators on one line, let+var+const together, and
a carried const alongside a top-level document.write().

The AST walk now sits inside the same try/except as the parse, matching
wabac.js, which wraps its whole parseGlobals call. Nothing here may throw
into a scrape.

Adds the changelog entry.
---
 CHANGELOG.md                          |  4 ++++
 src/zimscraperlib/rewriting/js_ast.py | 13 ++++++++---
 tests/rewriting/test_js_globals.py    | 31 +++++++++++++++++++++++++++
 3 files changed, 45 insertions(+), 3 deletions(-)

diff --git a/CHANGELOG.md b/CHANGELOG.md
index 2ab016e..5f05d16 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -7,6 +7,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 
 ## [Unreleased]
 
+### Fixed
+
+- Carry a script's top-level `const`, `let` and `class` declarations across the wombat block, so other scripts on the page can still see them (#329)
+
 ## [5.4.1] - 2026-07-31
 
 ### Fixed
diff --git a/src/zimscraperlib/rewriting/js_ast.py b/src/zimscraperlib/rewriting/js_ast.py
index f0f29e5..e687cf6 100644
--- a/src/zimscraperlib/rewriting/js_ast.py
+++ b/src/zimscraperlib/rewriting/js_ast.py
@@ -115,10 +115,17 @@ def parse_top_level(text: str) -> TopLevel | None:
         return None
     source = text.encode("utf-8")
     try:
-        tree = parser.parse(source)
-    except Exception:  # noqa: BLE001 - a parser crash must not fail a scrape
+        return _walk(parser.parse(source), source)
+    except Exception:  # noqa: BLE001 - nothing here may fail a scrape
+        # wabac.js wraps its whole parseGlobals in a try/catch, not just the
+        # parse, and this keeps that posture: any surprise from the parser or
+        # from walking what it returned means "no opinion", not an exception
+        # escaping into a scrape.
         return None
-    root = tree.root_node
+
+
+def _walk(tree, source: bytes) -> TopLevel | None:
+    root = tree.root_node if tree is not None else None
     if root is None:
         return None
 
diff --git a/tests/rewriting/test_js_globals.py b/tests/rewriting/test_js_globals.py
index 757b773..31fde90 100644
--- a/tests/rewriting/test_js_globals.py
+++ b/tests/rewriting/test_js_globals.py
@@ -104,6 +104,37 @@ class A {}
     ) in out
 
 
+def test_several_declarators_on_one_line(js_rewriter: JsRewriter):
+    # wabac.js: "multiple globals on same line". Each `let` name is declared
+    # before the block, but the statement's keyword is removed only once.
+    out = js_rewriter.rewrite(
+        "let a = document.location.href, b = 1, c = 2;\nconst foo = 4, bar = 5"
+    )
+    assert out.startswith("let a;\nlet b;\nlet c;\n")
+    assert "\n a = document.location.href, b = 1, c = 2;" in out
+    assert ";self.___WB_const_foo = foo;\nself.___WB_const_bar = bar;\n" in out
+    assert (
+        "const foo = self.___WB_const_foo; delete self.___WB_const_foo;\n"
+        "const bar = self.___WB_const_bar; delete self.___WB_const_bar;\n"
+    ) in out
+
+
+def test_a_carried_const_and_a_document_write_together(js_rewriter: JsRewriter):
+    # wabac.js: "global + document.close append". The close goes after the
+    # carrier assignment and still inside the block.
+    out = js_rewriter.rewrite("\n\nconst y = document.location;\ndocument.write(x);")
+    assert ";self.___WB_const_y = y;\n;document.close();" in out
+    assert "const y = self.___WB_const_y; delete self.___WB_const_y;\n" in out
+
+
+def test_let_var_and_const_in_one_script(js_rewriter: JsRewriter):
+    # wabac.js: "add global injection".
+    out = js_rewriter.rewrite("let a = document.location.href; var b = 5; const foo = 4")
+    assert out.startswith("let a;\n")
+    assert "\n a = document.location.href; var b = 5; const foo = 4" in out
+    assert ";self.___WB_const_foo = foo;\n" in out
+
+
 def test_nothing_is_carried_out_of_a_nested_scope(js_rewriter: JsRewriter):
     # Only the top level can leak a global; a const inside a function is the
     # function's business and must be left exactly as written.

From 974c11f848d579858fa2cc83a1cc8be8a4e88176 Mon Sep 17 00:00:00 2001
From: Eric Pheterson <epheterson@gmail.com>
Date: Tue, 8 Sep 2026 03:27:54 -0700
Subject: [PATCH 3/3] Fix QA: linting, strict types, and 100% coverage

Linting: imports moved to the top of the file, unused noqa directives
dropped, one long line wrapped. tree-sitter is a declared dependency, so
the guarded import it had was neither needed nor lintable.

Types: pyright strict had no types for the parser because nothing
annotated the Node parameters. Annotated, and _text now takes an optional
node and reads a missing one as empty, so its callers ask for optional
fields without a guard at each site.

Coverage is back to 100% with branches on both files. Two of the
uncovered branches were dead code rather than untested code, and are
gone: a lexical_declaration is only ever const or let (a "using"
declaration is its own node type in the grammar), and a member_expression
always has both of its fields. parse_top_level's own try/except is the
net for any parser surprise, and that net is now tested directly.

The rest is covered by four new cases: a destructured declaration is left
alone, an inline script stays on one line, a script the parser cannot
read is wrapped unchanged, and a parser that raises answers None rather
than throwing into a scrape.
---
 src/zimscraperlib/rewriting/js_ast.py | 134 ++++++++++----------------
 tests/rewriting/test_js_globals.py    |  55 +++++++++--
 2 files changed, 102 insertions(+), 87 deletions(-)

diff --git a/src/zimscraperlib/rewriting/js_ast.py b/src/zimscraperlib/rewriting/js_ast.py
index e687cf6..ffc27c9 100644
--- a/src/zimscraperlib/rewriting/js_ast.py
+++ b/src/zimscraperlib/rewriting/js_ast.py
@@ -8,11 +8,11 @@
 function so the choice of parser is one import to change.
 
 Only the top level matters. Nothing nested can leak a global, so this never
-walks into a function body, and it answers four questions:
+walks into a function body, and it answers three questions:
 
-  * which `const` / `let` / `class` names the script declares at the top level
-  * where each `let` statement starts, so the keyword can be removed
-  * which of the names shadow a global the wrapper is about to declare
+  * which `const`, `let`, `var` and `class` names the script declares at the
+    top level, and of what kind
+  * where each declaration starts, so a `let` keyword can be removed
   * whether the script calls `document.write()` at the top level
 
 Why tree-sitter and not a pure-Python parser: the scripts this runs on are
@@ -25,10 +25,14 @@
 
 from __future__ import annotations
 
-import functools
 from dataclasses import dataclass
 
-__all__ = ["Declaration", "TopLevel", "parse_top_level", "parser_available"]
+import tree_sitter_javascript
+from tree_sitter import Language, Node, Parser
+
+__all__ = ["Declaration", "TopLevel", "parse_top_level"]
+
+_PARSER = Parser(Language(tree_sitter_javascript.language()))
 
 
 @dataclass(frozen=True)
@@ -46,108 +50,76 @@ class TopLevel:
     has_document_write: bool
 
 
-@functools.lru_cache(maxsize=1)
-def _parser():
-    """The parser, built once. None when tree-sitter is not installed, which
-    is not an error: the rewriter falls back to its unparsed behaviour."""
-    try:
-        import tree_sitter_javascript
-        from tree_sitter import Language, Parser
-
-        return Parser(Language(tree_sitter_javascript.language()))
-    except Exception:  # noqa: BLE001 - any import or ABI trouble means no parser
-        return None
-
-
-def parser_available() -> bool:
-    return _parser() is not None
-
-
-def _text(node, source: bytes) -> str:
+def _text(node: Node | None, source: bytes) -> str:
+    """The source a node covers. A missing node reads as no text, so callers
+    can ask for an optional field without a guard at every site."""
+    if node is None:
+        return ""
     return source[node.start_byte : node.end_byte].decode("utf-8", errors="replace")
 
 
-def _identifiers(node, source: bytes) -> list[str]:
-    """The plain identifiers a declarator binds.
+def _identifiers(node: Node, source: bytes) -> list[str]:
+    """The plain identifiers a declaration binds.
 
     Destructuring (`const {a, b} = x`) is deliberately skipped, exactly as
     wabac.js skips anything whose id is not an Identifier: hoisting a
     destructured binding would mean rebuilding the pattern, and the names it
     binds are rare enough at the top level of a script to be worth leaving
     alone rather than getting subtly wrong."""
-    names = []
+    names: list[str] = []
     for child in node.named_children:
-        if child.type == "variable_declarator":
-            first = child.child_by_field_name("name")
-            if first is not None and first.type == "identifier":
-                names.append(_text(first, source))
+        name = child.child_by_field_name("name")
+        if name is not None and name.type == "identifier":
+            names.append(_text(name, source))
     return names
 
 
-def _is_document_write(node, source: bytes) -> bool:
-    if node.type != "expression_statement":
+def _is_document_write(node: Node, source: bytes) -> bool:
+    if node.type != "expression_statement" or not node.named_children:
         return False
-    call = node.named_children[0] if node.named_children else None
-    if call is None or call.type != "call_expression":
+    call = node.named_children[0]
+    if call.type != "call_expression":
         return False
     callee = call.child_by_field_name("function")
     if callee is None or callee.type != "member_expression":
         return False
+    # A member expression always has both fields; anything else is a parser
+    # surprise, and parse_top_level's own net catches those.
     obj = callee.child_by_field_name("object")
     prop = callee.child_by_field_name("property")
-    return (
-        obj is not None
-        and prop is not None
-        and obj.type == "identifier"
-        and prop.type == "property_identifier"
-        and _text(obj, source) == "document"
-        and _text(prop, source) == "write"
-    )
+    return _text(obj, source) == "document" and _text(prop, source) == "write"
 
 
 def parse_top_level(text: str) -> TopLevel | None:
     """Read a script's top-level declarations, or None when it cannot be read.
 
-    None means "no opinion" and the caller must fall back to leaving the
-    script alone, which is what happened before this existed."""
-    parser = _parser()
-    if parser is None:
-        return None
-    source = text.encode("utf-8")
+    None means "no opinion", and the caller leaves the script alone — which is
+    what happened to every script before this existed. wabac.js wraps its whole
+    parseGlobals in a try/catch for the same reason, and so does this: nothing
+    here may throw into a scrape."""
     try:
-        return _walk(parser.parse(source), source)
-    except Exception:  # noqa: BLE001 - nothing here may fail a scrape
-        # wabac.js wraps its whole parseGlobals in a try/catch, not just the
-        # parse, and this keeps that posture: any surprise from the parser or
-        # from walking what it returned means "no opinion", not an exception
-        # escaping into a scrape.
-        return None
-
-
-def _walk(tree, source: bytes) -> TopLevel | None:
-    root = tree.root_node if tree is not None else None
-    if root is None:
-        return None
-
-    declarations: list[Declaration] = []
-    has_document_write = False
-    for node in root.named_children:
-        if node.type == "lexical_declaration":
-            # `const` / `let`: the keyword is the first token of the statement.
-            kind = _text(node.children[0], source) if node.children else ""
-            if kind in ("const", "let"):
+        source = text.encode("utf-8")
+        root = _PARSER.parse(source).root_node
+        declarations: list[Declaration] = []
+        has_document_write = False
+        for node in root.named_children:
+            if node.type == "lexical_declaration":
+                # `const` or `let` — `using` has its own node type.
+                kind = _text(node.children[0], source)
                 for name in _identifiers(node, source):
                     declarations.append(Declaration(name, kind, node.start_byte))
-        elif node.type == "variable_declaration":
-            for name in _identifiers(node, source):
-                declarations.append(Declaration(name, "var", node.start_byte))
-        elif node.type == "class_declaration":
-            named = node.child_by_field_name("name")
-            if named is not None:
+            elif node.type == "variable_declaration":
+                for name in _identifiers(node, source):
+                    declarations.append(Declaration(name, "var", node.start_byte))
+            elif node.type == "class_declaration":
+                name_node = node.child_by_field_name("name")
                 declarations.append(
-                    Declaration(_text(named, source), "class", node.start_byte)
+                    Declaration(_text(name_node, source), "class", node.start_byte)
                 )
-        elif not has_document_write and _is_document_write(node, source):
-            has_document_write = True
-
-    return TopLevel(declarations=declarations, has_document_write=has_document_write)
+            elif not has_document_write and _is_document_write(node, source):
+                has_document_write = True
+        return TopLevel(
+            declarations=declarations, has_document_write=has_document_write
+        )
+    except Exception:
+        return None
diff --git a/tests/rewriting/test_js_globals.py b/tests/rewriting/test_js_globals.py
index 31fde90..99aca67 100644
--- a/tests/rewriting/test_js_globals.py
+++ b/tests/rewriting/test_js_globals.py
@@ -15,14 +15,10 @@
 
 import pytest
 
+from zimscraperlib.rewriting import js_ast
 from zimscraperlib.rewriting.js import JsRewriter
-from zimscraperlib.rewriting.js_ast import parser_available
 from zimscraperlib.rewriting.url_rewriting import ArticleUrlRewriter
 
-pytestmark = pytest.mark.skipif(
-    not parser_available(), reason="no JavaScript parser installed"
-)
-
 
 @pytest.fixture
 def js_rewriter(
@@ -129,7 +125,9 @@ def test_a_carried_const_and_a_document_write_together(js_rewriter: JsRewriter):
 
 def test_let_var_and_const_in_one_script(js_rewriter: JsRewriter):
     # wabac.js: "add global injection".
-    out = js_rewriter.rewrite("let a = document.location.href; var b = 5; const foo = 4")
+    out = js_rewriter.rewrite(
+        "let a = document.location.href; var b = 5; const foo = 4"
+    )
     assert out.startswith("let a;\n")
     assert "\n a = document.location.href; var b = 5; const foo = 4" in out
     assert ";self.___WB_const_foo = foo;\n" in out
@@ -169,3 +167,48 @@ def test_a_script_with_no_globals_is_unchanged_around_the_block(
     assert not out.startswith("let ")
     assert "___WB_const_" not in out
     assert out.endswith("\n\n}")
+
+
+def test_a_destructured_declaration_is_left_alone(js_rewriter: JsRewriter):
+    # Carrying `const {a, b} = x` across the block would mean rebuilding the
+    # pattern; wabac.js skips these for the same reason.
+    out = js_rewriter.rewrite("const {a, b} = window.data;\nwindow.x = 1;")
+    assert "___WB_const_" not in out
+    assert "const {a, b} = window.data;" in out
+
+
+def test_an_inline_script_stays_on_one_line(js_rewriter: JsRewriter):
+    # An inline script is rewritten into an attribute, where a newline would
+    # end it. The hoisting adds lines, so the collapse happens after it too.
+    out = js_rewriter.rewrite("const a = 1;\nwindow.x = 2;", opts={"inline": True})
+    assert "\n" not in out
+    assert "self.___WB_const_a = a;" in out
+
+
+def test_a_script_the_parser_cannot_read_is_wrapped_unchanged(
+    js_rewriter: JsRewriter, monkeypatch: pytest.MonkeyPatch
+):
+    # parse_top_level answers None for anything it cannot make sense of, and
+    # the rewriter must then do exactly what it did before this existed.
+    monkeypatch.setattr(
+        "zimscraperlib.rewriting.js.parse_top_level", lambda _text: None
+    )
+    out = js_rewriter.rewrite("const glyphs = {a: 1};\nwindow.x = 1;")
+    assert "___WB_const_" not in out
+    assert "const glyphs = {a: 1};" in out
+    assert out.endswith("\n\n}")
+
+
+def test_the_parser_never_throws_into_a_scrape(monkeypatch: pytest.MonkeyPatch):
+    class _Boom:
+        def parse(self, _source):
+            raise RuntimeError("the parser fell over")
+
+    monkeypatch.setattr(js_ast, "_PARSER", _Boom())
+    assert js_ast.parse_top_level("const a = 1;") is None
+
+
+def test_a_missing_node_reads_as_no_text():
+    # `_text` takes optional fields so its callers need no guard; an absent
+    # one is empty, never an exception.
+    assert js_ast._text(None, b"anything") == ""