mirror of
https://github.com/pewdiepie-archdaemon/odysseus.git
synced 2026-10-06 15:02:20 +02:00
fix(email): import spinnerModule in the modules that use it
Six of the seven modules call `spinnerModule.createWhirlpool(...)` and none of them imported it. Everything still parsed, every module still loaded, and the Email Settings page threw `ReferenceError: spinnerModule is not defined` the moment `settings.js` mounted it — the panel rendered its loading spinner and stopped there. The cause is worth writing down because it is the same shape as the bug: the import statements were collected with a pattern that was not anchored to the start of a line, and the header comment says "the old import path keeps resolving". That prose matched first and swallowed the real `import spinnerModule from '../spinner.js';` that followed it. `test_no_module_uses_a_package_name_it_never_bound` closes the class. Nothing else here can: `node --check` parses without resolving scope, and loading a module does not run the function body where the throw lives. It checks the package's own vocabulary — every name any module in it binds — rather than trying to model the browser's globals, so it has no false positives and still catches the one mistake a split actually makes: the declaration stays behind and the use moves.
This commit is contained in:
@@ -8,6 +8,7 @@
|
||||
// The context-draft key is versioned (`…:v2:`) and scoped by account, folder
|
||||
// and uid, so a draft cannot leak from one message to another.
|
||||
|
||||
import spinnerModule from '../spinner.js';
|
||||
import { topPortalZ } from '../toolWindowZOrder.js';
|
||||
import { showToast } from '../ui.js?v=20260916largetoolscroll1';
|
||||
import { state } from './state.js';
|
||||
|
||||
@@ -8,6 +8,7 @@
|
||||
// metadata: a card can know it *has* attachments without knowing what they
|
||||
// are, so the chips are rendered late and the card icon repaired afterwards.
|
||||
|
||||
import spinnerModule from '../spinner.js';
|
||||
import * as Modals from '../modalManager.js';
|
||||
import { state } from './state.js';
|
||||
import { _esc } from './utils.js';
|
||||
|
||||
@@ -9,6 +9,7 @@
|
||||
// document, task, session and memory menus. `tests/test_action_menu_order.py`
|
||||
// pins that.
|
||||
|
||||
import spinnerModule from '../spinner.js';
|
||||
import { SELECT_MENU_ICON, actionMenuRank, orderActionMenuItems } from '../actionMenuOrder.js';
|
||||
import { bindMenuDismiss, dismissOrRemove } from '../escMenuStack.js';
|
||||
import { topPortalZ } from '../toolWindowZOrder.js';
|
||||
|
||||
@@ -9,6 +9,7 @@
|
||||
// tab 2 until it closes, even if tab 1 closes first. The slot map here is what
|
||||
// holds that.
|
||||
|
||||
import spinnerModule from '../spinner.js';
|
||||
import * as Modals from '../modalManager.js';
|
||||
import { showToast } from '../ui.js?v=20260916largetoolscroll1';
|
||||
import { state } from './state.js';
|
||||
|
||||
@@ -10,6 +10,7 @@
|
||||
// delete sync against /api/calendar. And the inline-image preference is read by
|
||||
// ./bodyRender.js while rendering a message, not only by the settings form.
|
||||
|
||||
import spinnerModule from '../spinner.js';
|
||||
import { emailApiUrl } from '../emailShared.js';
|
||||
import { showToast } from '../ui.js?v=20260916largetoolscroll1';
|
||||
import { state } from './state.js';
|
||||
|
||||
@@ -10,6 +10,7 @@
|
||||
// agent-browser run finish. Nothing else in the package reads any of that,
|
||||
// which is why this is the one module with a single exported entry point.
|
||||
|
||||
import spinnerModule from '../spinner.js';
|
||||
import { emailApiUrl } from '../emailShared.js';
|
||||
import { showToast, styledConfirm } from '../ui.js?v=20260916largetoolscroll1';
|
||||
import { state } from './state.js';
|
||||
|
||||
@@ -127,6 +127,84 @@ def test_wrapper_exposes_every_statically_imported_name():
|
||||
assert checked, "no module imports names from static/js/emailLibrary.js"
|
||||
|
||||
|
||||
_IMPORT_STATEMENT = re.compile(
|
||||
r"^import\s+(\{[^}]*\}|\*\s+as\s+[\w$]+|[\w$]+)\s+from\s+'[^']+';", re.M | re.S
|
||||
)
|
||||
_TOP_LEVEL_DECL = re.compile(
|
||||
r"^(?:export\s+)?(?:async\s+)?(?:function|const|let|var)\s+([A-Za-z_$][\w$]*)", re.M
|
||||
)
|
||||
|
||||
|
||||
def _bound_names(source: str) -> set[str]:
|
||||
"""Local names a module binds: imports plus top-level declarations."""
|
||||
names = set(_TOP_LEVEL_DECL.findall(source))
|
||||
for clause in _IMPORT_STATEMENT.findall(source):
|
||||
clause = clause.strip()
|
||||
if clause.startswith("{"):
|
||||
names |= {
|
||||
part.strip().split(" as ")[-1].strip()
|
||||
for part in clause.strip("{}").split(",")
|
||||
if part.strip()
|
||||
}
|
||||
else:
|
||||
names.add(clause.split(" as ")[-1].strip())
|
||||
return names
|
||||
|
||||
|
||||
_REEXPORT = re.compile(r"^export\s*\{[^}]*\}\s*from\s*'[^']+';", re.M | re.S)
|
||||
_WHOLE_LINE_COMMENT = re.compile(r"^\s*(?://|/\*|\*/|\*(?!/)).*$", re.M)
|
||||
|
||||
|
||||
def _executable_body(source: str) -> str:
|
||||
"""The part of a module that actually runs a name.
|
||||
|
||||
Imports are dropped because a name in an import clause is the binding, not a
|
||||
use. Re-export lists go for the same reason. Whole-line comments go because
|
||||
prose says things like "no shared state" and `state` is a real binding here;
|
||||
only full lines are stripped, so a `//` inside a URL literal is left alone.
|
||||
"""
|
||||
ends = [m.end() for m in _IMPORT_STATEMENT.finditer(source)]
|
||||
body = source[max(ends):] if ends else source
|
||||
body = _REEXPORT.sub("", body)
|
||||
return _WHOLE_LINE_COMMENT.sub("", body)
|
||||
|
||||
|
||||
def test_no_module_uses_a_package_name_it_never_bound():
|
||||
"""A name used but never imported is a runtime ReferenceError, nothing less.
|
||||
|
||||
It is invisible to `node --check`, which parses without resolving scope, and
|
||||
invisible to loading the module, because the throw happens inside a function
|
||||
body the loader never calls. It is also the single most likely mistake when
|
||||
code moves between modules — the declaration stays behind and the use comes
|
||||
along.
|
||||
|
||||
The vocabulary checked is the package's own: every name any module here
|
||||
binds. That is narrow on purpose. It is not a general no-undef pass, so it
|
||||
never has to model the browser's globals and never produces a false
|
||||
positive; it catches exactly the shape a split produces.
|
||||
"""
|
||||
sources = {p.name: p.read_text(encoding="utf-8") for p in email_library_paths(True)}
|
||||
vocabulary: set[str] = set()
|
||||
for source in sources.values():
|
||||
vocabulary |= _bound_names(source)
|
||||
|
||||
unbound: dict[str, list[str]] = {}
|
||||
for name, source in sources.items():
|
||||
bound = _bound_names(source)
|
||||
body = _executable_body(source)
|
||||
missing = sorted(
|
||||
word
|
||||
for word in vocabulary - bound
|
||||
if re.search(r"(?<![\w$.])" + re.escape(word) + r"(?![\w$])", body)
|
||||
)
|
||||
if missing:
|
||||
unbound[name] = missing
|
||||
assert not unbound, (
|
||||
"modules use names they neither declare nor import, which throws at call "
|
||||
f"time and nowhere earlier: {unbound}"
|
||||
)
|
||||
|
||||
|
||||
def test_every_package_module_is_precached():
|
||||
sw = _SW.read_text(encoding="utf-8")
|
||||
for path in email_library_paths():
|
||||
|
||||
Reference in New Issue
Block a user