diff --git a/.gitignore b/.gitignore index 146f98fb..e62553ec 100644 --- a/.gitignore +++ b/.gitignore @@ -45,6 +45,7 @@ docs/.vitepress/dist/ host/client_app/modules.manifest.json host/client_app/modules.generated.ts host/client_app/modules.generated.css +host/client_app/modules.assets.json # Worktrees .worktrees/ diff --git a/CLAUDE.md b/CLAUDE.md index c10af6fb..54db095f 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -63,8 +63,15 @@ modules/// ├── endpoints/api.py # REST (JSON) ├── endpoints/views.py # Inertia view endpoints ├── pages/*.tsx # auto-discovered by Vite via modules.generated.ts +├── theme.css # optional — @theme tokens; imported UNLAYERED +├── styles.css # optional — component rules; imported into layer(components) └── locales/.json ``` +Both CSS files are optional and auto-detected; `gen-pages` emits an +`@import "#module//..."` for each, so nothing is added to the host's +`styles.css` by hand. The split is load-bearing: a `@theme` block inside a +cascade layer is inert, while unlayered CSS beats every Tailwind utility — +hence `SM022`/`SM023`. See `docs/module-authoring.md` § Styling. **Lifecycle hooks** (in `framework/core/simple_module_core/module.py`) — all no-op by default; subclasses override as needed: `register_settings` → `register_menu_items` / `register_permissions` / `register_feature_flags` / `register_event_handlers` / `register_health_checks` / `register_public_routes` → `register_exception_handlers` → `register_middleware` → `register_routes(api_router, view_router)` → async `on_startup` / `on_shutdown` (reverse order). `register_public_routes(registry)` lets a module exempt anonymous/read-only routes (STAC/OGC, webhooks) from `AuthMiddleware`; rules are method-aware (`registry.add_regex(r"…/tilejson$", methods={"GET"})`), so a GET read route can be public while sibling POST/PATCH mutations under the same prefix stay gated. See [docs/framework/public-routes.md](docs/framework/public-routes.md). @@ -94,7 +101,7 @@ Standard mixins in `simple_module_db.mixins`: `AuditMixin`, `SoftDeleteMixin` (b ## Diagnostic codes -Meaningful codes when reading `make doctor` output: `SM001` missing meta (error), `SM003` orphan page / `SM004` phantom render (warn), `SM007` module overrides no hooks (info), `SM008` duplicate name (error), `SM009` framework→plugin import (error), `SM010` DB revision behind head (error), `SM011` module table not in migration history (warn), `SM012` `register_settings` overridden but nothing on `app.state.` (warn, fires at dev boot only), `SM013`–`SM016` locale issues, `SM017` module ships `.tsx` pages but is missing `package.json`/`tsconfig.json` (warn), `SM018` Inertia `router.{post,patch,put,delete}()` in a page targets a JSON `/api/*` endpoint (warn — Inertia rejects non-Inertia responses), `SM019` module registers view routes (non-empty `view_prefix` + overrides `register_routes`) but overrides neither `register_menu_items` nor `register_permissions` (warn — pages exist with no sidebar entry and no role-editor visibility; admins can't reach them through the UI). Modules whose views are sub-pages of another module typically register permissions to stay discoverable in the role editor without needing their own sidebar entry. `SM020` multiple auth provider modules installed (error), `SM021` no auth provider module installed (warn). In production, errors fail boot. +Meaningful codes when reading `make doctor` output: `SM001` missing meta (error), `SM003` orphan page / `SM004` phantom render (warn), `SM007` module overrides no hooks (info), `SM008` duplicate name (error), `SM009` framework→plugin import (error), `SM010` DB revision behind head (error), `SM011` module table not in migration history (warn), `SM012` `register_settings` overridden but nothing on `app.state.` (warn, fires at dev boot only), `SM013`–`SM016` locale issues, `SM017` module ships `.tsx` pages but is missing `package.json`/`tsconfig.json` (warn), `SM018` Inertia `router.{post,patch,put,delete}()` in a page targets a JSON `/api/*` endpoint (warn — Inertia rejects non-Inertia responses), `SM019` module registers view routes (non-empty `view_prefix` + overrides `register_routes`) but overrides neither `register_menu_items` nor `register_permissions` (warn — pages exist with no sidebar entry and no role-editor visibility; admins can't reach them through the UI). Modules whose views are sub-pages of another module typically register permissions to stay discoverable in the role editor without needing their own sidebar entry. `SM020` multiple auth provider modules installed (error), `SM021` no auth provider module installed (warn), `SM022` `@theme`/`@custom-variant`/`@utility` in a module's `styles.css`, where `layer(components)` makes them inert (warn), `SM023` an unlayered rule in a module's `theme.css`, which outranks every Tailwind utility (warn). In production, errors fail boot. ## Tests & fixtures diff --git a/docs/module-authoring.md b/docs/module-authoring.md index 8b57cb48..d856d33c 100644 --- a/docs/module-authoring.md +++ b/docs/module-authoring.md @@ -243,6 +243,10 @@ Modules may ship TSX pages in `my_module/pages/*.tsx`. On host boot (and on - `client_app/modules.manifest.json` — machine-readable paths - `client_app/modules.generated.ts` — per-module `import.meta.glob` calls with absolute paths resolved via `importlib.resources` +- `client_app/modules.generated.css` — Tailwind `@source` entries, plus an + `@import` per module-shipped stylesheet (see [Styling](#styling)) +- `client_app/modules.assets.json` — the per-module asset record that + `vite.config.ts` builds its `#module/` aliases from Vite's `server.fs.allow` is extended to cover each installed module's package root, so pages shipped inside a wheel work for the dev server and @@ -263,6 +267,82 @@ class MyModule(ModuleBase): The host mounts each entry as `StaticFiles` during boot. +## Styling + +A module may ship two optional stylesheets beside its `pages/` directory. +Both are auto-detected exactly the way `pages/` is — there is no hook to +override and nothing to register: + +``` +my_module/ +├── module.py +├── theme.css # optional — @theme tokens, @custom-variant, @font-face +├── styles.css # optional — component rules, keyframes, vendor CSS +└── pages/ +``` + +`smpy host gen-pages` emits an `@import` for each into +`client_app/modules.generated.css`: + +```css +@import "#module/my_module/theme.css"; +@import "#module/my_module/styles.css" layer(components); +``` + +**Nothing needs to be added to the host's `styles.css` by hand.** The +`#module/` specifier is a Vite alias built from `modules.assets.json`, +so it resolves identically whether the module is a workspace member or +installed from a wheel — and no generated file ends up containing a +`../../../.venv/lib/python3.12/site-packages/...` path that would break the +next time the interpreter version changes. + +Imports are emitted in module discovery order, which is topological by +`ModuleMeta.depends_on`. A module that depends on another can therefore +override its dependency's styles. + +### Which file does what + +The split is not cosmetic — it is what makes the cascade rules structural +rather than merely documented. + +| | `theme.css` | `styles.css` | +|---|---|---| +| Imported | unlayered | `layer(components)` | +| For | `@theme`, `@custom-variant`, `@utility`, `@font-face`, `:root` tokens | component rules, keyframes, vendor CSS | +| Beats a Tailwind utility? | yes | no | + +Tailwind v4 expands `@import "tailwindcss"` into +`@layer theme, base, components, utilities`, and **unlayered CSS beats every +layered rule**. So a module shipping a bare `.card { padding: 0 }` unlayered +would silently override `p-4` on that element. But `@theme` blocks *must* be +unlayered to register design tokens at all — a `@theme` inside a layer is +inert. One file cannot satisfy both constraints, so each file gets one job. + +`make doctor` catches the two ways to get this wrong: **SM022** flags +`@theme`/`@custom-variant`/`@utility` sitting in `styles.css` (where they do +nothing), and **SM023** flags an unlayered rule in `theme.css` (where it +outranks every utility). Both are warnings — the CSS is legal either way, it +just cascades in a way you probably did not intend. + +### Cascade order + +``` +design-system @theme < module theme.css < app @theme overrides +``` + +A module normally *adds* tokens (`--color-map-water`); when it deliberately +redefines a design-system token it wins, and the consuming app still has the +final word from its own `@theme` block below the generated import. + +### Packaging + +**No packaging change is required.** The module wheel template already +declares `[tool.hatch.build.targets.wheel] packages = ["my_module"]`, and +Hatch includes every file under the package directory — `.css` along with +`.tsx`. The `force-include` block is only needed for artifacts that live +*outside* the package dir (`package.json`) or that are gitignored +(`static/dist`). + ## Templates Jinja2 template directories contributed via `ModuleBase.template_dirs()` diff --git a/docs/superpowers/plans/2026-08-05-module-css-packaging.md b/docs/superpowers/plans/2026-08-05-module-css-packaging.md new file mode 100644 index 00000000..9b759ca6 --- /dev/null +++ b/docs/superpowers/plans/2026-08-05-module-css-packaging.md @@ -0,0 +1,828 @@ +# Module CSS Packaging Implementation Plan + +> **For agentic workers:** REQUIRED SUB-SKILL: Use superpowers:subagent-driven-development (recommended) or superpowers:executing-plans to implement this plan task-by-task. Steps use checkbox (`- [ ]`) syntax for tracking. + +**Goal:** Let a pip-installed module ship real CSS (design tokens and component +styles) that lands in the host's Tailwind build automatically, with a defined +cascade position and no hand-edited paths. + +**Architecture:** A module may ship `/theme.css` and `/styles.css`, +auto-detected like `pages/`. `smpy gen-pages` emits `@import` lines into +`host/client_app/modules.generated.css` — `theme.css` unlayered (so `@theme` +tokens register) and `styles.css` as `layer(components)` (so module rules can +never beat a utility). Paths are per-module Vite aliases (`#module/`) fed +by a new additive `modules.assets.json`, so no generated file contains a +`../../../.venv/...` path. + +**Tech Stack:** Python 3.12, FastAPI, Hatch wheels, Vite 8, `@tailwindcss/vite` +4.2.4, Tailwind CSS v4, pytest. + +## Global Constraints + +- **300-line cap** on `.py`/`.ts`/`.tsx`, enforced by `scripts/check_file_size.py`. + `manifest.py` is already at 275 lines, so new asset logic goes in a new + `framework/hosting/simple_module_hosting/assets.py`. +- **SQLModel is the project-wide model standard.** Not relevant here (no models), + but do not introduce Pydantic `BaseModel` for the asset record — use a + `dataclass`. +- **No new Python dependencies.** In particular, no CSS parser for the SM022 / + SM023 lints. +- **`modules.manifest.json` keeps its exact current shape** (`{name: pages_dir}`). + Downstream apps (`smpy_gis`, `smpy_saas`, `laco_wiki_python`, + `smpy_pagebuilder`) hold their own copy of `vite.config.ts` and must keep + building untouched. +- **Alias segment is the lowercase Python package name** (`#module/blog_posts`), + not `ModuleMeta.name` (`BlogPosts`). +- Diagnostic codes: **SM022**, **SM023**. Existing set stops at SM021. +- Run `make ci-python-lint` rather than `make lint` — `make lint` fails + repo-wide on a pre-existing invalid `preset` key in `biome.json`. + +--- + +### Task 1: Verify the two load-bearing resolver claims + +The spec flags two assumptions as untested. Both change the design if wrong, so +they are checked before any code is written. + +**Files:** +- Create (throwaway): `$CLAUDE_JOB_DIR/tmp/css-spike/` + +- [x] **Step 1: Build a minimal Tailwind 4.2.4 spike** + +Create a scratch Vite project that imports a CSS file from outside its root via +a `resolve.alias`, and `@source`s a directory by absolute path. Put a class in +the aliased CSS and a utility class in a `.tsx` under the absolute `@source` +dir. + +- [x] **Step 2: Run the build and inspect output CSS** + +Confirm two things independently: +1. The aliased `@import` resolved (the module CSS rule appears in output). +2. The absolute `@source` scanned (the utility from that dir appears in output). + +- [x] **Step 3: Record the result** + +If the absolute `@source` fails, the `@source` section switches to alias +specifiers too, and Task 3's emission changes accordingly. If the alias +`@import` fails, stop — the design needs revisiting. + +- [x] **Step 4: Note whether `fs.allow` was needed** + +Run the dev server against a file outside the root without `fs.allow` and see +whether the CSS still resolves. `@import` is inlined at transform time, so it +may not need whitelisting. + +#### Findings (2026-08-06) + +Verified against the versions actually installed in this repo: +`@tailwindcss/vite` **4.2.4**, `tailwindcss` **4.2.4**, `vite` **8.0.10**. + +Both load-bearing claims hold, so **the design proceeds unchanged**: + +| Claim | Result | +|---|---| +| `resolve.alias` governs CSS `@import` | ✅ `@import "#module/spikemod/styles.css" layer(components)` resolved to a directory outside the Vite root and emitted as `@layer components{.spike-module-rule{color:#639}}` | +| Absolute `@source` scans | ✅ `@source "/abs/outside-src"` scanned a `.tsx` outside the root; `.p-7{padding:calc(var(--spacing) * 7)}` was emitted | + +Two further results beyond what the plan asked for: + +- **The unlayered/layered split works as designed.** `theme.css` imported + without a `layer()` clause registered `--color-spikebrand:#123456` as a real + design token, while `styles.css` imported with `layer(components)` landed + inside `@layer components`. The whole chain composes: a token defined in a + module's `theme.css` produced a working `text-spikebrand` utility for a class + used only in an absolute-`@source`d file outside the root. +- **`server.fs.allow` is NOT needed for module CSS.** The dev server, with no + `fs.allow` entry for the external directory, served `/app.css?direct` + (HTTP 200) containing all three signals. `@import` is inlined at transform + time rather than served as a URL, confirming the spec's hypothesis. Task 4 + still pushes each package dir onto `moduleFsAllow` — harmless, and it remains + genuinely required for `.tsx` *pages*, which are served as URLs. + +--- + +### Task 2: Asset discovery — `compute_module_assets` + +**Files:** +- Create: `framework/hosting/simple_module_hosting/assets.py` +- Test: `framework/cli/tests/test_module_css_assets.py` + +**Interfaces:** +- Consumes: `simple_module_core.get_module_package_name`, `ModuleBase`. +- Produces: + - `@dataclass(frozen=True) ModuleAssets` with fields + `name: str`, `package_name: str`, `package_dir: Path`, + `pages_dir: Path | None`, `theme_css: Path | None`, `styles_css: Path | None`. + - `compute_module_assets(modules: Sequence[ModuleBase]) -> list[ModuleAssets]` + — preserves input (discovery) order; includes a module if it has **any** of + pages/theme/styles. + +- [x] **Step 1: Write the failing test** + +```python +"""Tests for module CSS asset discovery and emission.""" + +from __future__ import annotations + + +class TestComputeModuleAssets: + async def test_detects_theme_and_styles(self, tmp_path, monkeypatch): + """A module shipping theme.css/styles.css has them detected.""" + import sys + + from simple_module_core import ModuleBase, ModuleMeta + from simple_module_hosting.assets import compute_module_assets + + pkg = tmp_path / "styled_mod" + (pkg / "pages").mkdir(parents=True) + (pkg / "theme.css").write_text("@theme { --color-x: red; }\n") + (pkg / "styles.css").write_text(".x { color: red; }\n") + (pkg / "__init__.py").write_text("") + monkeypatch.syspath_prepend(str(tmp_path)) + sys.modules.pop("styled_mod", None) + + module_src = "from simple_module_core import ModuleBase, ModuleMeta\n" + (pkg / "module.py").write_text(module_src) + + class StyledMod(ModuleBase): + meta = ModuleMeta(name="Styled") + + # Force the package association the helper resolves against. + StyledMod.__module__ = "styled_mod.module" + + result = compute_module_assets([StyledMod()]) + assert len(result) == 1 + entry = result[0] + assert entry.package_name == "styled_mod" + assert entry.theme_css is not None and entry.theme_css.name == "theme.css" + assert entry.styles_css is not None and entry.styles_css.name == "styles.css" + assert entry.pages_dir is not None + + async def test_css_only_module_is_included(self, tmp_path, monkeypatch): + """A module with CSS but no pages/ still appears (the manifest gap).""" + import sys + + from simple_module_core import ModuleBase, ModuleMeta + from simple_module_hosting.assets import compute_module_assets + + pkg = tmp_path / "cssonly_mod" + pkg.mkdir(parents=True) + (pkg / "__init__.py").write_text("") + (pkg / "styles.css").write_text(".y { color: blue; }\n") + monkeypatch.syspath_prepend(str(tmp_path)) + sys.modules.pop("cssonly_mod", None) + + class CssOnly(ModuleBase): + meta = ModuleMeta(name="CssOnly") + + CssOnly.__module__ = "cssonly_mod.module" + + result = compute_module_assets([CssOnly()]) + assert [e.name for e in result] == ["CssOnly"] + assert result[0].pages_dir is None + assert result[0].styles_css is not None + + async def test_preserves_discovery_order(self): + """Order follows discover_modules(), not alphabetical.""" + from simple_module_core import discover_modules + from simple_module_hosting.assets import compute_module_assets + + modules = discover_modules() + result = compute_module_assets(modules) + names = [e.name for e in result] + assert names != sorted(names) or len(names) <= 1 + discovery_order = [m.meta.name for m in modules] + assert names == [n for n in discovery_order if n in set(names)] +``` + +- [x] **Step 2: Run test to verify it fails** + +Run: `uv run pytest framework/cli/tests/test_module_css_assets.py -v` +Expected: FAIL with `ModuleNotFoundError: No module named 'simple_module_hosting.assets'` + +- [x] **Step 3: Write the implementation** + +```python +"""Per-module frontend asset discovery (pages + CSS). + +Separate from ``manifest.py`` because that file is already at the repo's +300-line cap. ``manifest.py`` imports from here. +""" + +from __future__ import annotations + +import importlib.resources +import logging +from collections.abc import Sequence +from dataclasses import dataclass +from pathlib import Path + +from simple_module_core import ModuleBase, get_module_package_name + +logger = logging.getLogger(__name__) + +THEME_CSS = "theme.css" +STYLES_CSS = "styles.css" + + +@dataclass(frozen=True) +class ModuleAssets: + """Frontend assets a single module contributes.""" + + name: str + package_name: str + package_dir: Path + pages_dir: Path | None + theme_css: Path | None + styles_css: Path | None + + +def compute_module_assets(modules: Sequence[ModuleBase]) -> list[ModuleAssets]: + """Return per-module frontend assets, preserving discovery order. + + Discovery order matters: ``discover_modules()`` topologically sorts by + ``depends_on``, so emitting CSS in this order lets a dependent module + override its dependency. A module is included only if it contributes at + least one asset. + """ + result: list[ModuleAssets] = [] + for mod in modules: + pkg_name = get_module_package_name(mod) + try: + pkg_root = Path(str(importlib.resources.files(pkg_name))) + except ModuleNotFoundError: + logger.debug( + "Module '%s': package %s not importable — skipping", mod.meta.name, pkg_name + ) + continue + pages_dir = pkg_root / "pages" + theme = pkg_root / THEME_CSS + styles = pkg_root / STYLES_CSS + entry = ModuleAssets( + name=mod.meta.name, + package_name=pkg_name, + package_dir=pkg_root.resolve(), + pages_dir=pages_dir.resolve() if pages_dir.is_dir() else None, + theme_css=theme.resolve() if theme.is_file() else None, + styles_css=styles.resolve() if styles.is_file() else None, + ) + if entry.pages_dir or entry.theme_css or entry.styles_css: + result.append(entry) + return result +``` + +- [x] **Step 4: Run test to verify it passes** + +Run: `uv run pytest framework/cli/tests/test_module_css_assets.py -v` +Expected: PASS + +- [x] **Step 5: Commit** + +```bash +git add framework/hosting/simple_module_hosting/assets.py framework/cli/tests/test_module_css_assets.py +git commit -m "feat(hosting): discover per-module theme.css/styles.css assets" +``` + +--- + +### Task 3: Emit CSS imports and `modules.assets.json` + +**Files:** +- Modify: `framework/hosting/simple_module_hosting/assets.py` (add emitters) +- Modify: `framework/hosting/simple_module_hosting/manifest.py` (CSS section + assets file) +- Test: `framework/cli/tests/test_module_css_assets.py` + +**Interfaces:** +- Consumes: `ModuleAssets`, `compute_module_assets` from Task 2. +- Produces: + - `ALIAS_PREFIX = "#module"` + - `render_modules_css(assets, *, in_repo: Callable[[Path], bool]) -> str` + - `render_assets_json(assets) -> str` + - `write_module_pages_manifest` returns an extra `"assets"` key. + +- [x] **Step 1: Write the failing test** + +```python +class TestCssEmission: + async def test_emits_alias_imports_with_no_relative_paths(self, tmp_path): + """Generated CSS references modules by alias, never by ../.. path.""" + from simple_module_core import discover_modules + from simple_module_hosting.manifest import write_module_pages_manifest + + modules = discover_modules() + written = write_module_pages_manifest(modules, tmp_path) + css = written["css"].read_text(encoding="utf-8") + + import_lines = [ln for ln in css.splitlines() if ln.startswith("@import")] + for line in import_lines: + assert "../" not in line, f"generated @import must not use relative paths: {line}" + assert '"#module/' in line, f"@import must use the alias prefix: {line}" + + async def test_styles_layered_theme_unlayered(self, tmp_path): + """theme.css imports unlayered; styles.css imports into layer(components).""" + from simple_module_hosting.assets import ModuleAssets, render_modules_css + + assets = [ + ModuleAssets( + name="Gis", + package_name="gis", + package_dir=tmp_path / "gis", + pages_dir=None, + theme_css=tmp_path / "gis" / "theme.css", + styles_css=tmp_path / "gis" / "styles.css", + ) + ] + css = render_modules_css(assets, in_repo=lambda _p: False) + + assert '@import "#module/gis/theme.css";' in css + assert '@import "#module/gis/styles.css" layer(components);' in css + # theme must precede styles so tokens exist before rules consume them + assert css.index("theme.css") < css.index("styles.css") + + async def test_source_skips_in_repo_but_import_does_not(self, tmp_path): + """@source is wheel-only; @import is emitted for every module.""" + from simple_module_hosting.assets import ModuleAssets, render_modules_css + + assets = [ + ModuleAssets( + name="Local", + package_name="local", + package_dir=tmp_path / "local", + pages_dir=tmp_path / "local" / "pages", + theme_css=None, + styles_css=tmp_path / "local" / "styles.css", + ) + ] + css = render_modules_css(assets, in_repo=lambda _p: True) + + assert "@source" not in css, "in-repo pages are covered by the static glob" + assert '@import "#module/local/styles.css" layer(components);' in css + + async def test_writes_assets_json(self, tmp_path): + """modules.assets.json is emitted alongside the existing three files.""" + import json + + from simple_module_core import discover_modules + from simple_module_hosting.manifest import write_module_pages_manifest + + written = write_module_pages_manifest(discover_modules(), tmp_path) + assets_path = tmp_path / "modules.assets.json" + assert assets_path.is_file() + assert written["assets"] == assets_path + + data = json.loads(assets_path.read_text(encoding="utf-8")) + entry = data["Dashboard"] + assert entry["package_name"] == "dashboard" + assert entry["package"].endswith("dashboard") + assert set(entry) == {"package_name", "package", "pages", "theme", "styles"} + + async def test_manifest_json_shape_unchanged(self, tmp_path): + """modules.manifest.json stays {name: pages_dir} for downstream vite configs.""" + import json + + from simple_module_core import discover_modules + from simple_module_hosting.manifest import write_module_pages_manifest + + write_module_pages_manifest(discover_modules(), tmp_path) + data = json.loads((tmp_path / "modules.manifest.json").read_text(encoding="utf-8")) + assert all(isinstance(v, str) for v in data.values()) +``` + +- [x] **Step 2: Run test to verify it fails** + +Run: `uv run pytest framework/cli/tests/test_module_css_assets.py -v` +Expected: FAIL — `render_modules_css` / `render_assets_json` undefined, and +`written` has no `"assets"` key. + +- [x] **Step 3: Implement the emitters in `assets.py`** + +```python +ALIAS_PREFIX = "#module" + +_CSS_HEADER = """\ +/* AUTO-GENERATED by simple_module_hosting.assets — do not edit by hand. + * Regenerate with: smpy gen-pages + * + * Emission order is module discovery order (topological by depends_on), + * so a dependent module's CSS can override its dependency's. + */ +""" + + +def render_modules_css( + assets: Sequence[ModuleAssets], + *, + in_repo: Callable[[Path], bool], +) -> str: + """Render ``modules.generated.css``. + + Three sections. ``@source`` is emitted only for wheel-installed modules + (in-repo pages are already covered by the static glob in the host's + styles.css), but ``@import`` is emitted for every module — there is no + static-glob equivalent for CSS. + + ``theme.css`` is imported unlayered so its ``@theme`` blocks register as + design tokens; ``styles.css`` is imported into ``layer(components)`` so a + module rule can never outrank a Tailwind utility. + """ + source_lines = [ + f'@source "{e.pages_dir.as_posix()}/**/*.{{ts,tsx}}";' + for e in assets + if e.pages_dir and not in_repo(e.pages_dir) + ] + theme_lines = [ + f'@import "{ALIAS_PREFIX}/{e.package_name}/{THEME_CSS}";' for e in assets if e.theme_css + ] + style_lines = [ + f'@import "{ALIAS_PREFIX}/{e.package_name}/{STYLES_CSS}" layer(components);' + for e in assets + if e.styles_css + ] + + out = [_CSS_HEADER] + for heading, lines in ( + ("/* ── @source: class scanning (wheel-installed modules) ── */", source_lines), + ("/* ── theme: unlayered, registers @theme tokens ── */", theme_lines), + ("/* ── styles: layer(components), always loses to utilities ── */", style_lines), + ): + if lines: + out.append("") + out.append(heading) + out.extend(lines) + out.append("") + return "\n".join(out) + + +def render_assets_json(assets: Sequence[ModuleAssets]) -> str: + """Render ``modules.assets.json`` — the richer companion to the pages manifest.""" + payload = { + e.name: { + "package_name": e.package_name, + "package": e.package_dir.as_posix(), + "pages": e.pages_dir.as_posix() if e.pages_dir else None, + "theme": e.theme_css.as_posix() if e.theme_css else None, + "styles": e.styles_css.as_posix() if e.styles_css else None, + } + for e in assets + } + return json.dumps(payload, indent=2, sort_keys=True) + "\n" +``` + +Add `import json` and `from collections.abc import Callable, Sequence` at the +top of `assets.py`. + +- [x] **Step 4: Rewire `manifest.py`** + +Replace the CSS-emission block in `write_module_pages_manifest` with calls to +the new renderers, add the `modules.assets.json` write, and include `"assets"` +in the returned dict. Delete the now-unused `_GENERATED_CSS_HEADER`. Keep +`_is_in_repo_module` and pass it as the `in_repo` callable: + +```python +assets = compute_module_assets(modules) +css_text = render_modules_css(assets, in_repo=lambda p: _is_in_repo_module(p, repo_root)) +assets_path = output_dir / "modules.assets.json" +wrote_assets = _write_if_changed(assets_path, render_assets_json(assets)) +``` + +- [x] **Step 5: Run tests to verify they pass** + +Run: `uv run pytest framework/cli/tests/test_module_css_assets.py framework/cli/tests/test_module_pages_manifest.py -v` +Expected: PASS (both files — the existing manifest tests must not regress) + +- [x] **Step 6: Check the file-size cap** + +Run: `uv run python scripts/check_file_size.py` +Expected: PASS — `manifest.py` must still be under 300 lines. + +- [x] **Step 7: Commit** + +```bash +git add framework/hosting/simple_module_hosting/ framework/cli/tests/test_module_css_assets.py +git commit -m "feat(hosting): emit aliased module CSS imports and modules.assets.json" +``` + +--- + +### Task 4: Wire Vite aliases and widen the host `@source` glob + +**Files:** +- Modify: `host/client_app/vite.config.ts` +- Modify: `host/client_app/styles.css` +- Modify: `framework/cli/simple_module_cli/templates/host/client_app/vite.config.ts` +- Modify: `framework/cli/simple_module_cli/templates/host/client_app/styles.css` + +**Interfaces:** +- Consumes: `modules.assets.json` from Task 3. +- Produces: a `#module/` alias per module, resolvable from CSS + and TSX alike. + +- [x] **Step 1: Read `modules.assets.json` in `vite.config.ts`** + +Add alongside the existing manifest read, leaving that read intact: + +```ts +// Aliases let generated CSS reference module files as +// "#module//styles.css" instead of a brittle +// ../../../.venv/lib/python3.12/site-packages//styles.css path. +// @tailwindcss/vite resolves CSS @import through Vite's resolver +// (createResolver({...config.resolve, ...})), so resolve.alias applies. +type ModuleAsset = { package_name: string; package: string }; +const moduleAliases: { find: string; replacement: string }[] = []; +const assetsPath = path.resolve(__dirname, 'modules.assets.json'); +if (fs.existsSync(assetsPath)) { + const assets = JSON.parse(fs.readFileSync(assetsPath, 'utf-8')) as Record; + for (const entry of Object.values(assets)) { + moduleAliases.push({ + find: `#module/${entry.package_name}`, + replacement: entry.package, + }); + if (!moduleFsAllow.includes(entry.package)) moduleFsAllow.push(entry.package); + } + // Longest find first so "#module/gis" can't shadow "#module/gis_extra". + moduleAliases.sort((a, b) => b.find.length - a.find.length); +} +``` + +- [x] **Step 2: Register the aliases** + +In the existing `resolve:` block, add `alias: moduleAliases,` beside +`tsconfigPaths` and `dedupe`. + +- [x] **Step 3: Widen the in-repo `@source` glob** + +In both `styles.css` files, change +`@source "../../modules/*/*/pages/**/*.{ts,tsx}";` to +`@source "../../modules/*/*/**/*.{ts,tsx}";` so `.ts`/`.tsx` outside `pages/` +is scanned. The `{ts,tsx}` filter keeps `.py` out. + +- [x] **Step 4: Mirror steps 1-2 into the scaffold template** + +Apply the same edits to +`framework/cli/simple_module_cli/templates/host/client_app/vite.config.ts` so +new `smpy new` apps get aliases from the start. + +- [x] **Step 5: Verify the templates still typecheck and the app builds** + +Run: `npx tsc --noEmit -p host/client_app/tsconfig.json` +Expected: PASS + +Run: `npm run build` +Expected: PASS + +- [x] **Step 6: Commit** + +```bash +git add host/client_app framework/cli/simple_module_cli/templates/host/client_app +git commit -m "feat(client): resolve module CSS through per-module Vite aliases" +``` + +--- + +### Task 5: Prove it end-to-end with a real module + +A green unit suite can coexist with Tailwind emitting nothing, so this is the +task that actually proves the feature. + +**Files:** +- Create: `modules/dashboard/dashboard/styles.css` +- Test: `framework/cli/tests/test_module_css_build.py` + +- [x] **Step 1: Add a real stylesheet to an in-repo module** + +```css +/* Dashboard module styles. Imported into layer(components) by + * modules.generated.css, so these rules always lose to Tailwind utilities. */ +@layer components { + .dashboard-stat-grid { + display: grid; + gap: var(--spacing, 0.25rem); + } +} +``` + +- [x] **Step 2: Write the failing build test** + +```python +"""Proves module-shipped CSS survives a real Tailwind build.""" + +from __future__ import annotations + +import subprocess +from pathlib import Path + +import pytest + +REPO_ROOT = Path(__file__).resolve().parents[3] + + +@pytest.mark.slow +class TestModuleCssReachesBundle: + def test_module_class_is_emitted_by_vite_build(self): + """A class defined only in a module's styles.css appears in built CSS.""" + subprocess.run( + ["uv", "run", "smpy", "gen-pages", "--host-dir", "host/client_app"], + cwd=REPO_ROOT, + check=True, + capture_output=True, + ) + subprocess.run(["npm", "run", "build"], cwd=REPO_ROOT, check=True, capture_output=True) + built = list((REPO_ROOT / "host" / "static" / "dist" / "assets").glob("*.css")) + assert built, "no CSS emitted by the build" + combined = "\n".join(p.read_text(encoding="utf-8") for p in built) + assert "dashboard-stat-grid" in combined +``` + +- [x] **Step 3: Run it and watch it fail, then pass** + +Run: `uv run pytest framework/cli/tests/test_module_css_build.py -v` + +Before Tasks 3-4 land this fails on the missing class. After they land it +passes. If it fails *after*, the alias did not resolve — re-check Task 1's +findings before changing the emitter. + +- [x] **Step 4: Commit** + +```bash +git add modules/dashboard/dashboard/styles.css framework/cli/tests/test_module_css_build.py +git commit -m "test(client): assert module-shipped CSS reaches the built bundle" +``` + +--- + +### Task 6: SM022 / SM023 diagnostics + +**Files:** +- Create: `framework/core/simple_module_core/diagnostics/_css.py` +- Modify: `framework/core/simple_module_core/diagnostics/_module.py:32-39` +- Test: `framework/core/tests/test_css_diagnostics.py` + +**Interfaces:** +- Consumes: `Diagnostic`, `DiagnosticLevel`, `ModuleBase`, and the `src_dir` + already computed by `ModuleDiagnostics.run`. +- Produces: `check_module_css(mod: ModuleBase, src_dir: Path) -> list[Diagnostic]`, + called exactly like the existing `check_js_workspace_files(mod, src_dir)`. + +- [x] **Step 1: Write the failing test** + +```python +"""SM022/SM023 — module CSS placed in the wrong file.""" + +from __future__ import annotations + + +class TestModuleCssDiagnostics: + def _mod(self): + from simple_module_core import ModuleBase, ModuleMeta + + class Styled(ModuleBase): + meta = ModuleMeta(name="Styled") + + return Styled() + + def test_sm022_theme_at_rule_in_styles_css(self, tmp_path): + """@theme inside styles.css is inert under layer(components).""" + from simple_module_core.diagnostics._css import check_module_css + + (tmp_path / "styles.css").write_text("@theme {\n --color-x: red;\n}\n") + diags = check_module_css(self._mod(), tmp_path) + assert [d.code for d in diags] == ["SM022"] + + def test_sm023_plain_rule_in_theme_css(self, tmp_path): + """An unlayered rule in theme.css outranks every Tailwind utility.""" + from simple_module_core.diagnostics._css import check_module_css + + (tmp_path / "theme.css").write_text(".card {\n padding: 0;\n}\n") + diags = check_module_css(self._mod(), tmp_path) + assert [d.code for d in diags] == ["SM023"] + + def test_root_block_allowed_in_theme_css(self, tmp_path): + """:root custom-property blocks are legitimate in theme.css.""" + from simple_module_core.diagnostics._css import check_module_css + + (tmp_path / "theme.css").write_text("@theme {\n --a: 1;\n}\n:root {\n --b: 2;\n}\n") + assert check_module_css(self._mod(), tmp_path) == [] + + def test_nested_at_rule_not_flagged(self, tmp_path): + """Only top-level constructs count — brace depth is tracked.""" + from simple_module_core.diagnostics._css import check_module_css + + (tmp_path / "styles.css").write_text("@layer components {\n .x { color: red; }\n}\n") + assert check_module_css(self._mod(), tmp_path) == [] + + def test_comments_stripped_before_scanning(self, tmp_path): + """A commented-out @theme is not a finding.""" + from simple_module_core.diagnostics._css import check_module_css + + (tmp_path / "styles.css").write_text("/* @theme { --x: 1; } */\n.y { color: red; }\n") + assert check_module_css(self._mod(), tmp_path) == [] + + def test_clean_module_has_no_findings(self, tmp_path): + from simple_module_core.diagnostics._css import check_module_css + + (tmp_path / "theme.css").write_text("@theme {\n --color-x: red;\n}\n") + (tmp_path / "styles.css").write_text("@layer components {\n .x { color: red; }\n}\n") + assert check_module_css(self._mod(), tmp_path) == [] + + def test_missing_files_are_not_findings(self, tmp_path): + from simple_module_core.diagnostics._css import check_module_css + + assert check_module_css(self._mod(), tmp_path) == [] +``` + +- [x] **Step 2: Run test to verify it fails** + +Run: `uv run pytest framework/core/tests/test_css_diagnostics.py -v` +Expected: FAIL — no module `simple_module_core.diagnostics._css` + +- [x] **Step 3: Implement `_css.py`** + +A line-oriented scan tracking brace depth, comments stripped first. No CSS +parser dependency — this catches the ordinary mistake, not pathological input. + +`THEME_ONLY_AT_RULES = {"@theme", "@custom-variant", "@utility"}` are the +constructs that must live in `theme.css`; `theme.css` may additionally hold +`@font-face`, `@import`, `@charset` and `:root` blocks. + +- [x] **Step 4: Run test to verify it passes** + +Run: `uv run pytest framework/core/tests/test_css_diagnostics.py -v` +Expected: PASS + +- [x] **Step 5: Wire into `ModuleDiagnostics.run`** + +Add `diagnostics.extend(check_module_css(mod, src_dir))` to the file-based +check loop, and the import at the top of `_module.py`. + +- [x] **Step 6: Verify the whole diagnostic suite and `make doctor`** + +Run: `uv run pytest framework/core/tests/ -v` +Run: `make doctor` +Expected: PASS, and `make doctor` reports no new findings for in-repo modules. + +- [x] **Step 7: Commit** + +```bash +git add framework/core/simple_module_core/diagnostics framework/core/tests/test_css_diagnostics.py +git commit -m "feat(doctor): add SM022/SM023 for misplaced module CSS" +``` + +--- + +### Task 7: Documentation + +**Files:** +- Modify: `docs/module-authoring.md` +- Modify: `CLAUDE.md` +- Modify: `framework/cli/simple_module_cli/templates/module/README.md.tpl` + +- [x] **Step 1: Add a "Styling" section to `docs/module-authoring.md`** + +Cover: the two-file convention; that `@theme` goes in `theme.css` and component +rules in `styles.css`; why (unlayered CSS beats every layered rule, and a +layered `@theme` is inert); the cascade order DS theme < module theme < app +overrides; and that no packaging change is needed because Hatch already ships +files under the package dir. + +- [x] **Step 2: Update `CLAUDE.md`** + +Add `theme.css` / `styles.css` to the module-layout tree, and add SM022/SM023 +to the diagnostic-code list. + +- [x] **Step 3: Mention the convention in the scaffold README template** + +One short paragraph — the scaffold deliberately does *not* create empty CSS +files, so the README is where authors learn the convention exists. + +- [x] **Step 4: Run the full check** + +Run: `make ci-python-lint` +Run: `make test-py` +Expected: PASS + +- [x] **Step 5: Commit** + +```bash +git add docs CLAUDE.md framework/cli/simple_module_cli/templates/module/README.md.tpl +git commit -m "docs: document the module theme.css/styles.css convention" +``` + +--- + +## Self-Review + +**Spec coverage:** §1 authoring surface → Task 2. §2 emission → Task 3. §3 +cascade → Tasks 3 (layering) + 4 (glob widening). §4 alias/manifest → Tasks 3 +(`modules.assets.json`) + 4 (Vite wiring). §5 diagnostics → Task 6. §6 tests → +Tasks 2, 3, 5, 6. §7 docs → Task 7. "To verify during implementation" → Task 1. +No gaps. + +**Type consistency:** `ModuleAssets` field names (`package_name`, `package_dir`, +`pages_dir`, `theme_css`, `styles_css`) are used identically in Tasks 2 and 3. +The JSON keys (`package_name`, `package`, `pages`, `theme`, `styles`) match +between `render_assets_json` in Task 3 and the `ModuleAsset` TS type in Task 4. +`check_module_css(mod, src_dir)` matches the existing +`check_js_workspace_files(mod, src_dir)` signature it sits beside. + +**Ordering note:** Task 5's build test only passes once Tasks 3 and 4 have both +landed, which is why it is sequenced after them. diff --git a/docs/superpowers/specs/2026-08-05-module-css-packaging-design.md b/docs/superpowers/specs/2026-08-05-module-css-packaging-design.md new file mode 100644 index 00000000..d4b95b41 --- /dev/null +++ b/docs/superpowers/specs/2026-08-05-module-css-packaging-design.md @@ -0,0 +1,285 @@ +# Module CSS after packaging — design + +**Date:** 2026-08-05 +**Status:** Approved, ready for implementation planning + +## Problem + +A module packaged as a wheel cannot contribute CSS to the host's Tailwind build. + +The only thing `smpy gen-pages` emits today for a wheel-installed module is a +`@source` line in `host/client_app/modules.generated.css`, which drives Tailwind +*class scanning* and nothing else. There is no mechanism for a module to ship +actual CSS — design tokens, component classes, keyframes, vendor stylesheets. + +This is not hypothetical. In `smpy_gis` the app had to hand-edit +`host/client_app/styles.css` with: + +```css +@import "../../modules/gis/gis/static/tokens.css"; +``` + +That line: + +- only resolves for an **in-repo** module; it breaks entirely once the module is + pip-installed into `.venv/.../site-packages/`, +- is **not regenerated** by `smpy gen-pages`, so installing a module means + remembering to hand-edit host CSS, +- has **no defined ordering** against other modules' CSS or against the design + system's own `@theme`. + +Four distinct pain points were confirmed as in scope: modules can't ship CSS, +`@source` handling is fragile, cascade/ordering is undefined, and dev/HMR +ergonomics are poor. + +## Non-goals + +- Runtime CSS injection or per-request stylesheet composition. +- Automatic scoping or class-name prefixing of module CSS. +- Enabling/disabling a module's CSS at runtime. +- Scaffolding empty `theme.css` / `styles.css` files from `make new-module`. + Files that are always imported but usually empty are noise; the convention is + documented instead. + +## Design + +### 1. Authoring surface — convention, zero config + +``` +modules/// +├── module.py +├── theme.css # optional — @theme tokens, @custom-variant, @font-face +├── styles.css # optional — component rules, keyframes, vendor CSS +└── pages/ +``` + +Both files are optional and auto-detected, exactly the way `pages/` is +auto-detected today by `compute_module_pages()`. No new `ModuleBase` hook, no +`module.py` change. + +**No packaging change is required.** The module wheel template already declares +`[tool.hatch.build.targets.wheel] packages = [""]`, and Hatch includes every +file under the package directory — `.tsx` and `.css` included. The existing +`force-include` block is only needed for artifacts that live *outside* the +package dir (`package.json`) or are gitignored (`static/dist`). + +Splitting into two files rather than one is what makes the cascade rules in §3 +structurally enforceable rather than merely documented. + +### 2. Emission — `modules.generated.css` + +`write_module_pages_manifest` gains a third responsibility and emits three +ordered sections: + +```css +/* AUTO-GENERATED by simple_module_hosting.manifest — do not edit by hand. */ + +/* ── @source: class scanning (wheel-installed modules only) ── */ +@source "/abs/path/to/site-packages/gis/**/*.{ts,tsx}"; + +/* ── theme: unlayered ── */ +@import "#module/gis/theme.css"; + +/* ── styles: layer(components) ── */ +@import "#module/gis/styles.css" layer(components); +``` + +Three decisions here: + +**Import order is module discovery order, not alphabetical.** +`discover_modules()` already returns modules topologically sorted by +`ModuleMeta.depends_on`, which is the same order the host invokes `register_*` +hooks in. Emitting CSS in that order means a module that depends on another can +override it, matching the ordering contract module authors already reason about. +The current generator does `for name in sorted(pages_map)`. + +**`@import` is emitted for every module; `@source` only for wheel-installed +ones.** This asymmetry is deliberate. In-repo module *pages* are already covered +by the static `@source "../../modules/*/*/pages/**/*.{ts,tsx}"` glob in +`host/client_app/styles.css`, so emitting an absolute `@source` for them would +duplicate it — that is what `_is_in_repo_module()` exists to prevent. There is no +static-glob equivalent for CSS, so the `_is_in_repo_module` skip must stay +confined to the `@source` section. + +**`@source` keeps absolute paths; `@import` uses an alias.** These go through +two different resolvers. `@source` is scanned by Tailwind's own machinery, where +an absolute path is fine. `@import` goes through Vite's resolver — see §4. + +### 3. Cascade + +Host `styles.css` becomes: + +```css +@import "../../packages/ui/src/styles/globals.css"; /* tailwindcss + DS @theme */ +@import "./modules.generated.css"; /* module theme + components */ +/* app-level @theme overrides go here — last word */ + +@source "./**/*.{ts,tsx}"; +@source "../../modules/*/*/**/*.{ts,tsx}"; /* widened from pages/** */ +``` + +Resulting token precedence: **design-system theme < module theme < app +overrides.** A module normally *adds* tokens (`--color-map-water`); when it +deliberately redefines a design-system token it wins, and the app still has the +final word via its own `@theme` block below the generated import. + +Module component rules land in `layer(components)` and therefore always lose to +Tailwind utilities. + +This is the whole reason for the two-file split. Tailwind v4 expands +`@import "tailwindcss"` into `@layer theme, base, components, utilities`, and +**unlayered CSS beats every layered rule**. A module shipping a bare +`.card { padding: 0 }` would silently override `p-4` on that element. But +`@theme` blocks *must* be unlayered to register design tokens, and a `@theme` +inside a layered import is inert. One file cannot satisfy both constraints; +two files give each one job and make the footgun impossible rather than +merely discouraged. + +**Module imports must stay after `@import "tailwindcss"`.** This is a +correctness constraint, not a style preference. Layer precedence is fixed by +the order layers are first declared. A `layer(components)` import appearing +before Tailwind's `@layer theme, base, components, utilities;` declaration +would register `components` as the *first* layer, dropping module component +rules below preflight. + +The in-repo `@source` glob widens from `pages/**` to `**` so `.ts`/`.tsx` living +outside `pages/` is scanned too. The `{ts,tsx}` extension filter keeps `.py` +files out of the scan. + +### 4. Alias resolution and the Vite manifest + +Module CSS is referenced through a per-module Vite alias, so no generated file +ever contains a path like +`../../../.venv/lib/python3.12/site-packages/gis/styles.css`. + +```ts +// vite.config.ts — built from modules.assets.json +resolve: { + alias: moduleAliases, // longest `find` first +} +``` + +**This is verified, not assumed.** `@tailwindcss/vite` 4.2.4 builds its CSS +import resolver as: + +```js +createResolver({ ...config.resolve, extensions: ['.css'], mainFields: ['style'], + conditions: ['style', 'development|production'], + tryIndex: false, preferRelative: true }) +``` + +in both of its code paths. The spread of `...config.resolve` carries `alias` +through, so `resolve.alias` governs CSS `@import`. The resolver additionally +only accepts a result that is absolute and ends in `.css`, which an +alias→directory replacement satisfies. + +Aliases must be sorted **longest `find` first** so that `#module/gis` does not +shadow `#module/gis-extra`. + +**Alias casing.** The alias segment is the module's **lowercase Python package +name** (`gis`, `background_tasks`), not `ModuleMeta.name`. `modules.manifest.json` +is keyed by `mod.meta.name`, which is PascalCase for Inertia page resolution +(`BlogPosts`); reusing that in a CSS specifier would read badly and invites +case-sensitivity bugs across filesystems. `modules.assets.json` therefore carries +the package name as an explicit field rather than leaving callers to derive it: + +```jsonc +{ + "BlogPosts": { + "package_name": "blog_posts", // -> alias "#module/blog_posts" + "package": "/abs/.../blog_posts", + "pages": "/abs/.../blog_posts/pages", + "theme": null, + "styles": "/abs/.../blog_posts/styles.css" + } +} +``` + +The top-level key stays `mod.meta.name` for consistency with the existing +manifest. + +**Manifest compatibility.** `modules.manifest.json` maps module name → *pages* +directory, and `vite.config.ts` derives the package dir as +`path.dirname(pagesDir)` for `server.fs.allow`. A CSS-only module with no +`pages/` never enters that manifest, so it would be missing from both the alias +list and `fs.allow`. + +Rather than change the manifest's value shape — `vite.config.ts` is scaffolded +*into* each app, so `smpy_gis`, `smpy_saas`, `laco_wiki_python` and +`smpy_pagebuilder` each hold their own copy and would all break at once — add a +new **additive** file: + +```jsonc +// modules.assets.json +{ + "Gis": { + "package": "/abs/.../gis", + "pages": "/abs/.../gis/pages", // or null + "theme": "/abs/.../gis/theme.css", // or null + "styles": "/abs/.../gis/styles.css" // or null + } +} +``` + +`modules.manifest.json` keeps its exact current shape and semantics. Existing +apps keep building untouched; they opt into module CSS by updating +`vite.config.ts`, which they must do anyway to gain the aliases. + +### 5. Doctor diagnostics + +Two new warnings. Both codes are free — the existing set stops at `SM021`. + +- **SM022** (warn) — `styles.css` contains a top-level `@theme`, + `@custom-variant`, or `@utility`. These are inert inside `layer(components)`; + move them to `theme.css`. +- **SM023** (warn) — `theme.css` contains a top-level rule whose selector is + anything other than `:root`. Unlayered rules beat every Tailwind utility; + move them to `styles.css`. + +These are warnings rather than errors: both describe CSS that is legal and will +build, just miscategorised in a way that produces surprising cascade behaviour. + +**Detection is a line-oriented scan of top-level at-rules and selectors, not a +CSS parse.** No CSS parser is added as a Python dependency for a lint. The check +tracks brace depth to distinguish top-level constructs from nested ones, strips +`/* */` comments first, and reports the offending line number. This misses +pathological input (an at-rule assembled by preprocessing, say) and that is +acceptable — the diagnostic exists to catch the ordinary mistake of putting +`@theme` in the wrong file, not to be a conformance checker. + +### 6. Tests + +Extend `framework/cli/tests/test_module_pages_manifest.py`: + +- `theme.css` / `styles.css` detection, including modules that ship one, both, + or neither. +- Alias specifiers are emitted (no `../` sequences anywhere in the generated + CSS). +- Emission follows discovery order, not alphabetical order. +- The asymmetry: `@import` for in-repo modules, no `@source` for them. +- A CSS-only module (no `pages/`) still appears in `modules.assets.json`. + +Doctor tests covering SM022 and SM023, positive and negative. + +One build-level assertion that a class used only in a module-shipped stylesheet +survives `npm run build` — the check that would actually catch this class of +regression, since every unit test above can pass while Tailwind still emits +nothing. + +### 7. Docs + +- `docs/module-authoring.md` — new "Styling" section covering the two-file + convention, the cascade rules, and why `@theme` belongs in `theme.css`. +- `CLAUDE.md` — note the convention alongside the existing module-layout tree, + and add SM022/SM023 to the diagnostic-code list. + +## To verify during implementation + +Two claims are load-bearing and should be tested rather than trusted, given one +resolver claim was already wrong once during design: + +1. `@source` with an absolute path genuinely works in Tailwind 4.2.4. If it does + not, the `@source` section switches to the same alias treatment as `@import`. +2. Whether `server.fs.allow` matters for CSS at all — `@import` is inlined at + transform time rather than served as a URL, so it may only affect HMR + watching of files outside the project root. diff --git a/framework/cli/simple_module_cli/templates/host/client_app/styles.css b/framework/cli/simple_module_cli/templates/host/client_app/styles.css index e520f7a6..da9205b1 100644 --- a/framework/cli/simple_module_cli/templates/host/client_app/styles.css +++ b/framework/cli/simple_module_cli/templates/host/client_app/styles.css @@ -1,10 +1,25 @@ @import "@simple-module-py/ui/styles/globals.css"; @import "./modules.generated.css"; -/* App-level source scanning — host code + workspace module pages. - * Wheel-installed modules are covered by modules.generated.css. */ +/* App-level source scanning — host code + workspace modules. + * Wheel-installed modules are covered by modules.generated.css. + * + * The module glob deliberately covers the whole package, not just pages/: + * shared components, hooks and Puck blocks carry Tailwind classes too, and + * scanning pages/ alone meant such a class only reached the bundle when some + * other scanned file happened to use it as well. The {ts,tsx} filter is what + * keeps .py files out of the scan. */ @source "./**/*.{ts,tsx}"; -@source "../../modules/*/*/pages/**/*.{ts,tsx}"; +@source "../../modules/*/*/**/*.{ts,tsx}"; +/* The second `*` above matches any directory under modules//, not just + * the Python package — so a module's sibling tests/ would be scanned too and + * test-only classes would land in the production bundle. + * + * Consequence: a module's Python package must not itself be named `tests` + * (i.e. modules/tests/tests/), or this line would exclude the package too. No + * glob can tell a package directory from a sibling, and `tests` is not a + * viable package name anyway — it collides with pytest collection. */ +@source not "../../modules/*/tests/**"; body { margin: 0; diff --git a/framework/cli/simple_module_cli/templates/host/client_app/vite.config.ts b/framework/cli/simple_module_cli/templates/host/client_app/vite.config.ts index d32e2998..b1244148 100644 --- a/framework/cli/simple_module_cli/templates/host/client_app/vite.config.ts +++ b/framework/cli/simple_module_cli/templates/host/client_app/vite.config.ts @@ -68,6 +68,37 @@ if (fs.existsSync(manifestPath)) { } } } + +// Per-module aliases, so generated CSS can say +// @import "#module/gis/styles.css" +// instead of a ../../../.venv/lib/python3.12/site-packages/gis/styles.css +// path that breaks the moment the interpreter version changes. +// +// `@tailwindcss/vite` builds its CSS import resolver with +// `createResolver({ ...config.resolve, ... })`, so `resolve.alias` governs +// CSS `@import` as well as JS — verified against @tailwindcss/vite 4.2.4. +// +// Read from modules.assets.json rather than modules.manifest.json: the +// manifest is keyed off `pages/`, so a module shipping only CSS never +// appears in it. +type ModuleAsset = { package_name: string; package: string }; +const moduleAliases: { find: string; replacement: string }[] = []; +const assetsPath = path.resolve(__dirname, 'modules.assets.json'); +let moduleAssets: Record = {}; +try { + moduleAssets = JSON.parse(fs.readFileSync(assetsPath, 'utf-8')); +} catch { + // Absent until `smpy gen-pages` runs — proceed with no aliases. +} +for (const entry of Object.values(moduleAssets)) { + moduleAliases.push({ find: `#module/${entry.package_name}`, replacement: entry.package }); + if (!moduleFsAllow.includes(entry.package)) moduleFsAllow.push(entry.package); +} +// Keep the alias list in a stable, longest-first order. Vite matches a string +// `find` on exact equality or a `/`-bounded prefix, so `#module/gis` could not +// swallow `#module/gis_extra` in any order — this is just determinism, not a +// correctness fix. +moduleAliases.sort((a, b) => b.find.length - a.find.length); const fakeWorkspaceImporter = path.join(fsRoot, 'package.json'); // CJS-only deps like `clsx`, `tailwind-merge`, `class-variance-authority` @@ -200,6 +231,9 @@ export default defineConfig({ plugins: [moduleBareImportResolver(), react(), tailwindcss()], root: __dirname, resolve: { + // `#module/` -> that module's package directory. Consumed by the + // @import lines in modules.generated.css. + alias: moduleAliases, dedupe: [...REACT_CORE_DEPS, '@simple-module-py/ui', '@simple-module-py/i18n'], }, optimizeDeps: { diff --git a/framework/cli/simple_module_cli/templates/module/README.md.tpl b/framework/cli/simple_module_cli/templates/module/README.md.tpl index f7ad5920..df8a4d47 100644 --- a/framework/cli/simple_module_cli/templates/module/README.md.tpl +++ b/framework/cli/simple_module_cli/templates/module/README.md.tpl @@ -39,6 +39,24 @@ Hosts mount the bundle automatically at when it exists, an empty dict otherwise (so dev without a build step doesn't fail). +## Styling (optional) + +This module can ship CSS that lands in the host's Tailwind build with no +wiring on the host side. Create either file beside `pages/` and it is +picked up automatically — the scaffold deliberately does not create them +empty, since a file that is always imported but usually empty is noise: + +- `{{PACKAGE_NAME}}/theme.css` — `@theme` tokens, `@custom-variant`, + `@font-face`. Imported **unlayered**, because a `@theme` block inside a + cascade layer registers nothing. +- `{{PACKAGE_NAME}}/styles.css` — component rules, keyframes, vendor CSS. + Imported into **`layer(components)`**, so these rules always lose to a + Tailwind utility and a consuming app can still override them. + +Putting a construct in the wrong file builds fine but cascades in a way you +probably did not intend, so `make doctor` warns: `SM022` for `@theme` in +`styles.css`, `SM023` for an unlayered rule in `theme.css`. + ## Continuous integration Two workflows live under `.github/workflows/`: diff --git a/framework/cli/tests/test_module_css_assets.py b/framework/cli/tests/test_module_css_assets.py new file mode 100644 index 00000000..c608dfd1 --- /dev/null +++ b/framework/cli/tests/test_module_css_assets.py @@ -0,0 +1,223 @@ +"""Tests for module CSS asset discovery and emission (`smpy gen-pages`).""" + +from __future__ import annotations + +import sys +from pathlib import Path + + +def _make_importable_module(tmp_path: Path, pkg_name: str, klass_name: str): + """Create a real importable package on disk and return a ModuleBase bound to it. + + ``get_module_package_name`` derives the package from the class's + ``__module__``, and ``compute_module_assets`` then resolves that package + via ``importlib.resources.files``. So the package has to genuinely exist + on ``sys.path`` — a mock won't exercise the code path we care about. + """ + from simple_module_core import ModuleBase, ModuleMeta + + pkg = tmp_path / pkg_name + pkg.mkdir(parents=True, exist_ok=True) + (pkg / "__init__.py").write_text("", encoding="utf-8") + if str(tmp_path) not in sys.path: + sys.path.insert(0, str(tmp_path)) + sys.modules.pop(pkg_name, None) + + klass = type( + klass_name, + (ModuleBase,), + {"meta": ModuleMeta(name=klass_name), "__module__": f"{pkg_name}.module"}, + ) + return klass(), pkg + + +class TestComputeModuleAssets: + async def test_detects_theme_and_styles(self, tmp_path): + """A module shipping theme.css/styles.css has both detected.""" + from simple_module_hosting.assets import compute_module_assets + + mod, pkg = _make_importable_module(tmp_path, "styled_mod", "Styled") + (pkg / "pages").mkdir() + (pkg / "theme.css").write_text("@theme { --color-x: red; }\n", encoding="utf-8") + (pkg / "styles.css").write_text(".x { color: red; }\n", encoding="utf-8") + + result = compute_module_assets([mod]) + + assert len(result) == 1 + entry = result[0] + assert entry.package_name == "styled_mod" + assert entry.theme_css is not None and entry.theme_css.name == "theme.css" + assert entry.styles_css is not None and entry.styles_css.name == "styles.css" + assert entry.pages_dir is not None + + async def test_css_only_module_is_included(self, tmp_path): + """A module with CSS but no pages/ still appears — the manifest.json gap.""" + from simple_module_hosting.assets import compute_module_assets + + mod, pkg = _make_importable_module(tmp_path, "cssonly_mod", "CssOnly") + (pkg / "styles.css").write_text(".y { color: blue; }\n", encoding="utf-8") + + result = compute_module_assets([mod]) + + assert [e.name for e in result] == ["CssOnly"] + assert result[0].pages_dir is None + assert result[0].theme_css is None + assert result[0].styles_css is not None + + async def test_module_with_no_assets_is_omitted(self, tmp_path): + """A module contributing nothing frontend-ish is skipped, not an error.""" + from simple_module_hosting.assets import compute_module_assets + + mod, _pkg = _make_importable_module(tmp_path, "headless_mod", "Headless") + + assert compute_module_assets([mod]) == [] + + async def test_preserves_discovery_order(self): + """Order follows discover_modules() (topological), not alphabetical.""" + from simple_module_core import discover_modules + from simple_module_hosting.assets import compute_module_assets + + modules = discover_modules() + result = compute_module_assets(modules) + + names = [e.name for e in result] + discovery_order = [m.meta.name for m in modules] + assert names == [n for n in discovery_order if n in set(names)] + + +def _assets(tmp_path: Path, **overrides): + """Build a single ModuleAssets entry with sensible defaults.""" + from simple_module_hosting.assets import ModuleAssets + + defaults = { + "name": "Gis", + "package_name": "gis", + "package_dir": tmp_path / "gis", + "pages_dir": None, + "theme_css": None, + "styles_css": None, + } + return ModuleAssets(**{**defaults, **overrides}) + + +class TestCssEmission: + async def test_styles_layered_theme_unlayered(self, tmp_path): + """theme.css imports unlayered; styles.css imports into layer(components).""" + from simple_module_hosting.assets import render_modules_css + + entry = _assets( + tmp_path, + theme_css=tmp_path / "gis" / "theme.css", + styles_css=tmp_path / "gis" / "styles.css", + ) + css = render_modules_css([entry], in_repo=lambda _p: False) + + assert '@import "#module/gis/theme.css";' in css + assert '@import "#module/gis/styles.css" layer(components);' in css + # Tokens must be declared before the rules that consume them. + assert css.index("theme.css") < css.index("styles.css") + + async def test_source_skips_in_repo_but_import_does_not(self, tmp_path): + """@source is wheel-only; @import is emitted for every module.""" + from simple_module_hosting.assets import render_modules_css + + entry = _assets( + tmp_path, + name="Local", + package_name="local", + pages_dir=tmp_path / "local" / "pages", + styles_css=tmp_path / "local" / "styles.css", + ) + css = render_modules_css([entry], in_repo=lambda _p: True) + + assert "@source" not in css, "in-repo pages are covered by the static glob" + assert '@import "#module/local/styles.css" layer(components);' in css + + async def test_source_emitted_for_wheel_modules(self, tmp_path): + """A wheel-installed module gets an absolute @source glob.""" + from simple_module_hosting.assets import render_modules_css + + pages = tmp_path / "gis" / "pages" + css = render_modules_css([_assets(tmp_path, pages_dir=pages)], in_repo=lambda _p: False) + + assert f'@source "{pages.as_posix()}/**/*.{{ts,tsx}}";' in css + + async def test_module_without_css_emits_no_import(self, tmp_path): + """Pages-only modules contribute @source but no @import.""" + from simple_module_hosting.assets import render_modules_css + + entry = _assets(tmp_path, pages_dir=tmp_path / "gis" / "pages") + css = render_modules_css([entry], in_repo=lambda _p: False) + + assert "@import" not in css + + async def test_output_is_formatter_clean(self, tmp_path): + """No doubled blank lines, exactly one trailing newline. + + `biome ci .` lints the whole tree, and `modules.generated.css` is + untracked but not exempt — so a stray blank line here fails `make lint` + for anyone who has run `gen-pages`. + """ + from simple_module_hosting.assets import render_modules_css + + for entry in ( + _assets(tmp_path, styles_css=tmp_path / "gis" / "styles.css"), + _assets(tmp_path, pages_dir=tmp_path / "gis" / "pages"), + ): + css = render_modules_css([entry], in_repo=lambda _p: False) + assert "\n\n\n" not in css, f"doubled blank line in:\n{css!r}" + assert css.endswith("\n") and not css.endswith("\n\n"), repr(css[-20:]) + + empty = render_modules_css([], in_repo=lambda _p: False) + assert "\n\n\n" not in empty and empty.endswith("\n") + + async def test_no_relative_paths_anywhere(self, tmp_path): + """The whole point of aliases: generated @import lines never use ../..""" + from simple_module_core import discover_modules + from simple_module_hosting.manifest import write_module_pages_manifest + + written = write_module_pages_manifest(discover_modules(), tmp_path) + css = written["css"].read_text(encoding="utf-8") + + for line in css.splitlines(): + if line.startswith("@import"): + assert "../" not in line, f"@import must not use a relative path: {line}" + assert '"#module/' in line, f"@import must use the alias prefix: {line}" + + +class TestAssetsManifest: + async def test_writes_assets_json(self, tmp_path): + """modules.assets.json is emitted alongside the existing three files.""" + import json + + from simple_module_core import discover_modules + from simple_module_hosting.manifest import write_module_pages_manifest + + written = write_module_pages_manifest(discover_modules(), tmp_path) + + assets_path = tmp_path / "modules.assets.json" + assert assets_path.is_file() + assert written["assets"] == assets_path + + data = json.loads(assets_path.read_text(encoding="utf-8")) + entry = data["Dashboard"] + assert entry["package_name"] == "dashboard" + assert entry["package"].endswith("dashboard") + assert set(entry) == {"package_name", "package", "pages", "theme", "styles"} + + async def test_manifest_json_shape_unchanged(self, tmp_path): + """modules.manifest.json stays {name: pages_dir} for downstream vite configs. + + Every downstream app holds its own copy of vite.config.ts and reads + this file; changing its value shape would break them all at once. + """ + import json + + from simple_module_core import discover_modules + from simple_module_hosting.manifest import write_module_pages_manifest + + write_module_pages_manifest(discover_modules(), tmp_path) + + data = json.loads((tmp_path / "modules.manifest.json").read_text(encoding="utf-8")) + assert data, "manifest should not be empty" + assert all(isinstance(v, str) for v in data.values()) diff --git a/framework/cli/tests/test_module_css_build.py b/framework/cli/tests/test_module_css_build.py new file mode 100644 index 00000000..da6c94f8 --- /dev/null +++ b/framework/cli/tests/test_module_css_build.py @@ -0,0 +1,72 @@ +"""Proves module-shipped CSS survives a real Tailwind build. + +Every unit test around `render_modules_css` can stay green while Tailwind +emits nothing at all — the generated `@import` only pays off if Vite resolves +the `#module/` alias and Tailwind keeps the rule. This is the one +assertion that exercises that whole chain, so it drives a genuine build +rather than inspecting the generated text. + +Skipped where a build is impossible: CI's `python-tests` job installs Python +deps only (`make install-py`), so `node_modules` is absent there. The JS jobs +and local runs do have it. +""" + +from __future__ import annotations + +import shutil +import subprocess +from pathlib import Path + +import pytest + +REPO_ROOT = Path(__file__).resolve().parents[3] + +pytestmark = [ + pytest.mark.skipif( + not (REPO_ROOT / "node_modules").is_dir(), + reason="node_modules absent — run `npm install` to exercise the real build", + ), + pytest.mark.skipif(shutil.which("npm") is None, reason="npm not on PATH"), +] + + +class TestModuleCssReachesBundle: + def test_module_class_is_emitted_by_vite_build(self): + """A class defined only in a module's styles.css appears in the built CSS. + + `.dashboard-stat-grid` is declared in modules/dashboard/dashboard/styles.css + and referenced by no TSX anywhere, so it can only reach the bundle via the + generated `@import "#module/dashboard/styles.css"`. + """ + subprocess.run( + [ + "uv", + "run", + "--project", + "host", + "smpy", + "host", + "gen-pages", + "--host-dir=host/client_app", + ], + cwd=REPO_ROOT, + check=True, + capture_output=True, + ) + + generated = (REPO_ROOT / "host/client_app/modules.generated.css").read_text( + encoding="utf-8" + ) + assert '@import "#module/dashboard/styles.css" layer(components);' in generated, ( + f"gen-pages did not emit the dashboard stylesheet import:\n{generated}" + ) + + subprocess.run(["npm", "run", "build"], cwd=REPO_ROOT, check=True, capture_output=True) + + built = list((REPO_ROOT / "host/static/dist/assets").glob("*.css")) + assert built, "no CSS emitted by the build" + combined = "\n".join(p.read_text(encoding="utf-8") for p in built) + assert "dashboard-stat-grid" in combined, ( + "module-shipped CSS did not reach the bundle — the #module alias " + "most likely failed to resolve" + ) diff --git a/framework/cli/tests/test_module_pages_manifest.py b/framework/cli/tests/test_module_pages_manifest.py index 23079665..28833ab4 100644 --- a/framework/cli/tests/test_module_pages_manifest.py +++ b/framework/cli/tests/test_module_pages_manifest.py @@ -33,7 +33,7 @@ class HeadlessMod(ModuleBase): assert "Headless" not in result async def test_write_manifest_emits_json_and_ts(self, tmp_path): - """write_module_pages_manifest emits the JSON manifest, TS glob, and Tailwind CSS files.""" + """write_module_pages_manifest emits the JSON manifest, TS glob, CSS and assets files.""" import json from simple_module_core import discover_modules @@ -45,10 +45,17 @@ async def test_write_manifest_emits_json_and_ts(self, tmp_path): manifest = tmp_path / "modules.manifest.json" generated = tmp_path / "modules.generated.ts" css = tmp_path / "modules.generated.css" + assets = tmp_path / "modules.assets.json" assert manifest.is_file() assert generated.is_file() assert css.is_file() - assert written == {"manifest": manifest, "generated": generated, "css": css} + assert assets.is_file() + assert written == { + "manifest": manifest, + "generated": generated, + "css": css, + "assets": assets, + } data = json.loads(manifest.read_text(encoding="utf-8")) assert "Dashboard" in data diff --git a/framework/core/simple_module_core/diagnostics/_css.py b/framework/core/simple_module_core/diagnostics/_css.py new file mode 100644 index 00000000..d718bcb8 --- /dev/null +++ b/framework/core/simple_module_core/diagnostics/_css.py @@ -0,0 +1,214 @@ +"""SM022/SM023: module CSS placed in the file with the wrong cascade position. + +A module ships its styles in two files, and which one a construct lands in +decides how it cascades: + +* ``theme.css`` is imported **unlayered**, because a ``@theme`` block inside a + cascade layer registers no design tokens at all. Unlayered CSS also outranks + every layered rule, so a plain rule here would beat any Tailwind utility. +* ``styles.css`` is imported into ``layer(components)``, so its rules always + lose to a utility — which is what you want for component CSS, and useless + for ``@theme``. + +Both codes are warnings, not errors: the misplaced CSS is legal and builds +fine, it just cascades in a way the author almost certainly did not intend. + +Detection is a line-oriented scan that tracks brace depth, not a CSS parse — +adding a CSS parser as a runtime dependency to power a lint is not worth it. +This catches the ordinary mistake of putting a construct in the wrong file; it +is not a conformance checker, and pathological input (an at-rule assembled by +preprocessing, say) is out of scope. +""" + +from __future__ import annotations + +from pathlib import Path +from typing import TYPE_CHECKING + +from simple_module_core.diagnostics._types import Diagnostic, DiagnosticLevel + +if TYPE_CHECKING: + from simple_module_core.module import ModuleBase + +THEME_CSS = "theme.css" +STYLES_CSS = "styles.css" + +# Constructs that only do anything unlayered, so they belong in theme.css. +THEME_ONLY_AT_RULES = ("@theme", "@custom-variant", "@utility") + + +def _top_level_preludes(text: str) -> list[tuple[str, int]]: + """Return ``(prelude, line_number)`` for every top-level construct. + + A "prelude" is the selector or at-rule text preceding a top-level ``{``, + or a whole statement at-rule terminated by ``;`` at depth 0. Anything + nested inside braces is skipped, which is what keeps a rule inside + ``@layer components { ... }`` from being mistaken for a top-level one. + + Comments and quoted strings are consumed inline rather than pre-stripped, + because a brace inside either is not structural. ``content: "{"`` in an + icon-font rule would otherwise desynchronise the depth counter for the + whole rest of the file — every later top-level construct would be read as + nested and silently skipped. + + A quote only opens a string when its partner is found before the line + ends, which is the rule CSS itself applies (a string cannot contain a raw + newline). Treating every quote as an opener is what makes an *unmatched* + one dangerous: the lone apostrophe in ``url(it's.png); }`` would swallow + the closing brace on that same line and desync the counter permanently. + An unmatched quote is therefore just an ordinary character. + """ + out: list[tuple[str, int]] = [] + depth = 0 + buf: list[str] = [] + line = 1 + buf_line = 1 + i = 0 + n = len(text) + + def flush() -> None: + prelude = "".join(buf).strip() + if prelude: + out.append((prelude, buf_line)) + + def string_end(start: int, quote: str) -> int | None: + """Index just past the closing quote, or None if it never closes.""" + j = start + 1 + while j < n: + c = text[j] + if c == "\\": + # An escape consumes the next character whole — including a + # newline, the one way a CSS string legitimately spans lines. + j += 2 + continue + if c == "\n": + return None + if c == quote: + return j + 1 + j += 1 + return None + + while i < n: + ch = text[i] + + if ch == "/" and i + 1 < n and text[i + 1] == "*": + end = text.find("*/", i + 2) + end = n if end == -1 else end + 2 + line += text.count("\n", i, end) + i = end + continue + + if ch == "\\" and i + 1 < n: + # CSS escapes apply outside strings too — Tailwind leans on this + # heavily (`.mt-\[773px\]`). Consuming the pair keeps an escaped + # quote from being mistaken for a string opener, which is also + # what stops `\'` repeated across a long line from making + # string_end re-scan that line once per quote (quadratic). + if depth == 0: + if not buf: + buf_line = line + buf.append(text[i : i + 2]) + if text[i + 1] == "\n": + line += 1 + i += 2 + continue + + if ch in "\"'": + end = string_end(i, ch) + if end is not None: + if depth == 0: + if not buf: + buf_line = line + buf.append(text[i:end]) + line += text.count("\n", i, end) + i = end + continue + # Unmatched: fall through and treat it as an ordinary character. + + if ch == "{": + if depth == 0: + flush() + buf.clear() + depth += 1 + elif ch == "}": + depth = max(0, depth - 1) + if depth == 0: + buf.clear() + elif depth == 0: + if ch == ";": + flush() + buf.clear() + elif buf: + buf.append(ch) + elif not ch.isspace(): + # Start the buffer at the first non-space character, so + # buf_line points at the construct rather than at whatever + # blank lines preceded it. + buf_line = line + buf.append(ch) + + if ch == "\n": + line += 1 + i += 1 + return out + + +def _is_root_selector(prelude: str) -> bool: + """True if every comma-separated part of the selector is :root-based. + + ``:root``, ``:root[data-theme="dark"]`` and a comma-separated list of both + are the normal way to declare design tokens, so they are legitimate in + theme.css even though they are plain rules. + """ + parts = [p.strip() for p in prelude.split(",") if p.strip()] + return bool(parts) and all(p.startswith(":root") for p in parts) + + +def check_module_css(mod: ModuleBase, src_dir: Path) -> list[Diagnostic]: + """Warn when module CSS sits in the file with the wrong cascade position.""" + diagnostics: list[Diagnostic] = [] + + styles = src_dir / STYLES_CSS + if styles.is_file(): + for prelude, line in _top_level_preludes(styles.read_text("utf-8")): + at_rule = prelude.split()[0].lower() if prelude.split() else "" + if at_rule in THEME_ONLY_AT_RULES: + diagnostics.append( + Diagnostic( + level=DiagnosticLevel.WARNING, + code="SM022", + message=( + f"{at_rule} in {STYLES_CSS} is inert — that file is " + "imported into layer(components)" + ), + module_name=mod.meta.name, + file=f"{styles}:{line}", + suggestion=( + f"Move the {at_rule} block to {src_dir / THEME_CSS}, which is " + "imported unlayered so its tokens actually register" + ), + ) + ) + + theme = src_dir / THEME_CSS + if theme.is_file(): + for prelude, line in _top_level_preludes(theme.read_text("utf-8")): + if prelude.startswith("@") or _is_root_selector(prelude): + continue + diagnostics.append( + Diagnostic( + level=DiagnosticLevel.WARNING, + code="SM023", + message=( + f"Unlayered rule '{prelude}' in {THEME_CSS} outranks every Tailwind utility" + ), + module_name=mod.meta.name, + file=f"{theme}:{line}", + suggestion=( + f"Move it to {src_dir / STYLES_CSS}, which is imported into " + "layer(components) so utilities still win" + ), + ) + ) + + return diagnostics diff --git a/framework/core/simple_module_core/diagnostics/_module.py b/framework/core/simple_module_core/diagnostics/_module.py index 64a2e836..e5fe1323 100644 --- a/framework/core/simple_module_core/diagnostics/_module.py +++ b/framework/core/simple_module_core/diagnostics/_module.py @@ -7,6 +7,7 @@ from typing import TYPE_CHECKING from simple_module_core.diagnostics._coupling import check_framework_module_coupling +from simple_module_core.diagnostics._css import check_module_css from simple_module_core.diagnostics._inertia_api import check_inertia_api_calls from simple_module_core.diagnostics._js_workspace import check_js_workspace_files from simple_module_core.diagnostics._pages import check_pages, find_render_calls @@ -37,6 +38,7 @@ def run(self, modules: list[ModuleBase]) -> list[Diagnostic]: diagnostics.extend(check_pages(mod, src_dir, rendered_pages)) diagnostics.extend(check_js_workspace_files(mod, src_dir)) diagnostics.extend(check_inertia_api_calls(mod, src_dir)) + diagnostics.extend(check_module_css(mod, src_dir)) return diagnostics diff --git a/framework/core/tests/test_css_diagnostics.py b/framework/core/tests/test_css_diagnostics.py new file mode 100644 index 00000000..2de87213 --- /dev/null +++ b/framework/core/tests/test_css_diagnostics.py @@ -0,0 +1,97 @@ +"""SM022/SM023 — module CSS placed in the wrong file. + +The theme.css / styles.css split is what makes the cascade rules structural +rather than merely documented, so putting a construct in the wrong file +produces surprising-but-legal CSS. Both codes are warnings for that reason: +the build succeeds either way. +""" + +from __future__ import annotations + + +def _mod(): + from simple_module_core import ModuleBase, ModuleMeta + + class Styled(ModuleBase): + meta = ModuleMeta(name="Styled") + + return Styled() + + +class TestSm022ThemeConstructsInStylesCss: + def test_theme_at_rule_in_styles_css(self, tmp_path): + """@theme inside styles.css is inert — styles.css is imported layered.""" + from simple_module_core.diagnostics._css import check_module_css + + (tmp_path / "styles.css").write_text("@theme {\n --color-x: red;\n}\n") + + diags = check_module_css(_mod(), tmp_path) + + assert [d.code for d in diags] == ["SM022"] + assert diags[0].level.value == "warning" + + def test_custom_variant_in_styles_css(self, tmp_path): + from simple_module_core.diagnostics._css import check_module_css + + (tmp_path / "styles.css").write_text("@custom-variant dark (&:where(.dark, .dark *));\n") + + assert [d.code for d in check_module_css(_mod(), tmp_path)] == ["SM022"] + + def test_utility_at_rule_in_styles_css(self, tmp_path): + from simple_module_core.diagnostics._css import check_module_css + + (tmp_path / "styles.css").write_text("@utility tab-4 {\n tab-size: 4;\n}\n") + + assert [d.code for d in check_module_css(_mod(), tmp_path)] == ["SM022"] + + def test_reports_the_offending_line(self, tmp_path): + from simple_module_core.diagnostics._css import check_module_css + + (tmp_path / "styles.css").write_text( + "@layer components {\n .x { color: red; }\n}\n\n@theme {\n --a: 1;\n}\n" + ) + + diags = check_module_css(_mod(), tmp_path) + + assert len(diags) == 1 + # Line number rides in `file` as path:line, matching _inertia_api.py. + assert diags[0].file.endswith("styles.css:5") + + +class TestSm023UnlayeredRulesInThemeCss: + def test_plain_rule_in_theme_css(self, tmp_path): + """An unlayered rule in theme.css outranks every Tailwind utility.""" + from simple_module_core.diagnostics._css import check_module_css + + (tmp_path / "theme.css").write_text(".card {\n padding: 0;\n}\n") + + diags = check_module_css(_mod(), tmp_path) + + assert [d.code for d in diags] == ["SM023"] + assert diags[0].level.value == "warning" + + def test_root_block_allowed(self, tmp_path): + """:root custom-property blocks are legitimate in theme.css.""" + from simple_module_core.diagnostics._css import check_module_css + + (tmp_path / "theme.css").write_text("@theme {\n --a: 1;\n}\n:root {\n --b: 2;\n}\n") + + assert check_module_css(_mod(), tmp_path) == [] + + def test_root_variants_allowed(self, tmp_path): + """Dark-mode token blocks keyed off :root are the normal pattern.""" + from simple_module_core.diagnostics._css import check_module_css + + (tmp_path / "theme.css").write_text(':root,\n:root[data-theme="dark"] {\n --b: 2;\n}\n') + + assert check_module_css(_mod(), tmp_path) == [] + + def test_font_face_allowed(self, tmp_path): + """@font-face is explicitly part of what theme.css is for.""" + from simple_module_core.diagnostics._css import check_module_css + + (tmp_path / "theme.css").write_text( + "@font-face {\n font-family: X;\n src: url(x.woff2);\n}\n" + ) + + assert check_module_css(_mod(), tmp_path) == [] diff --git a/framework/core/tests/test_css_scanner.py b/framework/core/tests/test_css_scanner.py new file mode 100644 index 00000000..52ddc760 --- /dev/null +++ b/framework/core/tests/test_css_scanner.py @@ -0,0 +1,242 @@ +"""The CSS scanner behind SM022/SM023. + +`check_module_css` is only as good as its ability to find *top-level* +constructs, and it does that with a hand-rolled brace-depth scan rather than a +CSS parse — no parser joins the runtime dependencies to power a lint. These +tests pin the tokenising rules that scan depends on: comments, quoted strings, +escapes, and the line numbers reported alongside a finding. + +Every case here is a bug that shipped at some point during review: each one is +a way a stray character silently desynced the depth counter and made the lint +stop reporting anything at all. +""" + +from __future__ import annotations + + +def _mod(): + from simple_module_core import ModuleBase, ModuleMeta + + class Styled(ModuleBase): + meta = ModuleMeta(name="Styled") + + return Styled() + + +class TestScannerRobustness: + def test_nested_at_rule_not_flagged(self, tmp_path): + """Only top-level constructs count — brace depth is tracked.""" + from simple_module_core.diagnostics._css import check_module_css + + (tmp_path / "styles.css").write_text("@layer components {\n .x { color: red; }\n}\n") + + assert check_module_css(_mod(), tmp_path) == [] + + def test_nested_rule_in_theme_css_not_flagged(self, tmp_path): + """A rule nested inside @theme is not a top-level rule.""" + from simple_module_core.diagnostics._css import check_module_css + + (tmp_path / "theme.css").write_text("@theme {\n --a: 1;\n}\n") + + assert check_module_css(_mod(), tmp_path) == [] + + def test_commented_out_construct_is_not_a_finding(self, tmp_path): + """A commented-out @theme is not a finding.""" + from simple_module_core.diagnostics._css import check_module_css + + (tmp_path / "styles.css").write_text("/* @theme { --x: 1; } */\n.y { color: red; }\n") + + assert check_module_css(_mod(), tmp_path) == [] + + def test_comment_does_not_shift_line_numbers(self, tmp_path): + """Newlines inside a skipped comment must still be counted.""" + from simple_module_core.diagnostics._css import check_module_css + + (tmp_path / "styles.css").write_text("/* a\n multi-line\n comment */\n@theme {\n}\n") + + diags = check_module_css(_mod(), tmp_path) + + assert [d.file.rsplit(":", 1)[1] for d in diags] == ["4"] + + def test_clean_module_has_no_findings(self, tmp_path): + from simple_module_core.diagnostics._css import check_module_css + + (tmp_path / "theme.css").write_text("@theme {\n --color-x: red;\n}\n") + (tmp_path / "styles.css").write_text("@layer components {\n .x { color: red; }\n}\n") + + assert check_module_css(_mod(), tmp_path) == [] + + def test_missing_files_are_not_findings(self, tmp_path): + from simple_module_core.diagnostics._css import check_module_css + + assert check_module_css(_mod(), tmp_path) == [] + + def test_open_brace_in_a_string_does_not_hide_later_findings(self, tmp_path): + """A brace inside a string must not desync the depth counter. + + An icon-font rule like `content: "{"` used to leave the scanner + permanently one level deep, so every later top-level construct read as + nested — silently swallowing the very @theme SM022 exists to catch. + """ + from simple_module_core.diagnostics._css import check_module_css + + (tmp_path / "styles.css").write_text('.icon { content: "{"; }\n@theme {\n --a: 1;\n}\n') + + diags = check_module_css(_mod(), tmp_path) + + assert [d.code for d in diags] == ["SM022"] + assert diags[0].file.endswith("styles.css:2") + + def test_close_brace_in_a_string_is_not_a_finding(self, tmp_path): + """A closing brace inside a string must not end the block early.""" + from simple_module_core.diagnostics._css import check_module_css + + (tmp_path / "theme.css").write_text(':root {\n --icon-close: "}";\n}\n') + + assert check_module_css(_mod(), tmp_path) == [] + + def test_escaped_quote_in_a_string(self, tmp_path): + """A backslash-escaped quote does not terminate the string.""" + from simple_module_core.diagnostics._css import check_module_css + + (tmp_path / "theme.css").write_text(':root {\n --q: "a\\"{b";\n}\n') + + assert check_module_css(_mod(), tmp_path) == [] + + def test_single_quoted_strings_handled(self, tmp_path): + from simple_module_core.diagnostics._css import check_module_css + + (tmp_path / "styles.css").write_text( + ".icon { content: '{'; }\n@utility tab-4 {\n tab-size: 4;\n}\n" + ) + + assert [d.code for d in check_module_css(_mod(), tmp_path)] == ["SM022"] + + def test_unterminated_string_does_not_swallow_the_file(self, tmp_path): + """A stray quote must not disable the lint for everything after it. + + A CSS string cannot contain a raw newline, so an unterminated one ends + at end-of-line. Without that bound, a single typo'd quote left the + scanner permanently "inside a string" and every later brace stopped + counting — silently hiding the rest of the file's findings. + """ + from simple_module_core.diagnostics._css import check_module_css + + (tmp_path / "styles.css").write_text( + '.a { color: red; }\n.b {\n content: "unterminated;\n}\n@theme {\n --b: 1;\n}\n' + ) + + diags = check_module_css(_mod(), tmp_path) + + assert [d.code for d in diags] == ["SM022"] + assert diags[0].file.endswith("styles.css:5") + + def test_apostrophe_in_unquoted_url_does_not_swallow_the_file(self, tmp_path): + """An unquoted url() token may hold a lone apostrophe, e.g. inline SVG.""" + from simple_module_core.diagnostics._css import check_module_css + + (tmp_path / "styles.css").write_text( + ".icon {\n" + " background: url(data:image/svg+xml,it's here);\n" + "}\n" + "@theme {\n" + " --a: 1;\n" + "}\n" + ) + + assert [d.code for d in check_module_css(_mod(), tmp_path)] == ["SM022"] + + def test_stray_quote_does_not_eat_a_brace_on_its_own_line(self, tmp_path): + """The unmatched-quote case, with the closing brace on the SAME line. + + Bounding strings at newlines was not enough: the `}` here sits on the + stray quote's own line, so it was consumed as string content and the + depth counter stayed desynced for the rest of the file. A quote only + opens a string if its partner appears before the line ends. + """ + from simple_module_core.diagnostics._css import check_module_css + + (tmp_path / "styles.css").write_text( + ".icon { background: url(it's.png); }\n@theme {\n --a: 1;\n}\n" + ) + + diags = check_module_css(_mod(), tmp_path) + + assert [d.code for d in diags] == ["SM022"] + assert diags[0].file.endswith("styles.css:2") + + def test_multiline_quoted_value_is_not_a_finding(self, tmp_path): + """grid-template-areas spreads several complete strings over lines.""" + from simple_module_core.diagnostics._css import check_module_css + + (tmp_path / "styles.css").write_text( + "@layer components {\n" + " .g {\n" + " grid-template-areas:\n" + ' "a b"\n' + ' "c d";\n' + " }\n" + "}\n" + ) + + assert check_module_css(_mod(), tmp_path) == [] + + def test_escaped_selector_is_not_a_string_opener(self, tmp_path): + """CSS escapes apply outside strings — Tailwind selectors rely on it.""" + from simple_module_core.diagnostics._css import check_module_css + + (tmp_path / "styles.css").write_text( + ".mt-\\[773px\\] { margin-top: 773px; }\n@theme {\n --a: 1;\n}\n" + ) + + diags = check_module_css(_mod(), tmp_path) + + assert [d.code for d in diags] == ["SM022"] + assert diags[0].file.endswith("styles.css:2") + + def test_scanner_is_linear_on_pathological_input(self, tmp_path): + """A long run of escaped quotes must not make the scan quadratic. + + `doctor` runs this over every installed module, including third-party + ones, so a minified or vendored stylesheet must not stall the lint. + Before escapes were consumed outside strings, 58KB of `\\'` took ~10s. + """ + import time + + from simple_module_core.diagnostics._css import check_module_css + + (tmp_path / "styles.css").write_text("\\'" * 60_000) + + started = time.perf_counter() + check_module_css(_mod(), tmp_path) + elapsed = time.perf_counter() - started + + assert elapsed < 2.0, f"scan took {elapsed:.1f}s — likely quadratic again" + + def test_escaped_newline_continues_a_string(self, tmp_path): + """A backslash-escaped newline is the one way a string spans lines.""" + from simple_module_core.diagnostics._css import check_module_css + + (tmp_path / "theme.css").write_text(':root {\n --a: "x\\\n y";\n}\n') + + assert check_module_css(_mod(), tmp_path) == [] + + def test_brace_inside_a_comment_is_not_structural(self, tmp_path): + """An unbalanced brace in a comment must not shift depth either.""" + from simple_module_core.diagnostics._css import check_module_css + + (tmp_path / "styles.css").write_text("/* } { */\n@theme {\n --a: 1;\n}\n") + + diags = check_module_css(_mod(), tmp_path) + + assert [d.code for d in diags] == ["SM022"] + assert diags[0].file.endswith("styles.css:2") + + def test_import_statement_allowed_in_either_file(self, tmp_path): + """A statement at-rule ending in ; is not a rule block.""" + from simple_module_core.diagnostics._css import check_module_css + + (tmp_path / "theme.css").write_text('@import "other.css";\n') + (tmp_path / "styles.css").write_text('@charset "utf-8";\n') + + assert check_module_css(_mod(), tmp_path) == [] diff --git a/framework/hosting/simple_module_hosting/assets.py b/framework/hosting/simple_module_hosting/assets.py new file mode 100644 index 00000000..c2b9e50e --- /dev/null +++ b/framework/hosting/simple_module_hosting/assets.py @@ -0,0 +1,160 @@ +"""Per-module frontend asset discovery and CSS emission. + +Split out from :mod:`simple_module_hosting.manifest`, which is already at the +repo's 300-line cap. ``manifest.py`` imports from here. + +A module may ship two optional stylesheets beside its ``pages/`` directory: + +* ``theme.css`` — ``@theme`` tokens, ``@custom-variant``, ``@font-face``. + Imported *unlayered*, because a ``@theme`` block inside a cascade layer is + inert and registers no design tokens. +* ``styles.css`` — component rules, keyframes, vendor CSS. Imported into + ``layer(components)``, because unlayered CSS outranks every Tailwind + utility: a module shipping a bare ``.card { padding: 0 }`` would otherwise + silently beat ``p-4``. + +Both are referenced through a per-module Vite alias (``#module/``) rather +than a filesystem path, so the generated CSS never contains something like +``../../../.venv/lib/python3.12/site-packages//styles.css``. +""" + +from __future__ import annotations + +import importlib.resources +import json +import logging +from collections.abc import Callable, Sequence +from dataclasses import dataclass +from pathlib import Path + +from simple_module_core import ModuleBase, get_module_package_name + +logger = logging.getLogger(__name__) + +THEME_CSS = "theme.css" +STYLES_CSS = "styles.css" +ALIAS_PREFIX = "#module" + + +@dataclass(frozen=True) +class ModuleAssets: + """Frontend assets a single module contributes.""" + + name: str + package_name: str + package_dir: Path + pages_dir: Path | None + theme_css: Path | None + styles_css: Path | None + + +def compute_module_assets(modules: Sequence[ModuleBase]) -> list[ModuleAssets]: + """Return per-module frontend assets, preserving discovery order. + + Discovery order matters. ``discover_modules()`` topologically sorts by + ``ModuleMeta.depends_on``, which is the order the host invokes + ``register_*`` hooks in; emitting CSS in that same order means a module + that depends on another can override its dependency's styles. + + A module is included only if it contributes at least one asset. + """ + result: list[ModuleAssets] = [] + for mod in modules: + pkg_name = get_module_package_name(mod) + try: + pkg_root = Path(str(importlib.resources.files(pkg_name))) + except (ModuleNotFoundError, TypeError): + logger.debug( + "Module '%s': package %s not importable — skipping", mod.meta.name, pkg_name + ) + continue + pages_dir = pkg_root / "pages" + theme = pkg_root / THEME_CSS + styles = pkg_root / STYLES_CSS + entry = ModuleAssets( + name=mod.meta.name, + package_name=pkg_name, + package_dir=pkg_root.resolve(), + pages_dir=pages_dir.resolve() if pages_dir.is_dir() else None, + theme_css=theme.resolve() if theme.is_file() else None, + styles_css=styles.resolve() if styles.is_file() else None, + ) + if entry.pages_dir or entry.theme_css or entry.styles_css: + result.append(entry) + return result + + +_CSS_HEADER = """\ +/* AUTO-GENERATED by simple_module_hosting.assets — do not edit by hand. + * Regenerate with: smpy gen-pages + * + * Emission order is module discovery order (topological by depends_on), so a + * dependent module's CSS can override its dependency's. + * + * theme.css is imported unlayered so its @theme blocks register as design + * tokens; styles.css is imported into layer(components) so a module rule can + * never outrank a Tailwind utility. + */""" + + +def render_modules_css( + assets: Sequence[ModuleAssets], + *, + in_repo: Callable[[Path], bool], +) -> str: + """Render the contents of ``modules.generated.css``. + + ``@source`` is emitted only for wheel-installed modules — in-repo module + pages are already covered by the static ``@source`` glob in the host's + ``styles.css``, so emitting an absolute one too would just duplicate it. + ``@import`` is emitted for *every* module, in-repo and wheel alike, because + there is no static-glob equivalent for CSS. + """ + source_lines = [ + f'@source "{e.pages_dir.as_posix()}/**/*.{{ts,tsx}}";' + for e in assets + if e.pages_dir and not in_repo(e.pages_dir) + ] + theme_lines = [ + f'@import "{ALIAS_PREFIX}/{e.package_name}/{THEME_CSS}";' for e in assets if e.theme_css + ] + style_lines = [ + f'@import "{ALIAS_PREFIX}/{e.package_name}/{STYLES_CSS}" layer(components);' + for e in assets + if e.styles_css + ] + + out = [_CSS_HEADER] + for heading, lines in ( + ("/* ── @source: class scanning (wheel-installed modules) ── */", source_lines), + ("/* ── theme: unlayered, registers @theme tokens ── */", theme_lines), + ("/* ── styles: layer(components), always loses to utilities ── */", style_lines), + ): + if lines: + out.append("") + out.append(heading) + out.extend(lines) + out.append("") + return "\n".join(out) + + +def render_assets_json(assets: Sequence[ModuleAssets]) -> str: + """Render ``modules.assets.json`` — the richer companion to the pages manifest. + + Kept separate from ``modules.manifest.json`` rather than replacing it: + ``vite.config.ts`` is scaffolded *into* each app, so changing the existing + manifest's value shape would break every downstream app at once. This file + is purely additive, and also covers CSS-only modules that never appear in + the pages manifest because they ship no ``pages/``. + """ + payload = { + e.name: { + "package_name": e.package_name, + "package": e.package_dir.as_posix(), + "pages": e.pages_dir.as_posix() if e.pages_dir else None, + "theme": e.theme_css.as_posix() if e.theme_css else None, + "styles": e.styles_css.as_posix() if e.styles_css else None, + } + for e in assets + } + return json.dumps(payload, indent=2, sort_keys=True) + "\n" diff --git a/framework/hosting/simple_module_hosting/host_cli.py b/framework/hosting/simple_module_hosting/host_cli.py index 830c6b79..601ecd58 100644 --- a/framework/hosting/simple_module_hosting/host_cli.py +++ b/framework/hosting/simple_module_hosting/host_cli.py @@ -37,7 +37,7 @@ def gen_pages( ), ] = Path("client_app"), ) -> None: - """Regenerate client_app/modules.{manifest.json,generated.ts,generated.css}.""" + """Regenerate client_app/modules.{manifest.json,generated.ts,generated.css,assets.json}.""" logging.basicConfig(level=logging.INFO, format="%(levelname)s: %(message)s") if not host_dir.is_dir(): typer.echo(f"ERROR: client_app directory not found at {host_dir}", err=True) @@ -47,7 +47,7 @@ def gen_pages( typer.echo( f"Module pages manifest: {len(modules)} module(s) " f"→ {written['manifest'].name}, {written['generated'].name}, " - f"{written['css'].name} in {host_dir}" + f"{written['css'].name}, {written['assets'].name} in {host_dir}" ) diff --git a/framework/hosting/simple_module_hosting/manifest.py b/framework/hosting/simple_module_hosting/manifest.py index 14a221fb..5fb911a4 100644 --- a/framework/hosting/simple_module_hosting/manifest.py +++ b/framework/hosting/simple_module_hosting/manifest.py @@ -1,10 +1,13 @@ """Frontend pages manifest + per-module JS dependency discovery. -* :func:`write_module_pages_manifest` emits the three generated files Vite +* :func:`write_module_pages_manifest` emits the four generated files Vite and Tailwind read at build time: - ``modules.manifest.json`` — name -> absolute pages dir - ``modules.generated.ts`` — ``import.meta.glob`` per module - - ``modules.generated.css`` — ``@source`` per wheel-installed module + - ``modules.generated.css`` — ``@source`` per wheel-installed module, plus + an ``@import`` per module-shipped stylesheet + - ``modules.assets.json`` — the richer per-module asset record the host's + ``vite.config.ts`` builds its ``#module/`` aliases from * :func:`read_module_package_json` / :func:`collect_module_js_deps` locate the per-module ``package.json`` shipped inside wheels (or alongside the source for editable installs) and aggregate its ``dependencies`` block. @@ -22,6 +25,12 @@ from simple_module_core import ModuleBase, get_module_package_name +from simple_module_hosting.assets import ( + compute_module_assets, + render_assets_json, + render_modules_css, +) + logger = logging.getLogger(__name__) _GENERATED_TS_HEADER = """\ @@ -33,16 +42,6 @@ // pip-installed module wheels are picked up by Vite's import.meta.glob. """ -_GENERATED_CSS_HEADER = """\ -/* AUTO-GENERATED by simple_module_hosting.manifest — do not edit by hand. - * Regenerate with: smpy gen-pages - * - * @source entries for module pages shipped inside pip-installed wheels. - * In-repo modules are covered by the static @source glob in - * host/client_app/styles.css. - */ -""" - def repo_root_from_client_app(client_app_dir: Path) -> Path: """Locate the workspace/repo root that contains ``client_app_dir``. @@ -144,11 +143,16 @@ def write_module_pages_manifest( output_dir: Path, repo_root: Path | None = None, ) -> dict[str, Path]: - """Write the three generated files (manifest JSON, glob TS, Tailwind CSS). + """Write the four generated files (manifest JSON, glob TS, CSS, assets JSON). Returns the paths that were written. Content is written only when different from what's on disk, so booting the host repeatedly in dev does not wake up Vite's file watcher. + + ``modules.manifest.json`` deliberately keeps its ``{name: pages_dir}`` + shape: every downstream app holds its own copy of ``vite.config.ts`` and + reads it, so changing the value shape would break them all at once. The + new per-module asset record goes in ``modules.assets.json`` instead. """ output_dir = Path(output_dir) output_dir.mkdir(parents=True, exist_ok=True) @@ -180,32 +184,35 @@ def write_module_pages_manifest( lines.append("") generated_text = "\n".join(lines) - # In-repo modules are already covered by the static glob in styles.css; - # only emit @source for wheel-installed ones. pages_map is keyed by - # module name so each pages_dir is unique — no further dedup needed. + # Rendered from the richer asset record rather than pages_map, so that a + # module shipping CSS but no pages/ still contributes its stylesheets. + assets = compute_module_assets(modules) css_path = output_dir / "modules.generated.css" - css_lines: list[str] = [_GENERATED_CSS_HEADER] - for name in sorted(pages_map): - pages_dir = pages_map[name] - if _is_in_repo_module(pages_dir, repo_root): - continue - css_lines.append(f'@source "{pages_dir.as_posix()}";') - css_lines.append("") - css_text = "\n".join(css_lines) + css_text = render_modules_css(assets, in_repo=lambda p: _is_in_repo_module(p, repo_root)) + + assets_path = output_dir / "modules.assets.json" + assets_text = render_assets_json(assets) wrote_manifest = _write_if_changed(manifest_path, manifest_text) wrote_generated = _write_if_changed(generated_path, generated_text) wrote_css = _write_if_changed(css_path, css_text) + wrote_assets = _write_if_changed(assets_path, assets_text) - if wrote_manifest or wrote_generated or wrote_css: + if wrote_manifest or wrote_generated or wrote_css or wrote_assets: logger.info( - "Wrote module pages manifest: %d module(s) -> %s, %s, %s", + "Wrote module pages manifest: %d module(s) -> %s, %s, %s, %s", len(pages_map), manifest_path.name, generated_path.name, css_path.name, + assets_path.name, ) - return {"manifest": manifest_path, "generated": generated_path, "css": css_path} + return { + "manifest": manifest_path, + "generated": generated_path, + "css": css_path, + "assets": assets_path, + } def _glob_pattern_for(pages_dir: Path, output_dir: Path) -> str: diff --git a/host/client_app/styles.css b/host/client_app/styles.css index 02d06d8f..892140fc 100644 --- a/host/client_app/styles.css +++ b/host/client_app/styles.css @@ -1,7 +1,22 @@ @import "../../packages/ui/src/styles/globals.css"; @import "./modules.generated.css"; -/* App-level source scanning — host code + in-repo module pages. - * Wheel-installed modules are covered by modules.generated.css. */ +/* App-level source scanning — host code + in-repo modules. + * Wheel-installed modules are covered by modules.generated.css. + * + * The module glob deliberately covers the whole package, not just pages/: + * shared components, hooks and Puck blocks carry Tailwind classes too, and + * scanning pages/ alone meant such a class only reached the bundle when some + * other scanned file happened to use it as well. The {ts,tsx} filter is what + * keeps .py files out of the scan. */ @source "./**/*.{ts,tsx}"; -@source "../../modules/*/*/pages/**/*.{ts,tsx}"; +@source "../../modules/*/*/**/*.{ts,tsx}"; +/* The second `*` above matches any directory under modules//, not just + * the Python package — so a module's sibling tests/ would be scanned too and + * test-only classes would land in the production bundle. + * + * Consequence: a module's Python package must not itself be named `tests` + * (i.e. modules/tests/tests/), or this line would exclude the package too. No + * glob can tell a package directory from a sibling, and `tests` is not a + * viable package name anyway — it collides with pytest collection. */ +@source not "../../modules/*/tests/**"; diff --git a/host/client_app/vite.config.ts b/host/client_app/vite.config.ts index f26452c1..fff8658b 100644 --- a/host/client_app/vite.config.ts +++ b/host/client_app/vite.config.ts @@ -53,6 +53,37 @@ for (const pagesDir of Object.values(manifest)) { } } +// Per-module aliases, so generated CSS can say +// @import "#module/gis/styles.css" +// instead of a ../../../.venv/lib/python3.12/site-packages/gis/styles.css +// path that breaks the moment the interpreter version changes. +// +// `@tailwindcss/vite` builds its CSS import resolver with +// `createResolver({ ...config.resolve, ... })`, so `resolve.alias` governs +// CSS `@import` as well as JS — verified against @tailwindcss/vite 4.2.4. +// +// Read from modules.assets.json rather than modules.manifest.json: the +// manifest is keyed off `pages/`, so a module shipping only CSS never +// appears in it. +type ModuleAsset = { package_name: string; package: string }; +const moduleAliases: { find: string; replacement: string }[] = []; +const assetsPath = path.resolve(import.meta.dirname, 'modules.assets.json'); +let moduleAssets: Record = {}; +try { + moduleAssets = JSON.parse(fs.readFileSync(assetsPath, 'utf-8')); +} catch { + // Absent until `smpy gen-pages` runs — proceed with no aliases. +} +for (const entry of Object.values(moduleAssets)) { + moduleAliases.push({ find: `#module/${entry.package_name}`, replacement: entry.package }); + if (!moduleFsAllow.includes(entry.package)) moduleFsAllow.push(entry.package); +} +// Keep the alias list in a stable, longest-first order. Vite matches a string +// `find` on exact equality or a `/`-bounded prefix, so `#module/gis` could not +// swallow `#module/gis_extra` in any order — this is just determinism, not a +// correctness fix. +moduleAliases.sort((a, b) => b.find.length - a.find.length); + // Gather every bare specifier a module's pages might import. We include // both `dependencies` (deps the module ships its own copy of) and // `peerDependencies` (deps the host is expected to provide, e.g. @@ -161,6 +192,9 @@ export default defineConfig(({ command }) => ({ // feed Vite the per-module tsconfigs. resolve: { tsconfigPaths: true, + // `#module/` -> that module's package directory. Consumed by the + // @import lines in modules.generated.css. + alias: moduleAliases, // Force every importer (host, workspace module, wheel-installed module) // to resolve to one React copy — without it, plugin-react's Fast // Refresh preamble check fires in a realm where its global was never diff --git a/modules/dashboard/dashboard/styles.css b/modules/dashboard/dashboard/styles.css new file mode 100644 index 00000000..bbfc973a --- /dev/null +++ b/modules/dashboard/dashboard/styles.css @@ -0,0 +1,18 @@ +/* Dashboard module component styles. + * + * Picked up automatically by `smpy host gen-pages`, which emits + * @import "#module/dashboard/styles.css" layer(components); + * into host/client_app/modules.generated.css. Nothing needs to be added to + * the host's styles.css by hand. + * + * Rules here live in layer(components) and therefore always lose to a + * Tailwind utility, so a consuming app can still override any of this with + * a class on the element. Design tokens belong in theme.css instead — a + * @theme block is inert inside a cascade layer. + */ +@layer components { + .dashboard-stat-grid { + display: grid; + gap: var(--spacing, 0.25rem); + } +}