Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -35,14 +35,25 @@ const fsRoot = findNodeModulesRoot(__dirname);
// dev server can read files outside the host root, and in
// optimizeDeps.entries so its dependency scanner discovers bare imports
// from wheel-installed pages and pre-bundles them.
//
// We also collect each module's package.json (one level up from pages/,
// where Hatch's force-include drops it) so the dependency walk in
// `collectOptimizeIncludes` reaches packages a wheel-installed page imports
// directly (`sonner`, `lucide-react`, ...). Without this seed, Vite's
// pre-bundler never sees those bare specifiers and Node module resolution
// walks up from inside .venv/site-packages — never reaching
// host/client_app/node_modules.
const manifestPath = path.resolve(__dirname, 'modules.manifest.json');
const moduleFsAllow: string[] = [];
const moduleOptimizeEntries: string[] = [];
const modulePkgJsonPaths: string[] = [];
if (fs.existsSync(manifestPath)) {
const manifest = JSON.parse(fs.readFileSync(manifestPath, 'utf-8')) as Record<string, string>;
for (const pagesDir of Object.values(manifest)) {
moduleFsAllow.push(path.dirname(pagesDir));
moduleOptimizeEntries.push(path.join(pagesDir, '**/*.tsx'));
const modulePkgJson = path.join(path.dirname(pagesDir), 'package.json');
if (fs.existsSync(modulePkgJson)) modulePkgJsonPaths.push(modulePkgJson);
}
}

Expand Down Expand Up @@ -99,7 +110,7 @@ function collectOptimizeIncludes(): string[] {
'use-sync-external-store/shim/with-selector',
]);
const visited = new Set<string>();
const queue: string[] = [path.join(__dirname, 'package.json')];
const queue: string[] = [path.join(__dirname, 'package.json'), ...modulePkgJsonPaths];
while (queue.length > 0) {
const pkgJsonPath = queue.shift();
if (!pkgJsonPath || visited.has(pkgJsonPath)) continue;
Expand Down
61 changes: 61 additions & 0 deletions framework/cli/tests/test_cli_new_regressions.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,61 @@
"""Regression tests for ``sm new`` scaffold bugs filed against released wheels.

Each test pins a specific issue's repro so the bug can't sneak back in
without a CI failure pointing at the fix.
"""

from __future__ import annotations

import json
from pathlib import Path

from simple_module_cli.cli import app
from typer.testing import CliRunner


def test_sm_new_flat_pins_inertia_react_to_v2(tmp_path: Path) -> None:
"""Issue #128: flat scaffold's root package.json must peer-match
@simple-module-py/ui's ``@inertiajs/react: ^2.0.0`` peer dep."""
runner = CliRunner()
target = tmp_path / "demo"
runner.invoke(
app,
["new", "demo", "--yes", "--flat", "--no-install", "--dest", str(target)],
)
data = json.loads((target / "package.json").read_text())
inertia = data.get("dependencies", {}).get("@inertiajs/react")
assert inertia is not None
assert inertia.startswith("^2."), f"expected ^2.x, got {inertia!r}"


def test_sm_new_sample_module_pins_match_framework_version(tmp_path: Path) -> None:
"""Issue #126/#119: sample hello module must pin framework deps to the
actual published framework version, not the future ``>=1.0,<2.0`` range."""
from importlib.metadata import version

expected = version("simple_module_cli")
runner = CliRunner()
target = tmp_path / "demo"
runner.invoke(
app,
["new", "demo", "--yes", "--db", "sqlite", "--no-install", "--dest", str(target)],
)
text = (target / "modules" / "hello" / "pyproject.toml").read_text()
for pkg in ("simple_module_core", "simple_module_db", "simple_module_hosting"):
assert f"{pkg}=={expected}" in text, f"{pkg} should be pinned to =={expected}"
# Dev pin: simple_module_test was the original >=0.1,<1.0 unsatisfiable case.
assert f"simple_module_test=={expected}" in text


def test_sm_new_sample_module_seeds_static_dist_placeholder(tmp_path: Path) -> None:
"""Issue #127: hatch's force-include resolves at uv-sync time. A fresh
scaffold must ship an empty ``static/dist/`` so the build doesn't fail
before vite has had a chance to run."""
runner = CliRunner()
target = tmp_path / "demo"
runner.invoke(
app,
["new", "demo", "--yes", "--db", "sqlite", "--no-install", "--dest", str(target)],
)
static_dist = target / "modules" / "hello" / "hello" / "static" / "dist"
assert static_dist.is_dir(), "static/dist/ must exist for hatch force-include"
29 changes: 27 additions & 2 deletions framework/hosting/simple_module_hosting/manifest.py
Original file line number Diff line number Diff line change
Expand Up @@ -45,13 +45,38 @@


def repo_root_from_client_app(client_app_dir: Path) -> Path:
"""Repo root is two levels above ``host/client_app/``.
"""Locate the workspace/repo root that contains ``client_app_dir``.

Both ``write_module_pages_manifest`` and the ``sync-js-deps`` CLI
derive the workspace root from the host's client_app directory.
Centralized here so the heuristic lives in exactly one place.

Walks up looking for the nearest ``package.json`` that declares
``workspaces`` (the workspace root npm uses), falling back to any
``package.json``. This handles both the framework repo's
``host/client_app/`` layout and the flat ``sm new`` scaffold's
``client_app/`` layout (where the parent IS the workspace root).
"""
return client_app_dir.resolve().parent.parent
here = client_app_dir.resolve()
fallback: Path | None = None
for parent in (here.parent, *here.parents):
pkg = parent / "package.json"
if not pkg.is_file():
continue
if fallback is None:
fallback = parent
try:
data = json.loads(pkg.read_text(encoding="utf-8"))
except json.JSONDecodeError:
continue
if "workspaces" in data:
return parent
if fallback is not None:
return fallback
# No package.json found anywhere on the way up — preserve the legacy
# two-levels-up shape so framework-internal callers still get a valid
# path even before any npm setup has happened.
return here.parent.parent


def compute_module_pages(modules: Sequence[ModuleBase]) -> dict[str, Path]:
Expand Down
60 changes: 60 additions & 0 deletions framework/hosting/tests/test_manifest.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,60 @@
"""Tests for ``simple_module_hosting.manifest`` helpers."""

from __future__ import annotations

import json
from pathlib import Path

from simple_module_hosting.manifest import repo_root_from_client_app


def test_repo_root_finds_workspace_root_in_framework_layout(tmp_path: Path) -> None:
"""``host/client_app/`` shape: walk up to the workspace package.json."""
workspace = tmp_path / "ws"
(workspace / "host" / "client_app").mkdir(parents=True)
(workspace / "package.json").write_text(
json.dumps({"name": "ws", "workspaces": ["host/client_app", "modules/*"]})
)
(workspace / "host" / "package.json").write_text(json.dumps({"name": "ws-host"}))

assert repo_root_from_client_app(workspace / "host" / "client_app") == workspace.resolve()


def test_repo_root_finds_root_in_flat_scaffold_layout(tmp_path: Path) -> None:
"""Flat scaffold: ``client_app/`` directly under the host root."""
host = tmp_path / "my-app"
(host / "client_app").mkdir(parents=True)
(host / "package.json").write_text(json.dumps({"name": "my-app"}))

assert repo_root_from_client_app(host / "client_app") == host.resolve()


def test_repo_root_prefers_workspace_package_json_over_nearer_one(tmp_path: Path) -> None:
"""When a non-workspace package.json sits between client_app and the
workspace root, the workspace one wins so npm install runs at the right cwd."""
workspace = tmp_path / "ws"
(workspace / "host" / "client_app").mkdir(parents=True)
(workspace / "package.json").write_text(
json.dumps({"name": "ws", "workspaces": ["host/client_app"]})
)
(workspace / "host" / "package.json").write_text(json.dumps({"name": "ws-host"}))

assert repo_root_from_client_app(workspace / "host" / "client_app") == workspace.resolve()


def test_repo_root_falls_back_to_nearest_package_json(tmp_path: Path) -> None:
"""No workspaces field anywhere — settle on the nearest package.json found."""
root = tmp_path / "root"
(root / "client_app").mkdir(parents=True)
(root / "package.json").write_text(json.dumps({"name": "root"}))

assert repo_root_from_client_app(root / "client_app") == root.resolve()


def test_repo_root_falls_back_to_two_levels_up_if_no_package_json(tmp_path: Path) -> None:
"""No package.json exists yet — preserve the legacy two-up behaviour so
framework-internal callers still get a path before any npm setup."""
deep = tmp_path / "outer" / "inner" / "client_app"
deep.mkdir(parents=True)

assert repo_root_from_client_app(deep) == (tmp_path / "outer").resolve()
6 changes: 6 additions & 0 deletions modules/background_tasks/background_tasks/module.py
Original file line number Diff line number Diff line change
Expand Up @@ -96,8 +96,14 @@ async def on_startup(self, app: FastAPI) -> None:

from background_tasks.celery_app import build_celery
from background_tasks.signals import bind_event_bus
from background_tasks.sync_db import set_database_url

services = app.state.background_tasks
# Pin the sync engine to the same URL the host's async settings
# resolved — pydantic-settings reads ``.env`` but never propagates
# to ``os.environ``, so signals would otherwise fall back to the
# SQLite default and silently drop ``TaskExecution`` rows.
set_database_url(app.state.sm.settings.database_url)
# build_celery imports `signals` for side effects and runs
# `autodiscover_tasks` across every installed module.
services.celery = build_celery(services.settings)
Expand Down
34 changes: 31 additions & 3 deletions modules/background_tasks/background_tasks/sync_db.py
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@

_engine: Engine | None = None
_session_factory: sessionmaker[Session] | None = None
_url_override: str | None = None


def _sync_url(async_url: str) -> str:
Expand All @@ -37,9 +38,35 @@ def _sync_url(async_url: str) -> str:
return async_url.replace("+aiosqlite", "").replace("+asyncpg", "+psycopg2")


def set_database_url(url: str | None) -> None:
"""Pin the URL used to build the sync engine.

The web process loads ``.env`` via pydantic-settings, but those values
never land in ``os.environ`` — so reading ``SM_DATABASE_URL`` directly
from the env can silently drop us back to the SQLite default while the
rest of the app uses Postgres. ``BackgroundTasksModule.on_startup``
calls this with the resolved ``settings.database_url`` so signals use
the same DB the app is on. Pass ``None`` to clear the override (used in
tests + on shutdown).
"""
global _url_override, _engine, _session_factory
if _url_override == url:
return
_url_override = url
if _engine is not None:
_engine.dispose()
_engine = None
_session_factory = None


def _resolve_url() -> str:
if _url_override is not None:
return _url_override
return os.environ.get("SM_DATABASE_URL", "sqlite:///./app.db")


def _build_engine() -> Engine:
url = os.environ.get("SM_DATABASE_URL", "sqlite:///./app.db")
sync_url = _sync_url(url)
sync_url = _sync_url(_resolve_url())
# Small pool — signals fire sequentially per worker process.
return create_engine(sync_url, pool_pre_ping=True, pool_size=2, max_overflow=3)

Expand All @@ -60,11 +87,12 @@ def dispose_sync_engine() -> None:
restarts within one process (test runners, uvicorn dev reload) don't
accumulate engines against the old DB URL.
"""
global _engine, _session_factory
global _engine, _session_factory, _url_override
if _engine is not None:
_engine.dispose()
_engine = None
_session_factory = None
_url_override = None


@contextmanager
Expand Down
60 changes: 60 additions & 0 deletions modules/background_tasks/tests/test_sync_db.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,60 @@
"""Tests for the sync session factory's URL resolution.

The web process loads ``.env`` via pydantic-settings but never propagates the
result to ``os.environ``. ``set_database_url`` lets the module's
``on_startup`` pin the sync engine to whatever the host settings resolved,
so signals don't silently fall back to SQLite while the rest of the app
talks to Postgres.
"""

from __future__ import annotations

from collections.abc import Iterator

import pytest
from background_tasks import sync_db


@pytest.fixture(autouse=True)
def _reset_sync_db() -> Iterator[None]:
sync_db.dispose_sync_engine()
yield
sync_db.dispose_sync_engine()


def test_resolve_url_falls_back_to_env(monkeypatch: pytest.MonkeyPatch) -> None:
monkeypatch.setenv("SM_DATABASE_URL", "sqlite:///./from-env.db")
sync_db.set_database_url(None)

assert sync_db._resolve_url() == "sqlite:///./from-env.db"


def test_set_database_url_overrides_env(monkeypatch: pytest.MonkeyPatch) -> None:
monkeypatch.setenv("SM_DATABASE_URL", "sqlite:///./from-env.db")
sync_db.set_database_url("postgresql+asyncpg://u:p@h/db")

assert sync_db._resolve_url() == "postgresql+asyncpg://u:p@h/db"


def test_set_database_url_resets_engine_when_url_changes(tmp_path) -> None:
db_a = tmp_path / "a.db"
db_b = tmp_path / "b.db"
sync_db.set_database_url(f"sqlite:///{db_a}")
factory_a = sync_db.get_sync_session_factory()
sync_db.set_database_url(f"sqlite:///{db_a}")
factory_a_again = sync_db.get_sync_session_factory()
assert factory_a is factory_a_again, "same URL must reuse the cached factory"

sync_db.set_database_url(f"sqlite:///{db_b}")
factory_b = sync_db.get_sync_session_factory()
assert factory_b is not factory_a, "URL change must dispose the old engine"


def test_dispose_sync_engine_clears_url_override(tmp_path) -> None:
url = f"sqlite:///{tmp_path / 'pinned.db'}"
sync_db.set_database_url(url)
assert sync_db._url_override == url

sync_db.dispose_sync_engine()

assert sync_db._url_override is None
Loading