From 5492b0e0383e9a5d970363e0f0ffbfa94bb071e0 Mon Sep 17 00:00:00 2001 From: Anto Subash Date: Fri, 7 Aug 2026 23:24:28 +0200 Subject: [PATCH 1/3] fix(hosting): emit absolute paths for module CSS imports (#253) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `gen-pages` emitted `@import "#module//styles.css"` into `modules.generated.css`. That specifier only resolves if the host's `vite.config.ts` defines a matching `resolve.alias` — but that file is scaffold output: written into an app once, then owned and edited there. It is versioned independently of these Python packages, so a Python-only bump 0.0.26 -> 0.0.27 broke `vite build` in every app scaffolded earlier, with `Can't resolve '#module//styles.css'` naming a specifier that appears nowhere in the app's own sources. Emit absolute paths instead, exactly as the `@source` lines in the same file already did. The stated objection to filesystem paths (an ugly `../../../.venv/lib/python3.12/site-packages/...`) only ever applied to *relative* ones. `modules.generated.css` is now self-contained and needs no host cooperation at all. Verified on the scaffold's own stack (vite 6.4.3 + @tailwindcss/vite 4.3.3) that an absolute `@import` resolves with no alias configured, and that the alias form fails there with the reporter's exact error; and on the repo's stack (vite 8) for both `vite build` and the dev server, including a module directory entirely outside the project root. The `#module/` alias stays in the scaffold template — it costs nothing and removing it would be a second breaking change to a file apps own — but nothing generated depends on it any more. Also: - biome.json: exclude `modules.generated.css` / `modules.assets.json`, matching the existing exclusions for the other two generated files. Absolute paths are long, so whether a line exceeds biome's 100-char lineWidth depended on where the repo happened to live — `make lint` passed or failed by checkout path. - CHANGELOG: move the #152/#155/#156/#173 entries out of [Unreleased] into the releases that actually shipped them (verified with `git tag --contains`). Leaving them there is what led #253 to conclude the #156 fix was unreleased when it has shipped since v0.0.15. --- CHANGELOG.md | 80 +++++++++++++++---- biome.json | 4 +- docs/module-authoring.md | 34 +++++--- .../templates/host/client_app/vite.config.ts | 27 ++++--- framework/cli/tests/test_module_css_assets.py | 53 +++++++----- framework/cli/tests/test_module_css_build.py | 13 +-- .../hosting/simple_module_hosting/assets.py | 31 ++++--- host/client_app/vite.config.ts | 27 ++++--- modules/dashboard/dashboard/styles.css | 6 +- 9 files changed, 191 insertions(+), 84 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 4698ad7a..1f30dde8 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,13 @@ All notable changes to this project are documented in this file. The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.1.0/) and this project adheres to [Semantic Versioning](https://semver.org/) post-1.0. +> **Coverage note.** Between 0.0.1 and 0.0.27 this file carried no per-version +> sections, and several already-released fixes sat under `[Unreleased]` long +> after shipping — which led a downstream app to conclude a fix it depended on +> was still unreleased (GH issue #253). Those entries have been moved to the +> release they actually shipped in, verified with `git tag --contains`. +> Versions not listed below still have no entry; consult the commit log. + ## [Unreleased] ### Added @@ -21,13 +28,47 @@ All notable changes to this project are documented in this file. The format is b production-mode containers pass `UsersSettings` boot validation. ### Fixed -- Vite's dev-mode dependency pre-bundling now resolves cross-package bare - imports (e.g. `maplibre-gl`, `pmtiles`) from module pages whose importers sit - outside the host's `client_app/` — including wheel-installed modules and - workspace modules shipping their own JS deps. The scaffold template (Vite 6) - seeds `optimizeDeps.esbuildOptions.nodePaths` and the framework repo (Vite 8) - seeds `optimizeDeps.rolldownOptions.resolve.modules` with the workspace - `node_modules/` as a NODE_PATH-style fallback for the dep scanner (GH issue #152). +- `smpy gen-pages` now emits module stylesheet `@import` lines as **absolute + paths** instead of `#module/` alias specifiers. The alias only resolved + if the host's `vite.config.ts` defined a matching `resolve.alias` — but that + file is scaffold output, written into an app once and then owned and edited + there, so it is versioned independently of these Python packages. Upgrading + `simple_module_*` 0.0.26 → 0.0.27 therefore broke `vite build` in every app + scaffolded earlier, failing with `Can't resolve '#module//styles.css'` — + naming a specifier that appears nowhere in the app's own sources. + + `modules.generated.css` is now self-contained: it resolves under any + `vite.config.ts`, with no alias configured at all, exactly as the `@source` + lines in the same file already did. **No host action is required** — upgrade + and re-run `gen-pages`. The scaffold template still defines the + `#module/` alias for hand-written imports, but nothing generated depends + on it any more (GH issue #253). + +## [0.0.27] — 2026-08-06 + +### Known issue +- Fixed in the `[Unreleased]` entry above. `gen-pages` emitted + `@import "#module//…"` into `modules.generated.css`, which resolves only + in hosts scaffolded at 0.0.27 or later; apps scaffolded earlier fail + `vite build` after a Python-only upgrade. Either upgrade past this release, + or add the alias to `host/client_app/vite.config.ts` by hand — build + `{ find: '#module/' + package_name, replacement: package }` from each entry + in `client_app/modules.assets.json` and pass the list as `resolve.alias` + (GH issue #253). + +## [0.0.16] — 2026-05-25 + +### Fixed +- The `users` module's post-login redirect (`login_redirect_url`) no longer + hard-codes a `/` fallback when the Dashboard module isn't installed — `/` + 404s on apps without a root route (e.g. `smpy_gis`, `--preset minimal`). It + now redirects to the first sibling module that exposes view routes, falling + back to `/` only as an absolute last resort. Operator-set overrides are + always preserved (GH issue #173). + +## [0.0.15] — 2026-05-21 + +### Fixed - The `moduleBareImportResolver` Vite plugin no longer short-circuits on `fsRoot`/`projectRoot` containment, so workspace-member modules at `modules///pages/` get the same workspace-root re-resolution as @@ -35,12 +76,19 @@ All notable changes to this project are documented in this file. The format is b the resolver root, so the previous early-return excluded the very modules that need it. Cross-package bare imports (`maplibre-gl`, `pmtiles`, peer deps) now resolve in both wheel and workspace install modes (GH issue #156). -- The `users` module's post-login redirect (`login_redirect_url`) no longer - hard-codes a `/` fallback when the Dashboard module isn't installed — `/` - 404s on apps without a root route (e.g. `smpy_gis`, `--preset minimal`). It - now redirects to the first sibling module that exposes view routes, falling - back to `/` only as an absolute last resort. Operator-set overrides are - always preserved (GH issue #173). +- The framework repo (Vite 8) seeds + `optimizeDeps.rolldownOptions.resolve.modules` with the workspace + `node_modules/` as a NODE_PATH-style fallback for the dep scanner + (GH issue #155). + +## [0.0.13] — 2026-05-15 + +### Fixed +- Vite's dev-mode dependency pre-bundling now resolves cross-package bare + imports (e.g. `maplibre-gl`, `pmtiles`) from module pages whose importers sit + outside the host's `client_app/`. The scaffold template (Vite 6) seeds + `optimizeDeps.esbuildOptions.nodePaths` with the workspace `node_modules/` + as a NODE_PATH-style fallback for the dep scanner (GH issue #152). ## [0.0.1] — 2026-04-21 @@ -73,5 +121,9 @@ Initial public release. All 12 Python packages publish to PyPI and all 3 JS pack - PyPI Trusted Publishing workflow (`.github/workflows/release.yml`) for zero-secret releases. - npm Trusted Publishing for all three JS packages. -[Unreleased]: https://github.com/antosubash/simple_module_python/compare/v0.0.1...HEAD +[Unreleased]: https://github.com/antosubash/simple_module_python/compare/v0.0.27...HEAD +[0.0.27]: https://github.com/antosubash/simple_module_python/compare/v0.0.26...v0.0.27 +[0.0.16]: https://github.com/antosubash/simple_module_python/compare/v0.0.15...v0.0.16 +[0.0.15]: https://github.com/antosubash/simple_module_python/compare/v0.0.14...v0.0.15 +[0.0.13]: https://github.com/antosubash/simple_module_python/compare/v0.0.12...v0.0.13 [0.0.1]: https://github.com/antosubash/simple_module_python/releases/tag/v0.0.1 diff --git a/biome.json b/biome.json index 5f20637d..d503df20 100644 --- a/biome.json +++ b/biome.json @@ -7,7 +7,9 @@ "modules/*/*/pages/**", "modules/*/*/components/**", "!host/client_app/modules.generated.ts", - "!host/client_app/modules.manifest.json" + "!host/client_app/modules.manifest.json", + "!host/client_app/modules.generated.css", + "!host/client_app/modules.assets.json" ], "ignoreUnknown": true }, diff --git a/docs/module-authoring.md b/docs/module-authoring.md index 4301a1d6..85e15cd7 100644 --- a/docs/module-authoring.md +++ b/docs/module-authoring.md @@ -246,15 +246,16 @@ Modules may ship TSX pages in `my_module/pages/*.tsx`. On host boot (and on - `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.config.ts` builds its `server.fs.allow` entries (and the optional + `#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 production build alike. **Inertia pages never need pre-bundling.** The consuming host's Vite build -compiles `pages/*.tsx` straight out of the installed wheel (via the -`#module/` aliases + `server.fs.allow`). `static/dist/` + +compiles `pages/*.tsx` straight out of the installed wheel (via +`modules.generated.ts` + `server.fs.allow`). `static/dist/` + `static_mounts()` exist only for assets *outside* that pipeline — vendor JS, standalone widgets, images. Build them with `smpy module build` (see [Developing out-of-tree](#developing-out-of-tree)) and expose them via @@ -290,16 +291,25 @@ my_module/ `client_app/modules.generated.css`: ```css -@import "#module/my_module/theme.css"; -@import "#module/my_module/styles.css" layer(components); +@import "/abs/path/to/site-packages/my_module/theme.css"; +@import "/abs/path/to/site-packages/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. +**Nothing needs to be added to the host's `styles.css` by hand, and nothing +needs to be added to its `vite.config.ts` either.** The paths are absolute — +resolved through `importlib.resources`, exactly like the `@source` entries in +the same file — so they resolve identically whether the module is a workspace +member or installed from a wheel, under any host config. + +That last part is load-bearing. `modules.generated.css` is regenerated from +whatever Python packages are installed, but `vite.config.ts` is *scaffold +output*: written into an app once and then owned and edited there. The two are +versioned independently, so anything the generated file emits must resolve +without host cooperation. Emitting a `#module/` alias specifier instead +broke exactly this way — upgrading the Python packages alone made the build +fail on a specifier the app had never written ([#253]). + +[#253]: https://github.com/antosubash/simple_module_python/issues/253 Imports are emitted in module discovery order, which is topological by `ModuleMeta.depends_on`. A module that depends on another can therefore @@ -372,7 +382,7 @@ npm run typecheck # tsc --noEmit over your pages `tsc` alone cannot tell you whether your pages and `theme.css`/`styles.css` survive a real host build (Vite import resolution, Tailwind scanning, the -`#module/` alias plumbing). `verify` answers that by scaffolding a +`gen-pages` CSS emission). `verify` answers that by scaffolding a throwaway host into `.smpy/verify-host/` (cached, gitignored), installing your module into it as an editable path dependency, and running the host's real `gen-pages` + `npm run build`: 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 b1244148..f40416c5 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 @@ -69,18 +69,25 @@ 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. +// Two things come out of modules.assets.json. +// +// 1. `server.fs.allow` entries. The dev server must be allowed to read each +// module's package dir. Read from modules.assets.json rather than +// modules.manifest.json because the manifest is keyed off `pages/`, so a +// module shipping only CSS never appears in it. +// +// 2. A convenience `#module/` alias. This is NOT required by +// `modules.generated.css` — that file imports module stylesheets by +// absolute path, so it resolves with no alias configured at all. Emitting +// an alias there made a generated file depend on this hand-owned config, +// and since `vite.config.ts` is scaffolded once and then owned by the app, +// a Python-only version bump broke every host scaffolded earlier +// (GH issue #253). The alias stays because it costs nothing and lets a +// module's own CSS/TS reference a sibling module by package name. // // `@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'); @@ -231,8 +238,8 @@ 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. + // `#module/` -> that module's package directory. Optional sugar for + // hand-written imports; modules.generated.css does not rely on it. alias: moduleAliases, dedupe: [...REACT_CORE_DEPS, '@simple-module-py/ui', '@simple-module-py/i18n'], }, diff --git a/framework/cli/tests/test_module_css_assets.py b/framework/cli/tests/test_module_css_assets.py index c608dfd1..621da3e2 100644 --- a/framework/cli/tests/test_module_css_assets.py +++ b/framework/cli/tests/test_module_css_assets.py @@ -105,15 +105,13 @@ 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", - ) + theme = tmp_path / "gis" / "theme.css" + styles = tmp_path / "gis" / "styles.css" + entry = _assets(tmp_path, theme_css=theme, styles_css=styles) 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 + assert f'@import "{theme.as_posix()}";' in css + assert f'@import "{styles.as_posix()}" layer(components);' in css # Tokens must be declared before the rules that consume them. assert css.index("theme.css") < css.index("styles.css") @@ -121,17 +119,18 @@ 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 + styles = tmp_path / "local" / "styles.css" entry = _assets( tmp_path, name="Local", package_name="local", pages_dir=tmp_path / "local" / "pages", - styles_css=tmp_path / "local" / "styles.css", + styles_css=styles, ) 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 + assert f'@import "{styles.as_posix()}" layer(components);' in css async def test_source_emitted_for_wheel_modules(self, tmp_path): """A wheel-installed module gets an absolute @source glob.""" @@ -154,9 +153,11 @@ async def test_module_without_css_emits_no_import(self, tmp_path): 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`. + `modules.generated.css` is excluded from `biome ci .` (it is generated, + and its absolute paths make its formatting machine-dependent — whether + a line exceeds biome's 100-char `lineWidth` depends on where the repo + happens to live). Nothing enforces tidiness here but this test, and a + file humans read when debugging a missing style should stay readable. """ from simple_module_hosting.assets import render_modules_css @@ -171,18 +172,34 @@ async def test_output_is_formatter_clean(self, tmp_path): 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 ../..""" + async def test_generated_css_is_self_contained(self, tmp_path): + """Every emitted path resolves without any host `vite.config.ts` help. + + Regression guard for GH issue #253. `modules.generated.css` is + regenerated from the installed Python packages, but `vite.config.ts` is + scaffold output owned by the app — so anything here that needs a host + alias to resolve breaks every host scaffolded before that alias + existed, on a Python-only version bump. + + Absolute, on-disk paths are the invariant that makes the two files + independently versionable. + """ 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}" + imports = [ln for ln in css.splitlines() if ln.startswith("@import")] + assert imports, "expected at least one module stylesheet import" + for line in imports: + target = Path(line.split('"')[1]) + assert target.is_absolute(), f"@import must be absolute: {line}" + assert "../" not in line, f"@import must not use a relative path: {line}" + assert not line.split('"')[1].startswith("#"), ( + f"@import must not use an alias the host has to define: {line}" + ) + assert target.is_file(), f"@import points at a file that does not exist: {line}" class TestAssetsManifest: diff --git a/framework/cli/tests/test_module_css_build.py b/framework/cli/tests/test_module_css_build.py index da6c94f8..16a530ae 100644 --- a/framework/cli/tests/test_module_css_build.py +++ b/framework/cli/tests/test_module_css_build.py @@ -1,10 +1,10 @@ """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. +emits nothing at all — the generated `@import` only pays off if Tailwind +resolves the path and 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 @@ -36,7 +36,7 @@ def test_module_class_is_emitted_by_vite_build(self): `.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"`. + generated `@import` of that stylesheet. """ subprocess.run( [ @@ -57,7 +57,8 @@ def test_module_class_is_emitted_by_vite_build(self): generated = (REPO_ROOT / "host/client_app/modules.generated.css").read_text( encoding="utf-8" ) - assert '@import "#module/dashboard/styles.css" layer(components);' in generated, ( + dashboard_css = REPO_ROOT / "modules/dashboard/dashboard/styles.css" + assert f'@import "{dashboard_css.as_posix()}" layer(components);' in generated, ( f"gen-pages did not emit the dashboard stylesheet import:\n{generated}" ) diff --git a/framework/hosting/simple_module_hosting/assets.py b/framework/hosting/simple_module_hosting/assets.py index c2b9e50e..4db61f40 100644 --- a/framework/hosting/simple_module_hosting/assets.py +++ b/framework/hosting/simple_module_hosting/assets.py @@ -13,9 +13,22 @@ 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``. +Both are referenced by **absolute** filesystem path, exactly as the ``@source`` +lines in the same file already are. + +This used to emit a per-module Vite alias (``#module//styles.css``) to +avoid a relative path like ``../../../.venv/lib/python3.12/site-packages/`` +— but that objection only ever applied to *relative* paths, and the alias made +this generated file depend on the host resolving it. ``vite.config.ts`` is +scaffold output: it is written once into an app and then owned and edited +there, so it is versioned independently of these Python packages. A host +scaffolded before the alias existed never receives it, and a Python-only +dependency bump would emit specifiers that host cannot resolve, failing the +build with ``Can't resolve '#module//styles.css'`` — naming something that +appears nowhere in the app's own sources (GH issue #253). + +Absolute paths keep the generated CSS self-contained: it resolves under any +``vite.config.ts``, old or new, with no alias configured at all. """ from __future__ import annotations @@ -33,7 +46,6 @@ THEME_CSS = "theme.css" STYLES_CSS = "styles.css" -ALIAS_PREFIX = "#module" @dataclass(frozen=True) @@ -109,19 +121,18 @@ def render_modules_css( ``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. + + Every path is absolute, so nothing here depends on the host's + ``vite.config.ts`` — see the module docstring for why that matters. """ 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 - ] + theme_lines = [f'@import "{e.theme_css.as_posix()}";' 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 + f'@import "{e.styles_css.as_posix()}" layer(components);' for e in assets if e.styles_css ] out = [_CSS_HEADER] diff --git a/host/client_app/vite.config.ts b/host/client_app/vite.config.ts index fff8658b..fdd1a10c 100644 --- a/host/client_app/vite.config.ts +++ b/host/client_app/vite.config.ts @@ -53,18 +53,25 @@ 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. +// Two things come out of modules.assets.json. +// +// 1. `server.fs.allow` entries. The dev server must be allowed to read each +// module's package dir. Read from modules.assets.json rather than +// modules.manifest.json because the manifest is keyed off `pages/`, so a +// module shipping only CSS never appears in it. +// +// 2. A convenience `#module/` alias. This is NOT required by +// `modules.generated.css` — that file imports module stylesheets by +// absolute path, so it resolves with no alias configured at all. Emitting +// an alias there made a generated file depend on this hand-owned config, +// and since `vite.config.ts` is scaffolded once and then owned by the app, +// a Python-only version bump broke every host scaffolded earlier +// (GH issue #253). The alias stays because it costs nothing and lets a +// module's own CSS/TS reference a sibling module by package name. // // `@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'); @@ -192,8 +199,8 @@ 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. + // `#module/` -> that module's package directory. Optional sugar for + // hand-written imports; modules.generated.css does not rely on it. alias: moduleAliases, // Force every importer (host, workspace module, wheel-installed module) // to resolve to one React copy — without it, plugin-react's Fast diff --git a/modules/dashboard/dashboard/styles.css b/modules/dashboard/dashboard/styles.css index bbfc973a..035903b5 100644 --- a/modules/dashboard/dashboard/styles.css +++ b/modules/dashboard/dashboard/styles.css @@ -1,9 +1,9 @@ /* Dashboard module component styles. * - * Picked up automatically by `smpy host gen-pages`, which emits - * @import "#module/dashboard/styles.css" layer(components); + * Picked up automatically by `smpy host gen-pages`, which emits an + * @import "" layer(components); * into host/client_app/modules.generated.css. Nothing needs to be added to - * the host's styles.css by hand. + * the host's styles.css — or its vite.config.ts — 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 From a41167afac49971ada170eabf3820166635a4df4 Mon Sep 17 00:00:00 2001 From: Anto Subash Date: Fri, 7 Aug 2026 23:43:33 +0200 Subject: [PATCH 2/3] feat(hosting): let a module import a sibling's TS by npm package name (#253) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Nothing in Node's own resolution made this work. A wheel-installed module is not an npm workspace member, so it never lands in node_modules at all; a workspace member *is* symlinked, but onto the source-tree module root — one level above the Python package — so subpaths landed somewhere that does not exist. Either way `@simple-module-py/foo/components/x` failed to resolve. `gen-pages` now records each module's `npm_name` in modules.assets.json and the host aliases it onto the module's **Python package directory**. That anchor is forced, not chosen. A wheel ships `site-packages//**` and nothing above it — Hatch force-includes the module-root package.json *into* the package — so the source-tree module root does not survive installation, and the package dir is the only anchor both layouts share. Verified by inspecting a real built wheel. The practical consequence is that the subpath is relative to the package: @simple-module-py/foo/components/Widget ok in both layouts @simple-module-py/foo/foo/components/Widget workspace-only The second shape is what npm's own symlink gives you for a workspace member, which is why hand-rolled aliases against the module root appear to work in a checkout and break once the module is wheel-installed. npm_name discovery skips a parent package.json unless that directory also holds a pyproject.toml, so a wheel-installed module cannot pick up a stray site-packages/package.json and alias itself onto a stranger. Sibling module names are also excluded from optimizeDeps: they resolve to source directories, not to pre-bundlable packages. Verified end-to-end in a real `smpy new` app on the template's vite 6 stack: a workspace module importing a wheel-installed module by package name — the exact shape from the issue — builds and pulls the sibling's component into the chunk. Also verified the new template degrades gracefully against 0.0.27 packages (no npm_name -> no alias, no crash). Structural changes to stay under the 300-line cap, per CLAUDE.md's "split by responsibility": - host/client_app/module-assets.ts — module discovery lifted out of vite.config.ts (which was already at 291 lines), mirroring the existing compress-assets.ts split. - framework/cli/tests/test_module_npm_aliases.py — npm identity tests, with the importable-module factory moved to a tests/conftest.py fixture so both files share it without a cross-file import. Tests: a real-build guard proving a cross-module import resolves (checked that it fails when the alias is removed, so it is not vacuous), plus unit coverage for both install layouts, the pyproject guard, malformed package.json, and npm-name uniqueness. --- CHANGELOG.md | 35 +++++ docs/module-authoring.md | 47 +++++++ .../templates/host/client_app/vite.config.ts | 24 +++- framework/cli/tests/conftest.py | 45 ++++++ framework/cli/tests/test_module_css_assets.py | 48 ++----- framework/cli/tests/test_module_css_build.py | 75 +++++++++- .../cli/tests/test_module_npm_aliases.py | 128 ++++++++++++++++++ .../hosting/simple_module_hosting/assets.py | 43 ++++++ host/client_app/module-assets.ts | 113 ++++++++++++++++ host/client_app/vite.config.ts | 97 +++---------- 10 files changed, 537 insertions(+), 118 deletions(-) create mode 100644 framework/cli/tests/conftest.py create mode 100644 framework/cli/tests/test_module_npm_aliases.py create mode 100644 host/client_app/module-assets.ts diff --git a/CHANGELOG.md b/CHANGELOG.md index 1f30dde8..215a7f0b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -44,6 +44,41 @@ All notable changes to this project are documented in this file. The format is b `#module/` alias for hand-written imports, but nothing generated depends on it any more (GH issue #253). +### Added +- A module can now import another module's TS/TSX by npm package name: + `import x from '@simple-module-py/pagebuilder/components/blockRegistry'`. + Nothing in Node's own resolution made this work — a wheel-installed module is + not an npm workspace member, so it never lands in `node_modules` at all; + a workspace member *is* symlinked, but onto the source-tree module root, one + level above the Python package, so subpaths landed somewhere nonexistent. + + `gen-pages` now records each module's `npm_name` in `modules.assets.json`, + and the host aliases it onto the module's **Python package directory**. That + anchor is forced, not chosen: a wheel ships `site-packages//**` and + nothing above it, so the module root does not survive installation and the + package directory is the only anchor both layouts share. The practical + consequence is that the subpath is relative to the *package*: + + ```tsx + import x from '@simple-module-py/foo/components/Widget'; // ✅ both layouts + import x from '@simple-module-py/foo/foo/components/Widget'; // ❌ workspace-only + ``` + + If you previously hand-rolled this alias against the module root, drop the + duplicated path segment. **Existing hosts need a `vite.config.ts` change** — + unlike the CSS fix above, the import lives in module source rather than a + generated file, so it cannot be made self-contained. In the loop over + `modules.assets.json` entries, add: + + ```ts + if (entry.npm_name) { + moduleAliases.push({ find: entry.npm_name, replacement: entry.package }); + } + ``` + + and skip those names when collecting `optimizeDeps.include` — they resolve to + source directories, not to pre-bundlable packages (GH issue #253). + ## [0.0.27] — 2026-08-06 ### Known issue diff --git a/docs/module-authoring.md b/docs/module-authoring.md index 85e15cd7..4dcc5ba3 100644 --- a/docs/module-authoring.md +++ b/docs/module-authoring.md @@ -273,6 +273,53 @@ class MyModule(ModuleBase): The host mounts each entry as `StaticFiles` during boot. +### Importing another module's TS/TSX + +Use the sibling's **npm package name** — the `name` in its `package.json`: + +```tsx +import { BlockRegistry } from '@simple-module-py/pagebuilder/components/blockRegistry'; +``` + +The host builds that alias from `modules.assets.json` and points it at the +sibling's **Python package directory** (`my_module/`). So everything after the +package name is a path *inside* that package — `components/…`, `pages/…`, +mirroring the layout in [Anatomy of a module package](#anatomy-of-a-module-package). + +That anchor is forced rather than chosen, and it is worth understanding because +the obvious alternative silently half-works: + +| | wheel install | workspace / editable | +|---|---|---| +| Python package | `site-packages/foo/` | `modules/foo/foo/` | +| `package.json` | `site-packages/foo/package.json` | `modules/foo/package.json` | +| module root | **does not exist** | `modules/foo/` | + +A wheel ships `site-packages/foo/**` and nothing above it — Hatch force-includes +the module-root `package.json` *into* the package. The source-tree module root +therefore does not survive installation, and the Python package directory is the +only anchor both layouts share. + +This means the shape npm gives you for a workspace member is the wrong one: + +```tsx +// ✅ same file in both layouts +import x from '@simple-module-py/foo/components/Widget'; + +// ❌ workspace-only. npm symlinks @simple-module-py/foo -> modules/foo/, so +// this happens to resolve in a checkout and breaks once foo is wheel-installed. +import x from '@simple-module-py/foo/foo/components/Widget'; +``` + +Declare the sibling in your `package.json` `peerDependencies` so the dependency +is explicit. Nothing pre-bundles it — it resolves to source, not to a +node_modules package. + +Note this is the one part of module frontend wiring that *does* depend on the +host's `vite.config.ts`, because the import lives in your source rather than in +a generated file. Apps scaffolded before this shipped need the alias block added +— see the CHANGELOG entry for the diff. + ## Styling A module may ship two optional stylesheets beside its `pages/` directory. 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 f40416c5..d3bcb058 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 @@ -69,7 +69,7 @@ if (fs.existsSync(manifestPath)) { } } -// Two things come out of modules.assets.json. +// Three things come out of modules.assets.json. // // 1. `server.fs.allow` entries. The dev server must be allowed to read each // module's package dir. Read from modules.assets.json rather than @@ -82,14 +82,22 @@ if (fs.existsSync(manifestPath)) { // an alias there made a generated file depend on this hand-owned config, // and since `vite.config.ts` is scaffolded once and then owned by the app, // a Python-only version bump broke every host scaffolded earlier -// (GH issue #253). The alias stays because it costs nothing and lets a -// module's own CSS/TS reference a sibling module by package name. +// (GH issue #253). The alias stays because it costs nothing. +// +// 3. An `` alias per module, so one module can import another's +// TS/TSX by package name. Aimed at the module's *Python package* dir — +// a wheel ships `site-packages/foo/**` and nothing above it, so the +// source-tree module root is not a target both layouts have. Needed in +// both: a wheel module is never in node_modules, and npm symlinks a +// workspace member onto the module root, one level too high. +// See docs/module-authoring.md § Importing another module's TS/TSX. // // `@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. -type ModuleAsset = { package_name: string; package: string }; +type ModuleAsset = { package_name: string; package: string; npm_name?: string | null }; const moduleAliases: { find: string; replacement: string }[] = []; +const moduleNpmNames = new Set(); const assetsPath = path.resolve(__dirname, 'modules.assets.json'); let moduleAssets: Record = {}; try { @@ -99,6 +107,10 @@ try { } for (const entry of Object.values(moduleAssets)) { moduleAliases.push({ find: `#module/${entry.package_name}`, replacement: entry.package }); + if (entry.npm_name) { + moduleAliases.push({ find: entry.npm_name, replacement: entry.package }); + moduleNpmNames.add(entry.npm_name); + } if (!moduleFsAllow.includes(entry.package)) moduleFsAllow.push(entry.package); } // Keep the alias list in a stable, longest-first order. Vite matches a string @@ -176,6 +188,10 @@ function collectOptimizeIncludes(): string[] { for (const block of [pkg.dependencies, pkg.peerDependencies]) { for (const name of Object.keys(block ?? {})) { if (name.startsWith('@types/')) continue; + // A sibling module declared as a dep/peer dep is not a node_modules + // package — it is aliased to a source directory above. Pre-bundling + // it would point the optimizer at raw .tsx with no entry point. + if (moduleNpmNames.has(name)) continue; const nested = findPackageJSON(name); if (!nested) continue; const nestedPkg = readPackageJSON(nested); diff --git a/framework/cli/tests/conftest.py b/framework/cli/tests/conftest.py new file mode 100644 index 00000000..171cd75d --- /dev/null +++ b/framework/cli/tests/conftest.py @@ -0,0 +1,45 @@ +"""Fixtures shared by the module-asset tests in this directory. + +These test files have no ``__init__.py`` (test basenames are globally unique +instead), so a plain helper import across files is not available — a fixture is +how the factory below gets shared. +""" + +from __future__ import annotations + +import sys +from pathlib import Path + +import pytest + + +@pytest.fixture +def make_importable_module(tmp_path: Path): + """Return a factory creating a real importable package bound to a ModuleBase. + + ``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. + + Returns ``(module_instance, package_dir)``. + """ + from simple_module_core import ModuleBase, ModuleMeta + + def _make(pkg_name: str, klass_name: str, *, root: Path | None = None): + base = root or tmp_path + pkg = base / pkg_name + pkg.mkdir(parents=True, exist_ok=True) + (pkg / "__init__.py").write_text("", encoding="utf-8") + if str(base) not in sys.path: + sys.path.insert(0, str(base)) + sys.modules.pop(pkg_name, None) + + klass = type( + klass_name, + (ModuleBase,), + {"meta": ModuleMeta(name=klass_name), "__module__": f"{pkg_name}.module"}, + ) + return klass(), pkg + + return _make diff --git a/framework/cli/tests/test_module_css_assets.py b/framework/cli/tests/test_module_css_assets.py index 621da3e2..3a07aa27 100644 --- a/framework/cli/tests/test_module_css_assets.py +++ b/framework/cli/tests/test_module_css_assets.py @@ -2,41 +2,15 @@ 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): + async def test_detects_theme_and_styles(self, make_importable_module): """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") + mod, pkg = make_importable_module("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") @@ -50,11 +24,11 @@ async def test_detects_theme_and_styles(self, tmp_path): 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): + async def test_css_only_module_is_included(self, make_importable_module): """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") + mod, pkg = make_importable_module("cssonly_mod", "CssOnly") (pkg / "styles.css").write_text(".y { color: blue; }\n", encoding="utf-8") result = compute_module_assets([mod]) @@ -64,11 +38,11 @@ async def test_css_only_module_is_included(self, tmp_path): 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): + async def test_module_with_no_assets_is_omitted(self, make_importable_module): """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") + mod, _pkg = make_importable_module("headless_mod", "Headless") assert compute_module_assets([mod]) == [] @@ -220,7 +194,15 @@ async def test_writes_assets_json(self, tmp_path): entry = data["Dashboard"] assert entry["package_name"] == "dashboard" assert entry["package"].endswith("dashboard") - assert set(entry) == {"package_name", "package", "pages", "theme", "styles"} + assert entry["npm_name"] == "@simple-module-py/dashboard" + assert set(entry) == { + "package_name", + "package", + "pages", + "theme", + "styles", + "npm_name", + } async def test_manifest_json_shape_unchanged(self, tmp_path): """modules.manifest.json stays {name: pages_dir} for downstream vite configs. diff --git a/framework/cli/tests/test_module_css_build.py b/framework/cli/tests/test_module_css_build.py index 16a530ae..197d79dc 100644 --- a/framework/cli/tests/test_module_css_build.py +++ b/framework/cli/tests/test_module_css_build.py @@ -68,6 +68,77 @@ def test_module_class_is_emitted_by_vite_build(self): 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" + "module-shipped CSS did not reach the bundle — the generated " + "@import most likely failed to resolve" + ) + + +# Written into a real module's pages/ so the host build treats it exactly like +# any other module page. Removed again in the fixture's teardown. +PROBE_MODULE_PAGES = REPO_ROOT / "modules/audit_log/audit_log/pages" +PROBE_PAGE = PROBE_MODULE_PAGES / "CrossModuleImportProbe.tsx" +PROBE_SOURCE = """\ +// Generated by test_module_css_build.py — safe to delete. +import { DemoPlaceholders } from '@simple-module-py/dashboard/pages/components/DemoPlaceholders'; + +export default function CrossModuleImportProbe() { + return ; +} +""" + + +@pytest.fixture +def cross_module_probe_page(): + """Add a page importing a sibling module by npm package name, then remove it.""" + PROBE_PAGE.write_text(PROBE_SOURCE, encoding="utf-8") + try: + yield PROBE_PAGE + finally: + PROBE_PAGE.unlink(missing_ok=True) + + +class TestCrossModuleImportResolves: + def test_module_can_import_a_sibling_by_npm_package_name(self, cross_module_probe_page): + """`@simple-module-py//…` resolves to the sibling's package dir. + + Nothing in Node's own resolution makes this work. A wheel-installed + module is not an npm workspace member at all, so it never lands in + node_modules; a workspace member *is* symlinked, but onto the + source-tree module root — one level above the Python package — so the + subpath lands somewhere that does not exist. Both cases are covered by + the `npm_name` aliases the host builds from modules.assets.json. + + This can only be caught by a real build: every unit test around + `npm_name` stays green while the alias itself is missing or misaimed. + """ + subprocess.run( + [ + "uv", + "run", + "--project", + "host", + "smpy", + "host", + "gen-pages", + "--host-dir=host/client_app", + ], + cwd=REPO_ROOT, + check=True, + capture_output=True, + ) + + result = subprocess.run( + ["npm", "run", "build"], cwd=REPO_ROOT, capture_output=True, text=True + ) + assert result.returncode == 0, ( + "build failed — a module could not import a sibling by npm package " + f"name:\n{result.stderr[-2000:]}" + ) + + chunks = list((REPO_ROOT / "host/static/dist/assets").glob("CrossModuleImportProbe*.js")) + assert chunks, "probe page produced no chunk" + # Resolution really happened if the sibling's component was pulled in + # rather than the import being dropped or stubbed out. + assert any("DemoPlaceholders" in c.read_text(encoding="utf-8") for c in chunks), ( + "probe chunk does not reference the sibling module's component" ) diff --git a/framework/cli/tests/test_module_npm_aliases.py b/framework/cli/tests/test_module_npm_aliases.py new file mode 100644 index 00000000..74951dbe --- /dev/null +++ b/framework/cli/tests/test_module_npm_aliases.py @@ -0,0 +1,128 @@ +"""Tests for a module's npm identity — the `npm_name` in `modules.assets.json`. + +That field is what lets one module import another's TS/TSX by package name. +The host aliases it onto the module's *Python package directory*, and the +tests here pin both halves: that the name is discovered in either install +layout, and that it is aimed at the one directory both layouts share. + +The real end-to-end proof lives in `test_module_css_build.py` — a bundler that +never resolves the alias would leave every assertion here green. +""" + +from __future__ import annotations + +import json +from pathlib import Path + + +class TestNpmNameDiscovery: + async def test_read_from_wheel_layout(self, make_importable_module): + """A wheel embeds package.json *inside* the Python package. + + Hatch force-includes the module-root `package.json` as + `/package.json`, so that is where an installed module carries its + npm identity. + """ + from simple_module_hosting.assets import compute_module_assets + + mod, pkg = make_importable_module("wheelish_mod", "Wheelish") + (pkg / "pages").mkdir() + (pkg / "package.json").write_text( + json.dumps({"name": "@simple-module-py/wheelish"}), encoding="utf-8" + ) + + assert compute_module_assets([mod])[0].npm_name == "@simple-module-py/wheelish" + + async def test_read_from_workspace_layout(self, make_importable_module): + """An editable install leaves package.json at the source-tree module root.""" + from simple_module_hosting.assets import compute_module_assets + + mod, pkg = make_importable_module("srcish_mod", "Srcish") + (pkg / "pages").mkdir() + (pkg.parent / "pyproject.toml").write_text("[project]\n", encoding="utf-8") + (pkg.parent / "package.json").write_text( + json.dumps({"name": "@simple-module-py/srcish"}), encoding="utf-8" + ) + + assert compute_module_assets([mod])[0].npm_name == "@simple-module-py/srcish" + + async def test_parent_package_json_ignored_without_pyproject(self, make_importable_module): + """No `pyproject.toml` beside it means that directory is not a module root. + + Without this guard a wheel-installed module would read whatever + `package.json` happened to sit in `site-packages/` and alias itself + onto a stranger's name. + """ + from simple_module_hosting.assets import compute_module_assets + + mod, pkg = make_importable_module("stray_mod", "Stray") + (pkg / "pages").mkdir() + (pkg.parent / "package.json").write_text( + json.dumps({"name": "totally-unrelated"}), encoding="utf-8" + ) + + assert compute_module_assets([mod])[0].npm_name is None + + async def test_module_without_package_json(self, make_importable_module): + """A Python-only module contributes no npm identity — and must not crash.""" + from simple_module_hosting.assets import compute_module_assets + + mod, pkg = make_importable_module("nojs_mod", "NoJs") + (pkg / "styles.css").write_text(".z { color: red; }\n", encoding="utf-8") + + assert compute_module_assets([mod])[0].npm_name is None + + async def test_malformed_package_json_is_ignored(self, make_importable_module): + """A broken package.json degrades to "no npm name", never to a boot failure.""" + from simple_module_hosting.assets import compute_module_assets + + mod, pkg = make_importable_module("broken_mod", "Broken") + (pkg / "pages").mkdir() + (pkg / "package.json").write_text("{not json", encoding="utf-8") + + assert compute_module_assets([mod])[0].npm_name is None + + +class TestNpmAliasContract: + async def test_alias_target_is_the_python_package_dir(self, tmp_path): + """`npm_name` must alias onto `package`, not the source-tree module root. + + This is the invariant that makes a cross-module import mean the same + thing in both install layouts. A wheel ships `site-packages//**` + and nothing above it — the module root does not survive installation — + so the Python package directory is the only anchor both layouts share. + + Concretely: `@simple-module-py/dashboard/pages/Home` has to land on + `/pages/Home`, which exists in a wheel and in a checkout. + """ + 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.assets.json").read_text(encoding="utf-8")) + + aliased = {k: v for k, v in data.items() if v["npm_name"]} + assert aliased, "expected at least one module shipping a package.json" + for name, entry in aliased.items(): + package = Path(entry["package"]) + assert package.is_dir(), f"{name}: alias target is not a directory" + assert package.name == entry["package_name"], ( + f"{name}: npm name must alias onto the Python package dir " + f"({entry['package_name']}), not {package.name}" + ) + if entry["pages"]: + assert Path(entry["pages"]).parent == package, ( + f"{name}: pages/ must sit directly under the alias target, " + "or subpath imports resolve differently per install layout" + ) + + async def test_npm_names_are_unique(self, tmp_path): + """Two modules claiming one npm name would make the alias order-dependent.""" + 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.assets.json").read_text(encoding="utf-8")) + + names = [v["npm_name"] for v in data.values() if v["npm_name"]] + assert len(names) == len(set(names)), f"duplicate npm package names: {names}" diff --git a/framework/hosting/simple_module_hosting/assets.py b/framework/hosting/simple_module_hosting/assets.py index 4db61f40..2e57745d 100644 --- a/framework/hosting/simple_module_hosting/assets.py +++ b/framework/hosting/simple_module_hosting/assets.py @@ -46,6 +46,7 @@ THEME_CSS = "theme.css" STYLES_CSS = "styles.css" +PACKAGE_JSON = "package.json" @dataclass(frozen=True) @@ -58,6 +59,39 @@ class ModuleAssets: pages_dir: Path | None theme_css: Path | None styles_css: Path | None + npm_name: str | None = None + + +def find_npm_name(pkg_root: Path) -> str | None: + """Return the module's npm package name, or ``None`` if it ships no JS. + + Two install layouts put ``package.json`` in different places: + + * **wheel** — Hatch force-includes the module-root ``package.json`` *into* + the Python package, so it lands at ``site-packages//package.json``. + * **editable / workspace** — it stays at the source-tree module root, + ``modules//package.json``, one level above the Python package. + + The parent candidate is accepted only when that directory also holds a + ``pyproject.toml``. Without that guard a wheel-installed module would + happily read ``site-packages/package.json`` — some unrelated file that + happens to sit there — and alias the module onto a stranger's name. + """ + candidates = [pkg_root / PACKAGE_JSON] + parent = pkg_root.parent + if (parent / "pyproject.toml").is_file(): + candidates.append(parent / PACKAGE_JSON) + for candidate in candidates: + if not candidate.is_file(): + continue + try: + name = json.loads(candidate.read_text(encoding="utf-8")).get("name") + except (OSError, ValueError): + logger.debug("Module package.json at %s is unreadable — ignoring", candidate) + continue + if isinstance(name, str) and name: + return name + return None def compute_module_assets(modules: Sequence[ModuleBase]) -> list[ModuleAssets]: @@ -90,6 +124,7 @@ def compute_module_assets(modules: Sequence[ModuleBase]) -> list[ModuleAssets]: 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, + npm_name=find_npm_name(pkg_root), ) if entry.pages_dir or entry.theme_css or entry.styles_css: result.append(entry) @@ -157,6 +192,13 @@ def render_assets_json(assets: Sequence[ModuleAssets]) -> str: 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/``. + + ``npm_name`` is what lets one module import another's TS/TSX by package + name. The host aliases it onto ``package`` — the module's *Python package + directory* — and that target is forced, not chosen: a wheel contains only + ``site-packages//**``, so the source-tree module root simply does not + exist once installed. Anchoring the npm name there is the only mapping + that can mean the same thing in both layouts. See ``find_npm_name``. """ payload = { e.name: { @@ -165,6 +207,7 @@ def render_assets_json(assets: Sequence[ModuleAssets]) -> str: "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, + "npm_name": e.npm_name, } for e in assets } diff --git a/host/client_app/module-assets.ts b/host/client_app/module-assets.ts new file mode 100644 index 00000000..fc652a26 --- /dev/null +++ b/host/client_app/module-assets.ts @@ -0,0 +1,113 @@ +// What the installed Python modules contribute to the frontend build. +// +// Split out of vite.config.ts, which is already at the repo's 300-line cap. +// Everything here is derived from the two files `smpy gen-pages` writes: +// `modules.manifest.json` (name -> absolute pages/ dir) and the richer +// `modules.assets.json`. Both are absent until gen-pages has run, which is a +// normal state — a fresh checkout resolves to an empty index rather than an +// error. +import fs from 'node:fs'; +import path from 'node:path'; + +type ModuleAsset = { package_name: string; package: string; npm_name?: string | null }; + +export type Alias = { find: string; replacement: string }; + +export type ModuleAssetIndex = { + /** Package dirs Vite must be allowed to read outside the workspace root. */ + fsAllow: string[]; + /** Glob per module pages/ dir, for optimizeDeps.entries. */ + optimizeEntries: string[]; + /** Each module's package.json — its deps declare what its pages may import. */ + pkgJsonPaths: string[]; + /** `` prefixes, for cheaply testing "is this importer a module page?". */ + pagesPrefixes: string[]; + /** `#module/` and `` aliases. */ + aliases: Alias[]; + /** npm names owned by modules — these resolve to source, so never pre-bundle them. */ + npmNames: Set; +}; + +function readJson(file: string, fallback: T): T { + try { + return JSON.parse(fs.readFileSync(file, 'utf-8')) as T; + } catch { + return fallback; + } +} + +export function loadModuleAssets(clientAppDir: string): ModuleAssetIndex { + const fsAllow: string[] = []; + const optimizeEntries: string[] = []; + const pkgJsonPaths: string[] = []; + const pagesPrefixes: string[] = []; + const aliases: Alias[] = []; + const npmNames = new Set(); + + // Each manifest entry points at an absolute pages/ dir — typically inside a + // pip-installed wheel under .venv/.../site-packages/. Two install modes put + // package.json in different places: wheels embed it next to the Python + // package (one level up from pages/, force-included by Hatch), while + // editable/workspace installs leave it at the source-tree module root (two + // levels up). We accept either. + const manifest = readJson>( + path.resolve(clientAppDir, 'modules.manifest.json'), + {}, + ); + for (const pagesDir of Object.values(manifest)) { + const pkgDir = path.dirname(pagesDir); + fsAllow.push(pkgDir); + optimizeEntries.push(path.join(pagesDir, '**/*.tsx')); + pagesPrefixes.push(pagesDir + path.sep); + for (const candidate of [ + path.join(pkgDir, 'package.json'), + path.join(path.dirname(pkgDir), 'package.json'), + ]) { + if (fs.existsSync(candidate)) { + pkgJsonPaths.push(candidate); + break; + } + } + } + + // modules.assets.json rather than the manifest: the manifest is keyed off + // `pages/`, so a module shipping only CSS never appears in it. + // + // The `#module/` alias is convenience only — `modules.generated.css` + // imports module stylesheets by absolute path and resolves with no alias + // configured at all. Emitting an alias there made a generated file depend on + // this hand-owned config, and since vite.config.ts is scaffolded once and + // then owned by the app, a Python-only version bump broke every host + // scaffolded earlier (GH issue #253). + // + // The `` alias is load-bearing: it is what lets one module import + // another's TS/TSX by package name. It aims at the module's *Python package* + // dir, because a wheel ships `site-packages/foo/**` and nothing above it — + // the source-tree module root is not a target both layouts have. Both + // layouts need it: a wheel module is never in node_modules, and npm symlinks + // a workspace member onto the module root, one level too high. + // See docs/module-authoring.md § Importing another module's TS/TSX. + // + // Both kinds work in CSS as well as JS: `@tailwindcss/vite` builds its CSS + // import resolver with `createResolver({ ...config.resolve, ... })`, so + // `resolve.alias` governs `@import` too — verified against 4.2.4. + const assets = readJson>( + path.resolve(clientAppDir, 'modules.assets.json'), + {}, + ); + for (const entry of Object.values(assets)) { + aliases.push({ find: `#module/${entry.package_name}`, replacement: entry.package }); + if (entry.npm_name) { + aliases.push({ find: entry.npm_name, replacement: entry.package }); + npmNames.add(entry.npm_name); + } + if (!fsAllow.includes(entry.package)) fsAllow.push(entry.package); + } + + // Stable, longest-first. 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 determinism, not a correctness fix. + aliases.sort((a, b) => b.find.length - a.find.length); + + return { fsAllow, optimizeEntries, pkgJsonPaths, pagesPrefixes, aliases, npmNames }; +} diff --git a/host/client_app/vite.config.ts b/host/client_app/vite.config.ts index fdd1a10c..b122499f 100644 --- a/host/client_app/vite.config.ts +++ b/host/client_app/vite.config.ts @@ -5,6 +5,7 @@ import react from '@vitejs/plugin-react'; import { visualizer } from 'rollup-plugin-visualizer'; import { defineConfig, type Plugin } from 'vite'; import { compressAssets } from './compress-assets.ts'; +import { loadModuleAssets } from './module-assets.ts'; const projectRoot = path.resolve(import.meta.dirname, '../..'); @@ -12,84 +13,17 @@ const projectRoot = path.resolve(import.meta.dirname, '../..'); // every chunk and its constituent modules. Open it to chase bundle bloat. const analyzeBundle = process.env.ANALYZE === '1'; -// Load the module pages manifest written by the Python host at boot. -// Each entry points at an absolute pages/ directory — typically inside a -// pip-installed module wheel under .venv/.../site-packages/. From each -// pages dir we derive three things: -// 1. The parent directory, for server.fs.allow (so Vite can serve the -// files from outside the workspace root). -// 2. The module's package.json. Two install modes ship it in different -// places: wheels embed it next to the Python package (one level up -// from pages/, force-included by Hatch), while editable/workspace -// installs leave it at the source-tree module root (two levels up). -// We accept either. Its `dependencies` + `peerDependencies` declare -// every bare specifier the module's pages can import. -// 3. A glob pattern for optimizeDeps.entries, so Vite's dependency -// scanner walks the pages and discovers their imports. -const manifestPath = path.resolve(import.meta.dirname, 'modules.manifest.json'); -const moduleFsAllow: string[] = []; -const moduleOptimizeEntries: string[] = []; -const modulePkgJsonPaths: string[] = []; -const modulePagesPrefixes: string[] = []; -let manifest: Record = {}; -try { - manifest = JSON.parse(fs.readFileSync(manifestPath, 'utf-8')); -} catch { - // Manifest absent (smpy gen-pages hasn't run yet) — proceed with empty set. -} -for (const pagesDir of Object.values(manifest)) { - const pkgDir = path.dirname(pagesDir); - moduleFsAllow.push(pkgDir); - moduleOptimizeEntries.push(path.join(pagesDir, '**/*.tsx')); - modulePagesPrefixes.push(pagesDir + path.sep); - for (const candidate of [ - path.join(pkgDir, 'package.json'), - path.join(path.dirname(pkgDir), 'package.json'), - ]) { - if (fs.existsSync(candidate)) { - modulePkgJsonPaths.push(candidate); - break; - } - } -} - -// Two things come out of modules.assets.json. -// -// 1. `server.fs.allow` entries. The dev server must be allowed to read each -// module's package dir. Read from modules.assets.json rather than -// modules.manifest.json because the manifest is keyed off `pages/`, so a -// module shipping only CSS never appears in it. -// -// 2. A convenience `#module/` alias. This is NOT required by -// `modules.generated.css` — that file imports module stylesheets by -// absolute path, so it resolves with no alias configured at all. Emitting -// an alias there made a generated file depend on this hand-owned config, -// and since `vite.config.ts` is scaffolded once and then owned by the app, -// a Python-only version bump broke every host scaffolded earlier -// (GH issue #253). The alias stays because it costs nothing and lets a -// module's own CSS/TS reference a sibling module by package name. -// -// `@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. -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); +// Everything the installed modules contribute to this build — fs.allow entries, +// dep-scan globs, package.json paths, and the `#module/` + npm-name +// aliases. See ./module-assets.ts for why each exists. +const { + fsAllow: moduleFsAllow, + optimizeEntries: moduleOptimizeEntries, + pkgJsonPaths: modulePkgJsonPaths, + pagesPrefixes: modulePagesPrefixes, + aliases: moduleAliases, + npmNames: moduleNpmNames, +} = loadModuleAssets(import.meta.dirname); // Gather every bare specifier a module's pages might import. We include // both `dependencies` (deps the module ships its own copy of) and @@ -109,7 +43,12 @@ function collectModuleDecls(): string[] { } for (const block of [pkg.dependencies, pkg.peerDependencies]) { for (const dep of Object.keys(block ?? {})) { - if (!dep.startsWith('@types/')) decls.add(dep); + if (dep.startsWith('@types/')) continue; + // A sibling module declared as a dep/peer dep is not a node_modules + // package — it is aliased to a source directory above. Pre-bundling + // it would point the optimizer at raw .tsx with no entry point. + if (moduleNpmNames.has(dep)) continue; + decls.add(dep); } } } From 1a9686545d48786542323b762f0cd12b1d555cbb Mon Sep 17 00:00:00 2001 From: Anto Subash Date: Fri, 7 Aug 2026 23:48:42 +0200 Subject: [PATCH 3/3] ci: run the module-asset build guards in a job that can actually run them MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit test_module_css_build.py self-skips without node_modules, and the `Python tests` job runs `make install-py` only — so both real-build guards silently skipped in CI (`test_module_css_build.py ss`). They are the only tests that prove module CSS and cross-module npm-name imports resolve through a real Tailwind/Vite build; every unit test around them stays green while the wiring is broken. `JS build (Vite)` already installs Python deps, JS deps, and runs gen-pages + build, so it is the one job with everything they need. --- .github/workflows/pr.yml | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/.github/workflows/pr.yml b/.github/workflows/pr.yml index 169e9da3..f50de711 100644 --- a/.github/workflows/pr.yml +++ b/.github/workflows/pr.yml @@ -125,6 +125,12 @@ jobs: - run: make install-js - run: make gen-pages - run: make build + # These drive a real gen-pages + vite build to prove module-shipped CSS + # and cross-module npm-name imports actually resolve. They self-skip + # without node_modules, so the `Python tests` job (make install-py only) + # silently skips them — this is the one job that can really run them. + - name: Module asset build guards + run: uv run pytest framework/cli/tests/test_module_css_build.py e2e-smoke: name: E2E smoke (Playwright)