Remove module ui.js support: unused by any module or the live app
Drops the registry lookup, manifest field, /modules route, ui.js loading in panels.js, MODULE_RULES lint, services.js, their tests and docs. Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01EymCrVCefq4m9Uv6MAkzz5
This commit is contained in:
Claude
@@ -1,12 +1,12 @@
|
||||
// Hide, don't warn -- shown rather than asserted.
|
||||
//
|
||||
// One good module and five broken ones, put through the SAME shell code the app
|
||||
// One good module and four broken ones, put through the SAME shell code the app
|
||||
// uses. The good one's panel appears. The broken ones do not appear at all: no
|
||||
// placeholder, no greyed row, no warning chip. What you see on this page is the
|
||||
// whole of what a user would see.
|
||||
//
|
||||
// These mirror core/tests/fixtures/fixture_modules, so the browser and the
|
||||
// Python suite are looking at the same five ways of being wrong.
|
||||
// Python suite are looking at the same four ways of being wrong.
|
||||
|
||||
export const GOOD = {
|
||||
id: "audio_clock",
|
||||
@@ -31,11 +31,6 @@ export const GOOD = {
|
||||
// Each of these is refused somewhere different, which is the point: they are
|
||||
// caught at the earliest layer that can catch them.
|
||||
export const BROKEN = [
|
||||
{
|
||||
why: "raises on import",
|
||||
caughtBy: "core: the scan imports it in a try",
|
||||
spec: { id: "explodes", title: "Explodes", mount: "demo.panel", settings: {}, ui: "/nope/missing.js" },
|
||||
},
|
||||
{
|
||||
why: "asks for a renderer its type does not have",
|
||||
caughtBy: "core: schema validation, before the manifest",
|
||||
|
||||
@@ -22,7 +22,7 @@ async function render(theme) {
|
||||
const section = el("section", "cat-section");
|
||||
section.append(el("h2", null, "What a user sees"));
|
||||
section.append(el("p", null,
|
||||
"Six modules go in: one valid, five broken in different ways. Everything below " +
|
||||
"Five modules go in: one valid, four broken in different ways. Everything below " +
|
||||
"is the whole of what reaches the screen."));
|
||||
|
||||
const pane = el("section", "cat-pane");
|
||||
|
||||
@@ -1,7 +0,0 @@
|
||||
export function setup({ composer, values }) {
|
||||
const button = composer.button.md({
|
||||
label: "Re-key",
|
||||
onClick: () => values.set("enabled", true),
|
||||
});
|
||||
return button;
|
||||
}
|
||||
@@ -1,12 +0,0 @@
|
||||
export function setup({ composer }) {
|
||||
const b = composer.button.md({ label: "x" });
|
||||
b.node.style.color = "#ff0000";
|
||||
b.node.classList.add("mine");
|
||||
b.node.innerHTML = "<b>hi</b>";
|
||||
const smuggled = '<div style="color: red">still styling</div>';
|
||||
const raw = document.createElement("div");
|
||||
raw.setAttribute("style", "margin: 12px");
|
||||
document.head.appendChild(raw);
|
||||
void smuggled;
|
||||
return new Function("return 1")();
|
||||
}
|
||||
@@ -0,0 +1,44 @@
|
||||
// The kit and the shell never interpret content. Modules ship no JavaScript, so
|
||||
// there is nothing to scan in modules/.
|
||||
|
||||
import test from "node:test";
|
||||
import assert from "node:assert/strict";
|
||||
import { fileURLToPath } from "node:url";
|
||||
import { dirname, join } from "node:path";
|
||||
|
||||
import { scan, scanSource, code } from "../../../../tools/kit_lint/scan.js";
|
||||
import { KIT_RULES } from "../../../../tools/kit_lint/rules.js";
|
||||
|
||||
const here = dirname(fileURLToPath(import.meta.url));
|
||||
const repo = join(here, "..", "..", "..", "..");
|
||||
|
||||
test("the kit never interprets content", () => {
|
||||
// The kit may style and may build elements; that is its job. What it may not
|
||||
// do is parse anything a module gave it.
|
||||
const violations = scan(join(repo, "app", "ui", "composer"), KIT_RULES);
|
||||
assert.deepEqual(violations, [],
|
||||
violations.map((v) => `${v.file}:${v.line} ${v.rule}`).join("\n"));
|
||||
});
|
||||
|
||||
test("the shell never interprets content either", () => {
|
||||
assert.deepEqual(scan(join(repo, "app", "ui", "shell"), KIT_RULES), []);
|
||||
});
|
||||
|
||||
// --- the scanner has to actually catch things ------------------------------
|
||||
|
||||
test("the scanner catches what it claims to", () => {
|
||||
// Without this, "zero violations" could mean "the scanner is broken".
|
||||
const caught = new Set(scanSource("el.innerHTML = x; eval(y);", KIT_RULES).map((v) => v.rule));
|
||||
for (const rule of ["markup", "eval"]) assert.ok(caught.has(rule), `not caught: "${rule}"`);
|
||||
});
|
||||
|
||||
test("prose is not code", () => {
|
||||
// Otherwise every rule becomes unmentionable in the comment explaining it.
|
||||
const commented = `
|
||||
// never write innerHTML here
|
||||
/* and eval( is fine to discuss */
|
||||
export const ok = 1;
|
||||
`;
|
||||
assert.deepEqual(scanSource(commented, KIT_RULES), []);
|
||||
assert.ok(!code(commented).includes("innerHTML"));
|
||||
});
|
||||
@@ -1,72 +0,0 @@
|
||||
// Rule 1: a module never styles anything.
|
||||
//
|
||||
// The runtime half is stronger than this -- a module is handed { composer,
|
||||
// values, on } and nothing that could style or build -- but a scanner catches
|
||||
// the attempt in the file rather than at the moment it would have run.
|
||||
|
||||
import test from "node:test";
|
||||
import assert from "node:assert/strict";
|
||||
import { fileURLToPath } from "node:url";
|
||||
import { dirname, join } from "node:path";
|
||||
import { readFileSync } from "node:fs";
|
||||
|
||||
import { scan, scanSource, code } from "../../../../tools/kit_lint/scan.js";
|
||||
import { MODULE_RULES, KIT_RULES } from "../../../../tools/kit_lint/rules.js";
|
||||
|
||||
const here = dirname(fileURLToPath(import.meta.url));
|
||||
const repo = join(here, "..", "..", "..", "..");
|
||||
const FIXTURES = join(here, "lint_fixtures");
|
||||
|
||||
test("every module in the tree is clean", () => {
|
||||
// Empty today; the value is that it stays empty as modules arrive.
|
||||
const violations = scan(join(repo, "modules"), MODULE_RULES);
|
||||
assert.deepEqual(violations, [],
|
||||
violations.map((v) => `${v.file}:${v.line} ${v.rule} -- ${v.why}`).join("\n"));
|
||||
});
|
||||
|
||||
test("the kit never interprets content", () => {
|
||||
// The kit may style and may build elements; that is its job. What it may not
|
||||
// do is parse anything a module gave it.
|
||||
const violations = scan(join(repo, "app", "ui", "composer"), KIT_RULES);
|
||||
assert.deepEqual(violations, [],
|
||||
violations.map((v) => `${v.file}:${v.line} ${v.rule}`).join("\n"));
|
||||
});
|
||||
|
||||
test("the shell never interprets content either", () => {
|
||||
assert.deepEqual(scan(join(repo, "app", "ui", "shell"), KIT_RULES), []);
|
||||
});
|
||||
|
||||
// --- the scanner has to actually catch things ------------------------------
|
||||
|
||||
test("a clean module passes", () => {
|
||||
const source = readFileSync(join(FIXTURES, "clean", "ui.js"), "utf8");
|
||||
assert.deepEqual(scanSource(source, MODULE_RULES), []);
|
||||
});
|
||||
|
||||
test("the scanner catches every rule it claims to", () => {
|
||||
// Without this, "zero violations" could mean "the scanner is broken" -- which
|
||||
// is the failure mode of every lint nobody tests.
|
||||
const source = readFileSync(join(FIXTURES, "offenders", "ui.js"), "utf8");
|
||||
const caught = new Set(scanSource(source, MODULE_RULES).map((v) => v.rule));
|
||||
for (const rule of ["inline-style", "style-attribute", "class-manipulation",
|
||||
"raw-element", "markup", "stylesheet", "hardcoded-colour",
|
||||
"hardcoded-size", "eval"]) {
|
||||
assert.ok(caught.has(rule), `the offender fixture was not caught by "${rule}"`);
|
||||
}
|
||||
});
|
||||
|
||||
test("prose is not code", () => {
|
||||
// Otherwise every rule becomes unmentionable in the comment explaining it.
|
||||
const commented = `
|
||||
// never write innerHTML here
|
||||
/* and #ff0000 and 12px are fine to discuss */
|
||||
export const ok = 1;
|
||||
`;
|
||||
assert.deepEqual(scanSource(commented, MODULE_RULES), []);
|
||||
assert.ok(!code(commented).includes("innerHTML"));
|
||||
});
|
||||
|
||||
test("a url containing a hash is not a colour", () => {
|
||||
assert.deepEqual(scanSource('export const a = "#abc";', MODULE_RULES).map((v) => v.rule),
|
||||
["hardcoded-colour"], "a real colour is caught");
|
||||
});
|
||||
@@ -76,21 +76,6 @@ test("nothing about a hidden module reaches the screen", async () => {
|
||||
assert.equal(host.textContent, "");
|
||||
});
|
||||
|
||||
test("a module whose ui.js fails to load is hidden, not half-mounted", async () => {
|
||||
const host = document.createElement("div");
|
||||
document.body.appendChild(host);
|
||||
mounts.offer("test.region", host);
|
||||
|
||||
const { mounted, hidden } = await mountAll(
|
||||
{ modules: [{ ...spec("has_ui", { on: { type: "bool", default: true, label: "On" } }), ui: "/missing.js" }] },
|
||||
{ load: () => { throw new Error("404"); } },
|
||||
);
|
||||
|
||||
assert.deepEqual(mounted, []);
|
||||
assert.equal(hidden[0].id, "has_ui");
|
||||
assert.equal(host.childElementCount, 0, "its panel did not stay behind");
|
||||
});
|
||||
|
||||
test("a module naming a mount point nobody offers is absent", async () => {
|
||||
// Hide, don't warn, one level up: a typo'd mount point makes a module vanish
|
||||
// rather than landing somewhere arbitrary.
|
||||
@@ -100,44 +85,3 @@ test("a module naming a mount point nobody offers is absent", async () => {
|
||||
assert.deepEqual(mounted, []);
|
||||
assert.match(hidden[0].why, /no region offers/);
|
||||
});
|
||||
|
||||
test("a module is handed no way to touch the document", async () => {
|
||||
const host = document.createElement("div");
|
||||
document.body.appendChild(host);
|
||||
mounts.offer("test.region", host);
|
||||
|
||||
let context = null;
|
||||
await mountAll(
|
||||
{ modules: [{ ...spec("m", { on: { type: "bool", default: true, label: "On" } }), ui: "/ui.js" }] },
|
||||
{ load: async () => ({ setup: (ctx) => { context = ctx; } }) },
|
||||
);
|
||||
|
||||
assert.deepEqual(Object.keys(context).sort(), ["on", "shell", "values"]);
|
||||
|
||||
// Not "no value IS a node" -- that was the old assertion, and it passed while
|
||||
// `composer` sat in this object handing out handles whose .node is a real
|
||||
// element, reachable to the whole document through node.ownerDocument. The
|
||||
// property that matters is that nothing here PRODUCES one either.
|
||||
for (const [name, value] of Object.entries(context)) {
|
||||
assert.equal(value instanceof globalThis.Node, false, `${name} is a DOM node`);
|
||||
}
|
||||
assert.equal("composer" in context, false,
|
||||
"an element factory is a way to obtain a DOM node");
|
||||
|
||||
// `on` is callable, and must hand back an unsubscribe rather than anything
|
||||
// that leads to the page.
|
||||
const off = context.on(() => {});
|
||||
assert.equal(typeof off, "function");
|
||||
assert.equal(off instanceof globalThis.Node, false);
|
||||
|
||||
// Nothing one level down is a node either -- the services are the other place
|
||||
// a handle could have been tucked away.
|
||||
for (const [name, value] of Object.entries(context.shell)) {
|
||||
for (const [inner, member] of Object.entries(value)) {
|
||||
assert.equal(member instanceof globalThis.Node, false, `shell.${name}.${inner}`);
|
||||
}
|
||||
}
|
||||
|
||||
// The service surface is named one by one, so it can be reviewed.
|
||||
assert.deepEqual(Object.keys(context.shell).sort(), ["density", "theme"]);
|
||||
});
|
||||
|
||||
+7
-62
@@ -1,28 +1,12 @@
|
||||
// Turning announcements into panels.
|
||||
//
|
||||
// Everything a module can reach is decided here, and it is deliberately little:
|
||||
// { values, on, shell }. No DOM node, no document, no element factory.
|
||||
//
|
||||
// `composer` used to be in that list, and it was the hole in the guarantee: a
|
||||
// factory returns a handle whose `.node` is a real element, so a module could
|
||||
// take one, style it, and reach the whole document through `node.ownerDocument`
|
||||
// -- no lint to evade, just the intended API. No module ever used it. Panels are
|
||||
// rendered from the DECLARATION, and a module that wants to draw something is a
|
||||
// module that should be declaring it instead.
|
||||
//
|
||||
// What is true, stated exactly: the kit hands a module nothing that touches the
|
||||
// DOM. What is NOT true, and must not be claimed: that a module CANNOT reach the
|
||||
// DOM. A ui.js is an ES module in the page realm and `document` is a global.
|
||||
// tools/kit_lint raises the cost of doing that accidentally; it is a discipline
|
||||
// check and not a boundary, and only real isolation (a worker, another realm)
|
||||
// would make it one. First-party modules in this repo are reviewed; a
|
||||
// third-party module would need that isolation before it could be trusted.
|
||||
// A module draws nothing. It declares its settings and this file has the kit
|
||||
// render them: no DOM node, no document, no element factory ever reaches a module.
|
||||
// A module with a broken declaration is hidden whole, never half-mounted.
|
||||
|
||||
import { composer } from "../composer/composer.js";
|
||||
import { renderPanel, defaultsOf } from "../composer/panel.js";
|
||||
import { hostFor, offered } from "./mounts.js";
|
||||
import * as values from "./values.js";
|
||||
import { services } from "./services.js";
|
||||
|
||||
/**
|
||||
* mountAll(manifest) -> { mounted, hidden }
|
||||
@@ -32,7 +16,7 @@ import { services } from "./services.js";
|
||||
* indistinguishable from one that was never installed, which is the point --
|
||||
* the alternative is a UI full of things that cannot be used.
|
||||
*/
|
||||
export async function mountAll(manifest, { load = (path) => import(path) } = {}) {
|
||||
export async function mountAll(manifest) {
|
||||
const mounted = [];
|
||||
const hidden = [];
|
||||
|
||||
@@ -40,8 +24,7 @@ export async function mountAll(manifest, { load = (path) => import(path) } = {})
|
||||
// Nothing to render. A module may exist purely to contribute ComfyUI nodes
|
||||
// (a loader, a model's compatibility module), and those have no panel and
|
||||
// no mount. Skipping is not hiding: there is no failure to report.
|
||||
const renders = Object.keys(spec.settings || {}).length > 0 || spec.ui;
|
||||
if (!renders) continue;
|
||||
if (!Object.keys(spec.settings || {}).length) continue;
|
||||
|
||||
const host = hostFor(spec.mount);
|
||||
if (!host) {
|
||||
@@ -50,58 +33,20 @@ export async function mountAll(manifest, { load = (path) => import(path) } = {})
|
||||
}
|
||||
|
||||
try {
|
||||
// Values first: a module's ui.js may read them during setup, and a
|
||||
// headless run has them without any of this having happened.
|
||||
values.seed(spec.id, defaultsOf(spec));
|
||||
|
||||
let ui = null;
|
||||
if (spec.ui) ui = await load(spec.ui);
|
||||
let teardown = null;
|
||||
|
||||
if (ui && typeof ui.setup === "function") {
|
||||
// Kept, not discarded: setup() returns the unsubscribe from on(), and
|
||||
// dropping it means every re-mount leaves another live listener behind.
|
||||
teardown = ui.setup({
|
||||
values: {
|
||||
get: () => values.valuesOf(spec.id),
|
||||
set: (key, value) => values.set(spec.id, key, value),
|
||||
},
|
||||
// Scoped to this module: a module hearing about everyone else's
|
||||
// edits would have to filter, and filtering needs an id it should
|
||||
// not need to know.
|
||||
on: (fn) => values.onChange((id, key, value) => {
|
||||
if (id === spec.id) fn(key, value);
|
||||
}),
|
||||
// The app services a module may use, named explicitly. A short,
|
||||
// reviewable list beats reaching for a global, which is how a module
|
||||
// ends up depending on something nobody meant to expose.
|
||||
shell: services,
|
||||
});
|
||||
}
|
||||
|
||||
// Built AFTER setup, so a control shows what setup settled on. The panel
|
||||
// is drawn from a snapshot of the values, so building it first meant a
|
||||
// module that adopted a live value in setup -- the theme, which the page
|
||||
// applies before any module exists -- had a control showing something the
|
||||
// app was not set to.
|
||||
const panel = renderPanel(spec, {
|
||||
values: values.valuesOf(spec.id),
|
||||
onChange: (key, value) => values.set(spec.id, key, value),
|
||||
});
|
||||
|
||||
// Only now, with a complete panel AND a setup() that returned, does
|
||||
// anything reach the DOM. Appending before setup ran left a module whose
|
||||
// setup threw reported as hidden with its panel still on screen -- which
|
||||
// is precisely the guarantee this file states it keeps.
|
||||
// Only a complete panel reaches the DOM.
|
||||
host.appendChild(panel.node);
|
||||
|
||||
mounted.push({
|
||||
id: spec.id,
|
||||
panel,
|
||||
destroy() {
|
||||
if (typeof teardown === "function") teardown();
|
||||
panel.destroy();
|
||||
},
|
||||
destroy() { panel.destroy(); },
|
||||
});
|
||||
} catch (err) {
|
||||
hidden.push({ id: spec.id, why: `${err.name}: ${err.message}` });
|
||||
|
||||
@@ -1,26 +0,0 @@
|
||||
// What the app offers modules beyond the kit.
|
||||
//
|
||||
// Deliberately tiny and named one by one. The alternative -- letting a module
|
||||
// reach for a global -- means the surface is "whatever happens to be on window",
|
||||
// which nobody can review and nothing can keep stable.
|
||||
|
||||
import { applyDensity, normaliseCols, rememberCols, recallCols } from "../composer/internals/density.js";
|
||||
|
||||
export const services = {
|
||||
theme: {
|
||||
set(choice) { window.ComposerTheme.apply(choice); },
|
||||
get: () => window.ComposerTheme.get(),
|
||||
resolved: () => window.ComposerTheme.resolved(),
|
||||
},
|
||||
density: {
|
||||
/** What the grids are set to, which the browser remembers between visits. */
|
||||
get: () => recallCols("app"),
|
||||
/** Column count for every adaptive grid on the page. */
|
||||
set(cols) {
|
||||
const n = normaliseCols(cols);
|
||||
for (const grid of document.querySelectorAll(".cx-gallery")) applyDensity(grid, n);
|
||||
rememberCols("app", n);
|
||||
return n;
|
||||
},
|
||||
},
|
||||
};
|
||||
+2
-4
@@ -9,11 +9,9 @@ MODULES_DIR = ROOT / "modules"
|
||||
|
||||
UI_PREFIX = "/funpack"
|
||||
|
||||
# Extension allowlists per served root. These are rule 1 ("a module never styles
|
||||
# anything") enforced at the transport layer: a .css inside modules/ is a 404, so
|
||||
# a module cannot ship a stylesheet even if someone writes one.
|
||||
# Extension allowlist for the served app. modules/ is never served: a module is
|
||||
# Python and a settings declaration, and draws nothing.
|
||||
APP_EXTS = frozenset({".js", ".css", ".html", ".woff2", ".svg"})
|
||||
MODULE_EXTS = frozenset({".js"})
|
||||
|
||||
# Where projects are kept. Inside the pack rather than ComfyUI's output tree: a
|
||||
# project is FunPack's own state, and clearing outputs must not take a user's
|
||||
|
||||
@@ -81,7 +81,6 @@ class ModuleSpec:
|
||||
# Hook points a sampler must offer before this module can run. Empty means
|
||||
# "any sampler will do", the same narrowing rule traits follow.
|
||||
hooks: List[str] = field(default_factory=list)
|
||||
ui: Optional[str] = None # served path to its ui.js
|
||||
status: str = "experimental" # or "proven"
|
||||
source: str = "" # dotted import path
|
||||
|
||||
@@ -109,7 +108,6 @@ class ModuleSpec:
|
||||
"before": list(self.before),
|
||||
"stage": self.stage,
|
||||
"category": self.category,
|
||||
"ui": self.ui,
|
||||
"status": self.status,
|
||||
"nodes": [_node_id(n) for n in self.nodes],
|
||||
}
|
||||
|
||||
@@ -132,12 +132,6 @@ def scan(root: Optional[Path] = None, package: Optional[str] = None) -> Registry
|
||||
# live in packages too.
|
||||
continue
|
||||
|
||||
# A module's ui.js is optional, and the default is none: most modules
|
||||
# are a settings schema and no JavaScript at all.
|
||||
ui_file = feature / "ui.js"
|
||||
if ui_file.is_file():
|
||||
raw["ui"] = f"{config.UI_PREFIX}/modules/{'/'.join(rel.parts)}/ui.js"
|
||||
|
||||
try:
|
||||
registry.add(validate(raw, source=dotted))
|
||||
except SchemaError as exc:
|
||||
|
||||
@@ -1197,10 +1197,6 @@ def register(routes, prefix=None):
|
||||
async def _app_asset(req):
|
||||
return _serve_under(req, config.APP_DIR, config.APP_EXTS)
|
||||
|
||||
@routes.get(P + "/modules/{tail:.*}")
|
||||
async def _module_asset(req):
|
||||
return _serve_under(req, config.MODULES_DIR, config.MODULE_EXTS)
|
||||
|
||||
@routes.get(P + "/")
|
||||
async def _index(_req):
|
||||
# The frontend: app/ui.
|
||||
|
||||
+1
-2
@@ -337,7 +337,7 @@ def validate(announcement: Dict[str, Any], source: str = "") -> ModuleSpec:
|
||||
# region would make it invent one -- which then shows up as "hidden", i.e.
|
||||
# a module reported as broken for having nothing to show.
|
||||
mount = announcement.get("mount")
|
||||
renders = bool(announcement.get("settings")) or bool(announcement.get("ui"))
|
||||
renders = bool(announcement.get("settings"))
|
||||
if renders and (not isinstance(mount, str) or not mount.strip()):
|
||||
raise SchemaError(
|
||||
f"module {announcement.get('id') or source or '?'} has settings to render "
|
||||
@@ -401,7 +401,6 @@ def validate(announcement: Dict[str, Any], source: str = "") -> ModuleSpec:
|
||||
before=_ids("before"),
|
||||
stage=stage,
|
||||
category=category,
|
||||
ui=announcement.get("ui"),
|
||||
status=status,
|
||||
source=source,
|
||||
)
|
||||
|
||||
@@ -1 +0,0 @@
|
||||
export default {};
|
||||
@@ -51,15 +51,6 @@ def test_every_failure_says_why(scanned):
|
||||
assert where and why, f"failure recorded with no reason: {where!r} {why!r}"
|
||||
|
||||
|
||||
def test_a_module_with_a_ui_file_advertises_its_path(scanned):
|
||||
assert scanned.specs["audio_clock"].ui.endswith("/modules/timing/audio_clock/ui.js")
|
||||
|
||||
|
||||
def test_a_module_without_a_ui_file_advertises_none(scanned):
|
||||
# The default is no JavaScript at all: a settings schema is a whole module.
|
||||
assert scanned.specs["momentum"].ui is None
|
||||
|
||||
|
||||
def test_defaults_are_available_without_any_ui(scanned):
|
||||
assert scanned.specs["audio_clock"].defaults() == {
|
||||
"enabled": True, "strength": 0.65, "mode": "beat",
|
||||
|
||||
@@ -41,7 +41,6 @@ def app(tmp_path, monkeypatch):
|
||||
|
||||
mod_dir = tmp_path / "modules" / "timing" / "audio_clock"
|
||||
mod_dir.mkdir(parents=True)
|
||||
(mod_dir / "ui.js").write_text("export default 1;")
|
||||
(mod_dir / "sneaky.css").write_text(".x{}")
|
||||
|
||||
monkeypatch.setattr(config, "APP_DIR", app_dir)
|
||||
@@ -113,13 +112,9 @@ def test_a_modules_url_cannot_reach_app_content(app):
|
||||
|
||||
# --- the allowlist on ordinary URLs ----------------------------------------
|
||||
|
||||
def test_module_css_is_refused(app):
|
||||
def test_modules_are_not_served(app):
|
||||
assert get(app, P + "/modules/timing/audio_clock/sneaky.css")[0] == 404
|
||||
|
||||
|
||||
def test_module_js_is_served(app):
|
||||
status, body = get(app, P + "/modules/timing/audio_clock/ui.js")
|
||||
assert status == 200 and "export default" in body
|
||||
assert get(app, P + "/modules/timing/audio_clock/ui.js")[0] == 404
|
||||
|
||||
|
||||
def test_app_asset_is_served(app):
|
||||
|
||||
@@ -1,8 +1,4 @@
|
||||
"""Rule 1, transport half: a module cannot ship anything but JavaScript.
|
||||
|
||||
These exist before there is any UI to enforce them against — the wall goes up
|
||||
first, so nothing can be built that leans on a hole in it.
|
||||
"""
|
||||
"""Rule 1, transport half: only the app is served; modules/ never is."""
|
||||
|
||||
import pytest
|
||||
|
||||
@@ -23,7 +19,6 @@ def tree(tmp_path):
|
||||
|
||||
mods = tmp_path / "modules"
|
||||
(mods / "timing" / "audio_clock").mkdir(parents=True)
|
||||
(mods / "timing" / "audio_clock" / "ui.js").write_text("export default {};")
|
||||
(mods / "timing" / "audio_clock" / "sneaky.css").write_text(".x{color:red}")
|
||||
(mods / "timing" / "audio_clock" / "panel.html").write_text("<b>no</b>")
|
||||
(mods / "timing" / "audio_clock" / "module.py").write_text("x = 1")
|
||||
@@ -36,35 +31,8 @@ def app_get(tree, rel):
|
||||
return serve(tree / "app", rel, config.APP_EXTS)
|
||||
|
||||
|
||||
def module_get(tree, rel):
|
||||
return serve(tree / "modules", rel, config.MODULE_EXTS)
|
||||
|
||||
|
||||
# --- the allowlist ---------------------------------------------------------
|
||||
|
||||
def test_module_js_is_served(tree):
|
||||
r = module_get(tree, "timing/audio_clock/ui.js")
|
||||
assert r.status == 200
|
||||
assert b"export default" in r.body
|
||||
|
||||
|
||||
def test_module_css_is_not_served(tree):
|
||||
assert module_get(tree, "timing/audio_clock/sneaky.css").status == 404
|
||||
|
||||
|
||||
def test_module_html_is_not_served(tree):
|
||||
assert module_get(tree, "timing/audio_clock/panel.html").status == 404
|
||||
|
||||
|
||||
def test_module_python_is_not_served(tree):
|
||||
assert module_get(tree, "timing/audio_clock/module.py").status == 404
|
||||
|
||||
|
||||
def test_module_directory_is_not_an_index(tree):
|
||||
# modules/ has no .html in its allowlist, so a directory is nothing to serve.
|
||||
assert module_get(tree, "timing/audio_clock").status == 404
|
||||
|
||||
|
||||
def test_app_serves_its_own_css_and_fonts(tree):
|
||||
assert app_get(tree, "composer/composer.css").status == 200
|
||||
assert app_get(tree, "assets/Inter.woff2").status == 200
|
||||
@@ -87,10 +55,6 @@ def test_traversal_is_forbidden(tree, rel):
|
||||
assert app_get(tree, rel).status == 403
|
||||
|
||||
|
||||
def test_traversal_out_of_modules_is_forbidden(tree):
|
||||
assert module_get(tree, "../app/boot.js").status == 403
|
||||
|
||||
|
||||
def test_null_byte_is_forbidden(tree):
|
||||
assert app_get(tree, "boot.js\x00.png").status == 403
|
||||
|
||||
@@ -106,7 +70,6 @@ def test_symlink_escape_is_forbidden(tree):
|
||||
def test_js_is_explicitly_text_javascript(tree):
|
||||
# With ES modules a text/plain guess makes the browser refuse the module.
|
||||
assert app_get(tree, "boot.js").content_type == "text/javascript"
|
||||
assert module_get(tree, "timing/audio_clock/ui.js").content_type == "text/javascript"
|
||||
|
||||
|
||||
def test_known_types_are_explicit():
|
||||
|
||||
@@ -87,11 +87,10 @@ There are three trust levels in the app code. Know which one you are writing.
|
||||
| You are writing | May style or build DOM? | Checked by |
|
||||
|---|---|---|
|
||||
| A **backend module's settings** (Python only) | No JavaScript at all. The kit draws the rows. | `core/schema.py`, `core/tests/test_shipped_hints.py` |
|
||||
| An optional **module `ui.js`** (`modules/<group>/<name>/ui.js`). Not loaded by the app; see [Writing a module](writing-a-module.md#optional-uijs). | No. `setup({ values, on, shell })` and nothing else. | `tools/kit_lint` scans `modules/` |
|
||||
| A **UI feature** in `app/ui/features/` | It builds with kit elements. A few features touch `document` or add `fp-` classes in `frame.css`; new work should not. | review; the kit rules are enforced on `composer/` and `shell/` |
|
||||
| The **kit** itself | Yes. It is the only place that styles. | `tools/kit_lint` `KIT_RULES`, tests in `composer/tests/` |
|
||||
|
||||
Be exact about what the lint is. `tools/kit_lint` is a discipline check, not a security boundary: a module `ui.js` runs in the page, so `document` is reachable. First-party modules are reviewed. A third-party module would need real isolation (a worker or another realm) before it could be trusted.
|
||||
Be exact about what the lint is. `tools/kit_lint` is a discipline check, not a security boundary. Modules run only in Python and are never served to the browser; first-party modules are reviewed.
|
||||
|
||||
## Where things live
|
||||
|
||||
|
||||
@@ -10,7 +10,7 @@ Read by `core/registry.py` from the module's `__init__.py` (the `FIELDS` table).
|
||||
|---|---|---|---|
|
||||
| `ID` | `str` | none | Required, non-empty. Unique across modules (a second claimant is left out and said). Keep it free of `.`: a fault is traced back to a module from its hook key `funpack.<id>`. |
|
||||
| `TITLE` | `str` | none | Required, non-empty. |
|
||||
| `MOUNT` | `str` | `""` | Required and non-empty when the module has `SETTINGS` (or a `ui.js`, which the app does not load). |
|
||||
| `MOUNT` | `str` | `""` | Required and non-empty when the module has `SETTINGS`. |
|
||||
| `SETTINGS` | `dict` | `{}` | `{key: setting}`. Must be a dict. |
|
||||
| `STAGE` | `str` | `"sampling"` | One of `load`, `conditioning`, `latent`, `guidance`, `sampling`, `post`. |
|
||||
| `CATEGORY` | `str` | `""` | `""` or one of `continuity`, `guidance`, `conditioning`, `sampling`, `post`, `system`. Empty shows under "Other". |
|
||||
@@ -25,8 +25,6 @@ Read by `core/registry.py` from the module's `__init__.py` (the `FIELDS` table).
|
||||
| `TRAITS` | callable | none | `traits(model) -> iterable[str]`. |
|
||||
| `PROVIDES` | `dict[str, callable]` | `{}` | Capability name to function. Names must be non-empty strings. |
|
||||
|
||||
Not read from constants: `ui`. Core sets it to `/funpack/modules/<group>/<name>/ui.js` when that file exists.
|
||||
|
||||
`CONTRACT_VERSION` in `core/contract.py` is `1`. It is sent to the browser as `contract` in `/funpack/api/modules`. Core does not read a version from modules, so there is nothing for a module to declare.
|
||||
|
||||
## Setting keys
|
||||
@@ -183,5 +181,4 @@ Most used:
|
||||
| `/funpack/` | the app (`index.html`) |
|
||||
| `/funpack/api/modules` | the manifest (`?traits=a,b` narrows to a model) |
|
||||
| `/funpack/api/m/<id>/...` | a module's own routes |
|
||||
| `/funpack/modules/<group>/<name>/ui.js` | a module's `ui.js`. Only `.js` is served from `modules/`. The app does not load it. |
|
||||
| `/funpack/app/...` | the app. Allowed types: `.js .css .html .woff2 .svg`. |
|
||||
|
||||
+1
-1
@@ -66,7 +66,7 @@ See [Adding an element](ui-kit-reference.md#adding-an-element). The registry-wid
|
||||
|
||||
### `tools/kit_lint`
|
||||
|
||||
`scan.js` applies `MODULE_RULES` to module `ui.js` files and `KIT_RULES` to the kit. `rule_no_module_styling.test.js` runs it and also runs the files in `composer/tests/lint_fixtures`, so a rule that stops catching its own bad example fails. It is a discipline check, not a security boundary.
|
||||
`scan.js` applies `KIT_RULES` to the kit and the shell: no markup parsing, no `eval`. `rule_kit_never_interprets.test.js` runs it and checks it still catches a bad example. It is a discipline check, not a security boundary.
|
||||
|
||||
## Looking at it
|
||||
|
||||
|
||||
@@ -17,7 +17,7 @@ How FunPack looks and behaves, and why. Read this before you add a screen, a set
|
||||
Only the kit styles anything: `app/ui/composer/` (tokens, internals, elements). A feature builds with kit elements and names no colour, size or class.
|
||||
|
||||
- A rule test fails the build if kit code parses content as markup or compiles code (`tools/kit_lint` `KIT_RULES`).
|
||||
- The same scanner is run over module `ui.js` files (the running app does not load them) with stricter rules (`MODULE_RULES`): no inline style, no `classList`, no `document`, no raw hex colour, no `px` size.
|
||||
- Modules ship no JavaScript, so there is nothing to scan in `modules/`.
|
||||
- A few older features touch `document` or add `fp-` classes in `shell/frame.css`. Do not copy them. New work uses the kit, and if the kit lacks the element, add the element (see [Adding an element](ui-kit-reference.md#adding-an-element)).
|
||||
|
||||
## Tokens
|
||||
|
||||
@@ -180,7 +180,7 @@ Used by elements. A feature does not import them. They exist in `composer/intern
|
||||
|---|---|
|
||||
| `rule_content_is_data.test.js` | Every string prop of every demo is shown as text. |
|
||||
| `rule_unknown_element_hides.test.js` | Asking for a missing element throws and hides the feature. |
|
||||
| `rule_no_module_styling.test.js` | `tools/kit_lint` finds nothing in `modules/`, and its own fixtures (`tests/lint_fixtures`) still trip every rule. |
|
||||
| `rule_kit_never_interprets.test.js` | `tools/kit_lint` finds no markup parsing or `eval` in the kit and the shell, and still catches a bad example. |
|
||||
| `register.test.js` | Duplicate definitions are refused. |
|
||||
| `overlays.test.js`, `dismiss.test.js`, `zlayer.test.js`, `portal.test.js` | Layering, Escape and portal behaviour. |
|
||||
| `panel_schema.test.js` | `panel.js` draws every setting type. |
|
||||
|
||||
@@ -132,7 +132,7 @@ Constants at the top of `__init__.py`, uppercase because they are declarations,
|
||||
|
||||
A declaration that is wrong does not load, and the Log says exactly what is wrong. There is no best-effort mode.
|
||||
|
||||
**About `MOUNT`.** Core requires it when a module has settings, so give it. In the app as shipped, though, module settings are drawn by Settings > Engine and the Sampler window, which group by `CATEGORY`, not `MOUNT`. `MOUNT` only matters to the optional `ui.js` path described at the end.
|
||||
**About `MOUNT`.** Core requires it when a module has settings, so give it. In the app as shipped, though, module settings are drawn by Settings > Engine and the Sampler window, which group by `CATEGORY`, not `MOUNT`. `MOUNT` only matters to the catalogue and the tests, which mount panels by it.
|
||||
|
||||
## Settings
|
||||
|
||||
@@ -182,7 +182,7 @@ What the tests add (`core/tests/test_shipped_hints.py`), so treat them as rules:
|
||||
Two code paths can draw a setting, and they differ:
|
||||
|
||||
- **Settings > Engine and the Sampler window** (`app/ui/features/settings/field.js`). This is what people use. It draws by `type`: `bool` as a toggle; `int` and `float` as a slider when `ui` is absent or `"slider"` and both `min` and `max` are set, otherwise as a number box; `enum` as a select; `color` as a swatch; `multiline` as a text area; `text` and `path` as a text box. A `ui` of `segmented`, `wheel`, `radioGroup`, `filterList` and so on is accepted by the schema but this page still draws the default for the type. `placeholder` is not used here.
|
||||
- **`composer/panel.js`** honours every `ui` hint in `UI_HINTS`. It is used by the catalogue and the tests, and by the optional `ui.js` route.
|
||||
- **`composer/panel.js`** honours every `ui` hint in `UI_HINTS`. It is used by the catalogue and the tests.
|
||||
|
||||
So: choose `ui` only between `slider` and `number` for numbers, and design for the select and toggle everywhere else.
|
||||
|
||||
@@ -322,22 +322,9 @@ Research or debug data does not go in the console: put a toggle, a Download butt
|
||||
- Fewest words in docstrings, hints and tooltips. No backstory and no future plans in the repo.
|
||||
- Nothing NSFW in code, tests, comments, commits or docs. Build test data from constructed fixtures.
|
||||
|
||||
## Optional `ui.js`
|
||||
## No JavaScript
|
||||
|
||||
A module may have `ui.js` next to its `__init__.py`. Core serves it at `/funpack/modules/<group>/<name>/ui.js` (only `.js` files are served from `modules/`, so a module cannot ship CSS) and puts the path in its manifest. The contract in `app/ui/shell/panels.js`:
|
||||
|
||||
```js
|
||||
export function setup({ values, on, shell }) {
|
||||
// values.get() / values.set(key, value) this module's settings only
|
||||
// on((key, value) => ...) hears this module's changes only
|
||||
// shell.theme, shell.density the small list of app services
|
||||
return () => { /* teardown */ };
|
||||
}
|
||||
```
|
||||
|
||||
A module's `ui.js` gets **no element factory and no DOM**. It cannot style, build or inject anything: `tools/kit_lint` fails the build on inline styles, class changes, `document`, `window`, `globalThis`, `innerHTML`, hardcoded colours or pixel sizes, stylesheets and `eval`. The settings panel is drawn for it from the declaration.
|
||||
|
||||
**The running app ignores `ui.js` on purpose.** `boot.js` never calls `mountAll`; only the catalogue and the tests do, and no shipped module has a `ui.js`. A module does not get to draw anything. It declares what it wants in `SETTINGS` (type, `ui` hint, range, hint text) and the UI kit builds the control from that declaration. Anything else a person needs to see in the app is a [UI feature](writing-a-ui-feature.md), which is app code and not a module. Do not add a `ui.js`.
|
||||
A module ships no JavaScript and draws nothing. It declares what it wants in `SETTINGS` (type, `ui` hint, range, hint text) and the UI kit builds the control from that declaration. `modules/` is never served to the browser. Anything else a person needs to see in the app is a [UI feature](writing-a-ui-feature.md), which is app code and not a module.
|
||||
|
||||
## Before you open a pull request
|
||||
|
||||
|
||||
+6
-39
@@ -1,44 +1,11 @@
|
||||
// What a module may not do, and what the kit may not do.
|
||||
// What the kit and the shell may not do.
|
||||
//
|
||||
// A DISCIPLINE CHECK, NOT A BOUNDARY. Say it plainly, because the difference
|
||||
// matters: a module's ui.js is an ES module in the page realm, so `document` is
|
||||
// a global it can reach and no regex can stop it. `globalThis["docu"+"ment"]`
|
||||
// defeats every rule below and always will; only real isolation -- a worker,
|
||||
// another realm -- would be a boundary.
|
||||
//
|
||||
// What this does buy: the accidental and the casual are caught in the file
|
||||
// rather than at the moment they would have run, which is the difference
|
||||
// between a failing test and a support ticket. The bracket-access spellings are
|
||||
// here because they were the first thing tried and they went straight through.
|
||||
//
|
||||
// The runtime half is the real work: the kit hands a module nothing that
|
||||
// touches the DOM, so following the contract requires no restraint.
|
||||
// A DISCIPLINE CHECK, NOT A BOUNDARY: it catches the accidental in the file
|
||||
// rather than at the moment it would have run. Modules need no rules here
|
||||
// because they ship no JavaScript.
|
||||
|
||||
export const MODULE_RULES = [
|
||||
{ name: "inline-style", why: "presentation belongs to the kit",
|
||||
test: /\.style\s*[.[]|\[\s*['"`]style['"`]\s*\]|setAttribute\s*\(\s*['"`]style/ },
|
||||
{ name: "style-attribute", why: "presentation belongs to the kit",
|
||||
test: /\bstyle\s*=\s*['"`]/ },
|
||||
{ name: "class-manipulation", why: "kit classes are the kit's; a module names elements, not classes",
|
||||
test: /\bclassList\b|setAttribute\s*\(\s*['"`]class/ },
|
||||
{ name: "raw-element", why: "every element comes from the kit, not from a module",
|
||||
test: /document\s*[.[]|createElementNS|\[\s*['"`]createElement['"`]\s*\]/ },
|
||||
{ name: "reaching-for-globals", why: "a module is handed what it may use; the rest is not for it",
|
||||
test: /\bglobalThis\b|\bwindow\s*[.[]|\bself\s*\[/ },
|
||||
{ name: "markup", why: "content is data; the kit renders it as text",
|
||||
test: /\binnerHTML\b|\bouterHTML\b|insertAdjacentHTML|\[\s*['"`](inner|outer)HTML['"`]\s*\]/ },
|
||||
{ name: "stylesheet", why: "a module cannot ship styling, and cannot inject it either",
|
||||
test: /adoptedStyleSheets|document\s*\.\s*head|<\s*(link|style)\b/ },
|
||||
{ name: "hardcoded-colour", why: "colour comes from the theme tokens",
|
||||
test: /#[0-9a-fA-F]{3,8}\b/ },
|
||||
{ name: "hardcoded-size", why: "sizes come from the spacing and control scales",
|
||||
test: /\b\d+(\.\d+)?px\b/ },
|
||||
{ name: "eval", why: "nothing in a module is compiled at runtime",
|
||||
test: /\beval\s*[([]|new\s+Function\s*\(|\[\s*['"`](eval|Function)['"`]\s*\]/ },
|
||||
];
|
||||
|
||||
// The kit itself may style and may build elements -- that is its job -- but it
|
||||
// must never interpret content.
|
||||
// The kit may style and build elements -- that is its job -- but it must never
|
||||
// interpret content.
|
||||
export const KIT_RULES = [
|
||||
{ name: "markup", why: "the kit renders content as text and never parses it",
|
||||
test: /\binnerHTML\b|\bouterHTML\b|insertAdjacentHTML|document\s*\.\s*write/ },
|
||||
|
||||
Reference in New Issue
Block a user