Carry a script's top-level declarations across the wombat block

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

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"]

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

src/zimscraperlib/rewriting/js_ast.py

@@ -0,0 +1,153 @@
+"""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:
+        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"):
+                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)