mirror of
https://github.com/pewdiepie-archdaemon/odysseus.git
synced 2026-10-06 06:52:20 +02:00
refactor(email): move emailLibrary.js into static/js/emailLibrary/
The email library was 11,375 lines in one file, the second-largest JS module in the repo. `static/js/emailLibrary/` already held four extracted helpers, so the package existed; the bulk of the code just was not in it. The implementation moves to `emailLibrary/index.js` and the old path becomes a re-export wrapper. Five call sites import that path, four of them dynamically with a `?v=` string, and `sw.js` caches URLs verbatim, so a wrapper is what makes the move need no coordinated edit to any of them. The test side is the part worth reviewing. 58 tests read `static/js/emailLibrary.js` as text. Pointing them at `emailLibrary/index.js` would buy one move and break again on the next one, which is exactly what happened to the stylesheet tests (they now go through `tests/helpers/stylesheets.py`). So the same shape: `tests/helpers/js_modules.py` reads the whole package, and assertions stop caring which module a function sits in. `tests/test_email_library_module_graph_js.py` is new. This frontend has no module-graph validation, and a package fails in ways a single file cannot: a wrapper that drops an export is `undefined` at call time rather than an error at load time, and a module that reads a `const` across an import cycle throws only when that module is entered first. It pins the wrapper's surface against the entry module's, evaluates every module on its own in a browser, and requires both import paths to hand out one instance. It also caught a live gap while being written: `emailLibrary/replyRecipients.js` is imported by `emailInbox.js`, an app shell module, and was never in the `sw.js` precache.
This commit is contained in:
@@ -0,0 +1,187 @@
|
||||
"""Module-graph coverage for the split email library.
|
||||
|
||||
``specs/frontend.md`` records that this frontend has "no build-time type
|
||||
checking, module graph validation, or script-order validation". That is
|
||||
tolerable for a single 11k-line file and not tolerable for a package: splitting
|
||||
``emailLibrary.js`` into modules that import each other adds three failure modes
|
||||
nothing else here would catch.
|
||||
|
||||
1. The compatibility wrapper at ``static/js/emailLibrary.js`` drops an export.
|
||||
Five call sites import that path; a missing name is ``undefined`` at call
|
||||
time, not an error at load time, so the panel just stops responding.
|
||||
2. A module in the package fails to evaluate — a stale relative specifier after
|
||||
a move, or a temporal-dead-zone read across an import cycle. The package has
|
||||
cycles by construction: extracted modules call back into ``index.js`` from
|
||||
event handlers. That is safe for hoisted function declarations and *not* safe
|
||||
for a ``const`` read while the graph is still evaluating, and which one you
|
||||
wrote is invisible in a diff.
|
||||
3. A new module is missing from the ``sw.js`` precache, so the panel that works
|
||||
online cannot open offline.
|
||||
|
||||
The first and third are read off the source. The second needs a real module
|
||||
loader, so it runs in a browser: each module is imported *on its own*, in a
|
||||
fresh page, because entering the cycle at a submodule rather than at the entry
|
||||
module is the order that exposes a dead-zone read.
|
||||
"""
|
||||
|
||||
import json
|
||||
import re
|
||||
import subprocess
|
||||
from pathlib import Path
|
||||
|
||||
from tests.helpers.js_modules import (
|
||||
EMAIL_LIBRARY_ENTRY,
|
||||
EMAIL_LIBRARY_WRAPPER,
|
||||
email_library_paths,
|
||||
)
|
||||
|
||||
ROOT = Path(__file__).resolve().parents[1]
|
||||
_SW = ROOT / "static" / "sw.js"
|
||||
|
||||
# `export function foo`, `export async function foo`, `export const foo`.
|
||||
_EXPORT_DECL = re.compile(
|
||||
r"^export\s+(?:async\s+)?(?:function|const|let|class)\s+([A-Za-z_$][\w$]*)",
|
||||
re.M,
|
||||
)
|
||||
# `export { a, b } from '...'` and `export { a, b }`.
|
||||
_EXPORT_LIST = re.compile(r"export\s*\{([^}]*)\}", re.S)
|
||||
|
||||
|
||||
def _declared_exports(path: Path) -> set[str]:
|
||||
return set(_EXPORT_DECL.findall(path.read_text(encoding="utf-8")))
|
||||
|
||||
|
||||
def _listed_exports(path: Path) -> set[str]:
|
||||
names: set[str] = set()
|
||||
for block in _EXPORT_LIST.findall(path.read_text(encoding="utf-8")):
|
||||
for raw in block.split(","):
|
||||
name = raw.strip().split(" as ")[-1].strip()
|
||||
if name:
|
||||
names.add(name)
|
||||
return names
|
||||
|
||||
|
||||
def test_wrapper_re_exports_the_entry_module_surface_exactly():
|
||||
"""The old path must expose the same names as the package entry module.
|
||||
|
||||
Not a subset and not a superset: a missing name breaks a caller silently,
|
||||
and a name the entry module no longer exports is a load-time error.
|
||||
"""
|
||||
entry = _declared_exports(EMAIL_LIBRARY_ENTRY) | _listed_exports(EMAIL_LIBRARY_ENTRY)
|
||||
wrapper = _listed_exports(EMAIL_LIBRARY_WRAPPER)
|
||||
assert wrapper, f"{EMAIL_LIBRARY_WRAPPER} re-exports nothing"
|
||||
assert wrapper == entry, (
|
||||
"static/js/emailLibrary.js and static/js/emailLibrary/index.js disagree "
|
||||
f"on the public surface; only in the wrapper: {sorted(wrapper - entry)}; "
|
||||
f"only in the entry module: {sorted(entry - wrapper)}"
|
||||
)
|
||||
|
||||
|
||||
def test_every_package_module_is_precached():
|
||||
sw = _SW.read_text(encoding="utf-8")
|
||||
for path in email_library_paths():
|
||||
url = "/static/js/emailLibrary/" + path.name
|
||||
assert f"'{url}'" in sw, (
|
||||
f"{url} is not in the static/sw.js precache, so a panel that works "
|
||||
"online will not open offline"
|
||||
)
|
||||
|
||||
|
||||
def test_every_package_module_evaluates_on_its_own_in_a_browser():
|
||||
"""Import each module first, alone, and require it to evaluate.
|
||||
|
||||
Entering the package at a submodule is what turns an import cycle from
|
||||
harmless into a `ReferenceError: cannot access '…' before initialization`.
|
||||
Importing the entry module first would hide exactly that.
|
||||
"""
|
||||
urls = ["/static/js/emailLibrary.js"] + [
|
||||
"/static/js/emailLibrary/" + p.name for p in email_library_paths()
|
||||
]
|
||||
script = r'''
|
||||
import { chromium } from 'playwright';
|
||||
const origin = process.env.ODYSSEUS_TEST_STATIC_ORIGIN;
|
||||
const urls = JSON.parse(process.env.ODYSSEUS_EMAIL_MODULE_URLS);
|
||||
const browser = await chromium.launch({ headless: true });
|
||||
const results = {};
|
||||
for (const url of urls) {
|
||||
const page = await browser.newPage();
|
||||
// Same synthetic shell the other email browser tests use: these
|
||||
// modules touch #toast and #sidebar while evaluating.
|
||||
await page.goto(`${origin}/static/js/documentStats.js`);
|
||||
await page.setContent('<div id="toast"></div><div id="chat-container"></div><div id="sidebar"></div>');
|
||||
results[url] = await page.evaluate(async (target) => {
|
||||
try {
|
||||
const mod = await import(target);
|
||||
return { ok: true, exports: Object.keys(mod).sort() };
|
||||
} catch (err) {
|
||||
return { ok: false, error: String(err && err.message || err) };
|
||||
}
|
||||
}, url);
|
||||
await page.close();
|
||||
}
|
||||
console.log(JSON.stringify(results));
|
||||
await browser.close();
|
||||
'''
|
||||
result = subprocess.run(
|
||||
["node", "--input-type=module", "-e", script],
|
||||
cwd=ROOT,
|
||||
check=False,
|
||||
capture_output=True,
|
||||
text=True,
|
||||
env={
|
||||
**__import__("os").environ,
|
||||
"ODYSSEUS_EMAIL_MODULE_URLS": json.dumps(urls),
|
||||
},
|
||||
)
|
||||
assert result.returncode == 0, result.stderr
|
||||
data = json.loads(result.stdout)
|
||||
broken = {url: info["error"] for url, info in data.items() if not info["ok"]}
|
||||
assert not broken, f"modules that failed to evaluate on their own: {broken}"
|
||||
assert set(data) == set(urls)
|
||||
|
||||
|
||||
def test_wrapper_and_entry_module_hand_out_the_same_functions():
|
||||
"""Importing either path must give one live module instance.
|
||||
|
||||
Two instances would mean two copies of the panel's module state, and the
|
||||
panel is a singleton keyed on DOM ids — the second copy would fight the
|
||||
first over `#email-lib-modal`.
|
||||
"""
|
||||
script = r'''
|
||||
import { chromium } from 'playwright';
|
||||
const origin = process.env.ODYSSEUS_TEST_STATIC_ORIGIN;
|
||||
const browser = await chromium.launch({ headless: true });
|
||||
const page = await browser.newPage();
|
||||
await page.goto(`${origin}/static/js/documentStats.js`);
|
||||
await page.setContent('<div id="toast"></div><div id="chat-container"></div><div id="sidebar"></div>');
|
||||
const out = await page.evaluate(async () => {
|
||||
const wrapper = await import('/static/js/emailLibrary.js');
|
||||
const entry = await import('/static/js/emailLibrary/index.js');
|
||||
const names = Object.keys(wrapper).sort();
|
||||
return {
|
||||
names,
|
||||
identical: names.filter((n) => wrapper[n] === entry[n]),
|
||||
callable: names.filter((n) => typeof wrapper[n] === 'function'),
|
||||
};
|
||||
});
|
||||
console.log(JSON.stringify(out));
|
||||
await browser.close();
|
||||
'''
|
||||
result = subprocess.run(
|
||||
["node", "--input-type=module", "-e", script],
|
||||
cwd=ROOT,
|
||||
check=False,
|
||||
capture_output=True,
|
||||
text=True,
|
||||
)
|
||||
assert result.returncode == 0, result.stderr
|
||||
data = json.loads(result.stdout)
|
||||
assert data["names"], "the wrapper exported nothing at runtime"
|
||||
assert data["identical"] == data["names"], (
|
||||
"the wrapper and the entry module handed out different objects for "
|
||||
f"{sorted(set(data['names']) - set(data['identical']))}"
|
||||
)
|
||||
assert data["callable"] == data["names"], (
|
||||
"not every export is callable: "
|
||||
f"{sorted(set(data['names']) - set(data['callable']))}"
|
||||
)
|
||||
Reference in New Issue
Block a user