Skip to content

Commit b3d3365

Browse files
antosubashclaude
andauthored
fix(vite): resolve cross-package bare imports from out-of-workspace modules (#154)
* fix(vite): resolve cross-package bare imports from out-of-workspace modules (#152) When a module's pages live under .venv/.../site-packages/ (wheel install) or otherwise sit outside the workspace root, Vite's resolver walks up from the importer looking for node_modules and never reaches <repo>/node_modules where npm hoists deps. Bare imports like `maplibre-gl`, `pmtiles`, or even host-provided peers (`@inertiajs/react`, `@simple-module-py/ui`) fail with "Failed to resolve import … Does the file exist?". The fix adds a `pre` resolveId Vite plugin that catches bare imports from any registered module-pages dir and re-resolves them as if the importer lived at the workspace root, putting <repo>/node_modules back on the resolver's path. Combined with pre-bundling module-declared deps and peer deps via `optimizeDeps.include`, dev server, scan-imports, and production build all converge on the host's hoisted copy. Tested by pointing the manifest at a copy of the dashboard module outside the workspace; without the fix `vite dev`/`vite build` fail with the issue's error, with it both succeed and pages render normally. * fix(tests): ruff lint — sort imports and lowercase variable names in test_manifest * refactor(vite): tighten module-pages guard + lift hot-path concat Three quality fixes from /simplify review: - moduleBareImportResolver now guards on pagesDir prefixes (modulePagesPrefixes), not pkgDir — so only files actually under `pages/` trigger the workspace re-resolution, not arbitrary files in the module package. - Precompute the workspace-prefix string + fakeWorkspaceImporter path once at config load, instead of rebuilding them inside the resolveId hot path on every import. - Drop three inline comments that narrated the guard logic; the block comment above the function already explains the why. Test side: extend fake_module_factory to write peerDependencies and assert read_module_package_json surfaces them — the TS-side optimizeDeps walk reads both blocks and was previously uncovered. --------- Co-authored-by: Claude <noreply@anthropic.com>
1 parent 8152248 commit b3d3365

3 files changed

Lines changed: 292 additions & 29 deletions

File tree

‎framework/cli/simple_module_cli/templates/host/client_app/vite.config.ts‎

Lines changed: 85 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@ import fs from 'node:fs';
22
import path from 'node:path';
33
import tailwindcss from '@tailwindcss/vite';
44
import react from '@vitejs/plugin-react';
5-
import { defineConfig } from 'vite';
5+
import { type Plugin, defineConfig } from 'vite';
66

77
// Force every importer (host, workspace module, wheel-installed module)
88
// to resolve to one React copy + a single Inertia hook context. Without
@@ -36,26 +36,40 @@ const fsRoot = findNodeModulesRoot(__dirname);
3636
// optimizeDeps.entries so its dependency scanner discovers bare imports
3737
// from wheel-installed pages and pre-bundles them.
3838
//
39-
// We also collect each module's package.json (one level up from pages/,
40-
// where Hatch's force-include drops it) so the dependency walk in
41-
// `collectOptimizeIncludes` reaches packages a wheel-installed page imports
42-
// directly (`sonner`, `lucide-react`, ...). Without this seed, Vite's
43-
// pre-bundler never sees those bare specifiers and Node module resolution
44-
// walks up from inside .venv/site-packages — never reaching
45-
// host/client_app/node_modules.
39+
// We also collect each module's package.json — wheels embed it next to
40+
// the Python package (one level up from pages/, force-included by Hatch),
41+
// while editable/workspace installs leave it at the source-tree module
42+
// root (two levels up). We accept either. The dep walk in
43+
// `collectOptimizeIncludes` uses it to reach packages a module's pages
44+
// import directly (`sonner`, `lucide-react`, `maplibre-gl`, …). Without
45+
// this seed, Vite's pre-bundler never sees those bare specifiers and Node
46+
// module resolution walks up from inside .venv/site-packages — never
47+
// reaching host/client_app/node_modules.
4648
const manifestPath = path.resolve(__dirname, 'modules.manifest.json');
4749
const moduleFsAllow: string[] = [];
4850
const moduleOptimizeEntries: string[] = [];
4951
const modulePkgJsonPaths: string[] = [];
52+
const modulePagesPrefixes: string[] = [];
5053
if (fs.existsSync(manifestPath)) {
5154
const manifest = JSON.parse(fs.readFileSync(manifestPath, 'utf-8')) as Record<string, string>;
5255
for (const pagesDir of Object.values(manifest)) {
53-
moduleFsAllow.push(path.dirname(pagesDir));
56+
const pkgDir = path.dirname(pagesDir);
57+
moduleFsAllow.push(pkgDir);
5458
moduleOptimizeEntries.push(path.join(pagesDir, '**/*.tsx'));
55-
const modulePkgJson = path.join(path.dirname(pagesDir), 'package.json');
56-
if (fs.existsSync(modulePkgJson)) modulePkgJsonPaths.push(modulePkgJson);
59+
modulePagesPrefixes.push(pagesDir + path.sep);
60+
for (const candidate of [
61+
path.join(pkgDir, 'package.json'),
62+
path.join(path.dirname(pkgDir), 'package.json'),
63+
]) {
64+
if (fs.existsSync(candidate)) {
65+
modulePkgJsonPaths.push(candidate);
66+
break;
67+
}
68+
}
5769
}
5870
}
71+
const fsRootPrefix = fsRoot + path.sep;
72+
const fakeWorkspaceImporter = path.join(fsRoot, 'package.json');
5973

6074
// CJS-only deps like `clsx`, `tailwind-merge`, `class-variance-authority`
6175
// expose named exports only after esbuild's CJS→ESM transform. Vite's
@@ -70,6 +84,7 @@ type Pkg = {
7084
module?: string;
7185
exports?: unknown;
7286
dependencies?: Record<string, string>;
87+
peerDependencies?: Record<string, string>;
7388
};
7489

7590
const pkgCache = new Map<string, Pkg | null>();
@@ -117,26 +132,72 @@ function collectOptimizeIncludes(): string[] {
117132
visited.add(pkgJsonPath);
118133
const pkg = readPackageJSON(pkgJsonPath);
119134
if (!pkg) continue;
120-
for (const name of Object.keys(pkg.dependencies ?? {})) {
121-
if (name.startsWith('@types/')) continue;
122-
const nested = findPackageJSON(name);
123-
if (!nested) continue;
124-
const nestedPkg = readPackageJSON(nested);
125-
if (!nestedPkg) continue;
126-
// Skip packages that ship only sub-paths (`@babel/runtime`); vite
127-
// refuses to pre-bundle them and bare imports against them resolve
128-
// naturally through Node's normal module-walk anyway.
129-
if (hasTopLevelEntry(nestedPkg)) {
130-
includes.add(name);
135+
// Walk both `dependencies` and `peerDependencies`: a module's pages
136+
// routinely import host-provided peer deps (`@inertiajs/react`,
137+
// `@simple-module-py/ui`, …) as bare specifiers and we need those
138+
// pre-bundled too, not just the deps the module ships its own copy of.
139+
for (const block of [pkg.dependencies, pkg.peerDependencies]) {
140+
for (const name of Object.keys(block ?? {})) {
141+
if (name.startsWith('@types/')) continue;
142+
const nested = findPackageJSON(name);
143+
if (!nested) continue;
144+
const nestedPkg = readPackageJSON(nested);
145+
if (!nestedPkg) continue;
146+
// Skip packages that ship only sub-paths (`@babel/runtime`); vite
147+
// refuses to pre-bundle them and bare imports against them resolve
148+
// naturally through Node's normal module-walk anyway.
149+
if (hasTopLevelEntry(nestedPkg)) {
150+
includes.add(name);
151+
}
152+
queue.push(nested);
131153
}
132-
queue.push(nested);
133154
}
134155
}
135156
return [...includes];
136157
}
137158

159+
// Cross-package bare imports from module pages (`maplibre-gl`, `pmtiles`,
160+
// `@inertiajs/react`, …) live in fsRoot/node_modules after `npm install`.
161+
// But when a module's pages sit outside fsRoot — under
162+
// `.venv/.../site-packages/<pkg>/pages/` for wheel installs — Vite's
163+
// resolver walks up from the importer looking for node_modules and never
164+
// reaches fsRoot/node_modules. Resolution fails with: "Failed to resolve
165+
// import … Does the file exist?".
166+
//
167+
// This plugin recovers by retrying any unresolved bare import from a
168+
// module-pages importer as if the importer lived at fsRoot, which puts
169+
// fsRoot/node_modules back on the resolver's path. Combined with the
170+
// `optimizeDeps.include` walk above (module deps + peer deps), dev,
171+
// dep-scan, and production builds all converge on the host's hoisted copy.
172+
function moduleBareImportResolver(): Plugin {
173+
return {
174+
name: 'simple-module:resolve-module-bare-imports',
175+
enforce: 'pre',
176+
async resolveId(source, importer) {
177+
if (!importer) return null;
178+
if (
179+
source.startsWith('.') ||
180+
source.startsWith('/') ||
181+
source.startsWith('\0') ||
182+
source.startsWith('virtual:')
183+
) {
184+
return null;
185+
}
186+
const importerPath = importer.split('?')[0];
187+
if (importerPath.startsWith(fsRootPrefix)) return null;
188+
if (!modulePagesPrefixes.some((prefix) => importerPath.startsWith(prefix))) {
189+
return null;
190+
}
191+
const resolved = await this.resolve(source, fakeWorkspaceImporter, {
192+
skipSelf: true,
193+
});
194+
return resolved ?? null;
195+
},
196+
};
197+
}
198+
138199
export default defineConfig({
139-
plugins: [react(), tailwindcss()],
200+
plugins: [moduleBareImportResolver(), react(), tailwindcss()],
140201
root: __dirname,
141202
resolve: {
142203
dedupe: [...REACT_CORE_DEPS, '@simple-module-py/ui', '@simple-module-py/i18n'],

‎framework/hosting/tests/test_manifest.py‎

Lines changed: 106 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,10 +2,18 @@
22

33
from __future__ import annotations
44

5+
import importlib.util
56
import json
7+
import sys
68
from pathlib import Path
79

8-
from simple_module_hosting.manifest import repo_root_from_client_app
10+
import pytest
11+
from simple_module_core import ModuleBase, ModuleMeta
12+
from simple_module_hosting.manifest import (
13+
collect_module_js_deps,
14+
read_module_package_json,
15+
repo_root_from_client_app,
16+
)
917

1018

1119
def test_repo_root_finds_workspace_root_in_framework_layout(tmp_path: Path) -> None:
@@ -58,3 +66,100 @@ def test_repo_root_falls_back_to_two_levels_up_if_no_package_json(tmp_path: Path
5866
deep.mkdir(parents=True)
5967

6068
assert repo_root_from_client_app(deep) == (tmp_path / "outer").resolve()
69+
70+
71+
@pytest.fixture
72+
def fake_module_factory(tmp_path: Path, monkeypatch: pytest.MonkeyPatch):
73+
"""Build a fake installed module with a configurable on-disk layout.
74+
75+
Returns a callable that takes a ``layout`` ∈ {"wheel", "source"} plus
76+
optional ``dependencies`` and ``peer_dependencies`` maps, lays the
77+
files down under ``tmp_path``, registers the package in ``sys.modules``,
78+
and returns a ``ModuleBase`` subclass pinned to it.
79+
"""
80+
counter = {"n": 0}
81+
82+
def _build(
83+
layout: str,
84+
dependencies: dict[str, str] | None = None,
85+
peer_dependencies: dict[str, str] | None = None,
86+
) -> type[ModuleBase]:
87+
counter["n"] += 1
88+
pkg_name = f"fake_module_{counter['n']}"
89+
if layout == "wheel":
90+
# Wheel: <pkg>/ contains code + package.json (Hatch force-include).
91+
pkg_root = tmp_path / pkg_name
92+
pkg_root.mkdir()
93+
pkg_json_path = pkg_root / "package.json"
94+
elif layout == "source":
95+
# Source-tree / editable: package.json sits above the Python pkg.
96+
module_root = tmp_path / f"{pkg_name}_repo"
97+
module_root.mkdir()
98+
pkg_root = module_root / pkg_name
99+
pkg_root.mkdir()
100+
pkg_json_path = module_root / "package.json"
101+
else:
102+
raise ValueError(f"unknown layout: {layout}")
103+
init_py = pkg_root / "__init__.py"
104+
init_py.write_text("")
105+
pkg_json: dict[str, object] = {
106+
"name": f"@fake/{pkg_name}",
107+
"dependencies": dependencies or {},
108+
}
109+
if peer_dependencies is not None:
110+
pkg_json["peerDependencies"] = peer_dependencies
111+
pkg_json_path.write_text(json.dumps(pkg_json))
112+
113+
# Register the package with a real importlib spec so that
114+
# importlib.resources.files() can locate the on-disk pkg_root.
115+
spec = importlib.util.spec_from_file_location(
116+
pkg_name, init_py, submodule_search_locations=[str(pkg_root)]
117+
)
118+
assert spec is not None and spec.loader is not None
119+
mod = importlib.util.module_from_spec(spec)
120+
spec.loader.exec_module(mod)
121+
monkeypatch.setitem(sys.modules, pkg_name, mod)
122+
123+
class FakeMod(ModuleBase):
124+
meta = ModuleMeta(name=pkg_name.title().replace("_", ""))
125+
126+
FakeMod.__module__ = pkg_name
127+
return FakeMod
128+
129+
return _build
130+
131+
132+
def test_read_module_package_json_finds_wheel_layout(fake_module_factory) -> None:
133+
"""Wheel install: package.json sits next to the Python package."""
134+
fake_cls = fake_module_factory(
135+
"wheel",
136+
{"dep-a": "^1.0.0"},
137+
peer_dependencies={"@host/peer": "^2.0.0"},
138+
)
139+
pkg = read_module_package_json(fake_cls())
140+
assert pkg is not None
141+
assert pkg["dependencies"] == {"dep-a": "^1.0.0"}
142+
# The TS-side vite.config.ts also reads peerDependencies for its
143+
# optimizeDeps walk — make sure the raw dict surfaces both blocks.
144+
assert pkg["peerDependencies"] == {"@host/peer": "^2.0.0"}
145+
146+
147+
def test_read_module_package_json_finds_source_layout(fake_module_factory) -> None:
148+
"""Source-tree / workspace: package.json sits at the module repo root."""
149+
fake_cls = fake_module_factory("source", {"dep-b": "^2.0.0"})
150+
pkg = read_module_package_json(fake_cls())
151+
assert pkg is not None
152+
assert pkg["dependencies"] == {"dep-b": "^2.0.0"}
153+
154+
155+
def test_collect_module_js_deps_aggregates_across_layouts(fake_module_factory) -> None:
156+
"""Mixed-layout modules all contribute their declared deps."""
157+
wheel_cls = fake_module_factory("wheel", {"cmdk": "^1.0.0"})
158+
source_cls = fake_module_factory("source", {"maplibre-gl": "^4.7.0", "pmtiles": "^3.2.0"})
159+
empty_cls = fake_module_factory("wheel", {})
160+
161+
deps = collect_module_js_deps([wheel_cls(), source_cls(), empty_cls()])
162+
# Empty deps are dropped; both populated modules appear by their meta.name.
163+
assert empty_cls.meta.name not in deps
164+
assert deps[wheel_cls.meta.name] == {"cmdk": "^1.0.0"}
165+
assert deps[source_cls.meta.name] == {"maplibre-gl": "^4.7.0", "pmtiles": "^3.2.0"}

0 commit comments

Comments
 (0)