From 01564d42a21f400606c6aa814d6ec2f999d86e0e Mon Sep 17 00:00:00 2001 From: Masen Furer Date: Fri, 8 May 2026 07:01:18 -0700 Subject: [PATCH 01/14] Move bun.lock under reflex.lock/bun.lock This will avoid conflicts with reflex apps that also host some javascript app in the same directory. --- docs/getting_started/project-structure.md | 2 +- .../reflex-base/src/reflex_base/constants/installer.py | 5 +++++ reflex/utils/frontend_skeleton.py | 8 ++++++-- tests/units/test_prerequisites.py | 8 ++++++-- 4 files changed, 18 insertions(+), 5 deletions(-) diff --git a/docs/getting_started/project-structure.md b/docs/getting_started/project-structure.md index 8068f316410..b757e96e0b0 100644 --- a/docs/getting_started/project-structure.md +++ b/docs/getting_started/project-structure.md @@ -44,7 +44,7 @@ This is where the compiled Javascript files will be stored. You will never need Each Reflex page will compile to a corresponding `.js` file in the `.web/pages` directory. -If Reflex installs frontend dependencies with Bun, the canonical `bun.lock` lives in your project root and should be committed to version control. Reflex mirrors it into `.web` when it needs to run the package manager. +If Reflex installs frontend dependencies with Bun, the canonical `bun.lock` lives in a `reflex.lock/` directory at your project root and should be committed to version control. The dedicated directory keeps the file from clashing with a `bun.lock` from a user-managed bun project that may live alongside the Reflex project. Reflex mirrors `reflex.lock/bun.lock` into `.web` when it needs to run the package manager. ## Assets diff --git a/packages/reflex-base/src/reflex_base/constants/installer.py b/packages/reflex-base/src/reflex_base/constants/installer.py index 2133a4bba11..8f546b41d13 100644 --- a/packages/reflex-base/src/reflex_base/constants/installer.py +++ b/packages/reflex-base/src/reflex_base/constants/installer.py @@ -33,6 +33,11 @@ class Bun(SimpleNamespace): # Path of the bun lockfile. LOCKFILE_PATH = "bun.lock" + # Directory in the app root where the canonical bun lockfile is stored. + # A dedicated directory avoids clashes with a user's own bun project + # that may sit in the same directory as the Reflex project. + ROOT_LOCKFILE_DIR = "reflex.lock" + @classproperty @classmethod def ROOT_PATH(cls): diff --git a/reflex/utils/frontend_skeleton.py b/reflex/utils/frontend_skeleton.py index 12054b9a2d3..147485eeef5 100644 --- a/reflex/utils/frontend_skeleton.py +++ b/reflex/utils/frontend_skeleton.py @@ -151,12 +151,15 @@ def initialize_requirements_txt( def get_root_bun_lock_path() -> Path: """Get the canonical bun lock path in the app root. - This assumes the current working directory is the Reflex app root. + The lockfile is stored inside a dedicated directory under the app root so + it cannot collide with a user-managed bun project that lives at the same + level as the Reflex project. This assumes the current working directory + is the Reflex app root. Returns: The canonical bun lock path in the app root. """ - return Path.cwd() / constants.Bun.LOCKFILE_PATH + return Path.cwd() / constants.Bun.ROOT_LOCKFILE_DIR / constants.Bun.LOCKFILE_PATH def get_web_bun_lock_path() -> Path: @@ -193,6 +196,7 @@ def sync_web_bun_lock_to_root(): return root_bun_lock_path = get_root_bun_lock_path() + path_ops.mkdir(root_bun_lock_path.parent) console.debug(f"Copying {web_bun_lock_path} to {root_bun_lock_path}") path_ops.cp(web_bun_lock_path, root_bun_lock_path) diff --git a/tests/units/test_prerequisites.py b/tests/units/test_prerequisites.py index 0fc6321e3d4..7ca16a11071 100644 --- a/tests/units/test_prerequisites.py +++ b/tests/units/test_prerequisites.py @@ -89,10 +89,12 @@ def patch_pm(package_managers: list[str], run_package_manager: Callable) -> None def install(packages: set[str] | None = None) -> None: js_runtimes.install_frontend_packages(packages or set(), config) + root_lock = tmp_path / constants.Bun.ROOT_LOCKFILE_DIR / constants.Bun.LOCKFILE_PATH + root_lock.parent.mkdir(parents=True, exist_ok=True) env = InstallPackagesEnv( tmp_path=tmp_path, web_dir=web_dir, - root_lock=tmp_path / constants.Bun.LOCKFILE_PATH, + root_lock=root_lock, web_lock=web_dir / constants.Bun.LOCKFILE_PATH, config=config, patch_pm=patch_pm, @@ -200,7 +202,9 @@ def test_initialize_web_directory_restores_root_bun_lock(tmp_path, monkeypatch): ) web_dir = tmp_path / constants.Dirs.WEB - (tmp_path / constants.Bun.LOCKFILE_PATH).write_text("root-lock") + root_lock = tmp_path / constants.Bun.ROOT_LOCKFILE_DIR / constants.Bun.LOCKFILE_PATH + root_lock.parent.mkdir(parents=True, exist_ok=True) + root_lock.write_text("root-lock") _patch_web_dir(monkeypatch, web_dir) with chdir(tmp_path): From f5e514686af6734b56d83e5da4d11b8d3ee3bc94 Mon Sep 17 00:00:00 2001 From: Masen Furer Date: Fri, 8 May 2026 11:37:48 -0700 Subject: [PATCH 02/14] Exclude `reflex.lock` from hot reload paths Avoid reload loop when a code change results in a lock file change. --- reflex/utils/exec.py | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/reflex/utils/exec.py b/reflex/utils/exec.py index 0889e9359e1..9e4b326da56 100644 --- a/reflex/utils/exec.py +++ b/reflex/utils/exec.py @@ -499,7 +499,11 @@ def is_excluded_by_default(path: Path) -> bool: if path.name.startswith("__"): # ignore things like __pycache__ return True - return path.name in (".gitignore", "uploaded_files") + return path.name in ( + ".gitignore", + "uploaded_files", + constants.Bun.ROOT_LOCKFILE_DIR, + ) reload_paths = ( tuple( From 6431e90185e196ef455d4776f3955e254fbf531d Mon Sep 17 00:00:00 2001 From: Masen Furer Date: Fri, 8 May 2026 12:26:02 -0700 Subject: [PATCH 03/14] Preserve package.json in reflex.lock This allows restoration of the previous package.json for the app. Explicitly pinned packages will be updated with `bun add` so they will overwrite what might have been in the lock file / package.json previously. This is important for updating framework dependencies across reflex versions which are strictly pinned. Unpinned packages are added with `bun add --only-missing` so they will NOT override previously saved version pins. --- reflex/utils/frontend_skeleton.py | 74 ++++++++++++- reflex/utils/js_runtimes.py | 155 ++++++++++++++++++++++++-- tests/units/test_prerequisites.py | 177 +++++++++++++++++++++++++++++- 3 files changed, 387 insertions(+), 19 deletions(-) diff --git a/reflex/utils/frontend_skeleton.py b/reflex/utils/frontend_skeleton.py index 147485eeef5..4980a41acd3 100644 --- a/reflex/utils/frontend_skeleton.py +++ b/reflex/utils/frontend_skeleton.py @@ -171,6 +171,27 @@ def get_web_bun_lock_path() -> Path: return get_web_dir() / constants.Bun.LOCKFILE_PATH +def get_root_package_json_path() -> Path: + """Get the persisted package.json path in the app root. + + Stored alongside ``bun.lock`` inside the dedicated lockfile directory so + resolved dependency pins survive a fresh ``reflex init``. + + Returns: + The persisted package.json path in the app root. + """ + return Path.cwd() / constants.Bun.ROOT_LOCKFILE_DIR / constants.PackageJson.PATH + + +def get_web_package_json_path() -> Path: + """Get the package.json path in the .web directory. + + Returns: + The package.json path in the .web directory. + """ + return get_web_dir() / constants.PackageJson.PATH + + def sync_root_bun_lock_to_web(): """Mirror the canonical root bun.lock into .web. @@ -201,6 +222,42 @@ def sync_web_bun_lock_to_root(): path_ops.cp(web_bun_lock_path, root_bun_lock_path) +def sync_web_package_json_to_root(): + """Persist the resolved .web package.json back to the app root. + + Captures the dependency pins produced by ``bun add`` so the next + ``reflex init`` can restore them as the starting point for the new + package.json. + """ + web_package_json_path = get_web_package_json_path() + if not web_package_json_path.exists(): + return + + root_package_json_path = get_root_package_json_path() + path_ops.mkdir(root_package_json_path.parent) + console.debug(f"Copying {web_package_json_path} to {root_package_json_path}") + path_ops.cp(web_package_json_path, root_package_json_path) + + +def _read_persisted_package_json() -> dict: + """Read the persisted package.json from the app root. + + Returns: + The parsed JSON object, or an empty dict if the file is missing or + cannot be parsed. + """ + root_package_json_path = get_root_package_json_path() + if not root_package_json_path.exists(): + return {} + try: + return json.loads(root_package_json_path.read_text()) + except (json.JSONDecodeError, OSError) as e: + console.warn( + f"Failed to read {root_package_json_path}: {e}; starting with empty dependency lists." + ) + return {} + + def initialize_web_directory(): """Initialize the web directory on reflex init.""" console.log("Initializing the web directory.") @@ -277,13 +334,26 @@ def _update_react_router_config(config: Config, prerender_routes: bool = False): def _compile_package_json(): + """Build package.json content for .web. + + Recovers ``dependencies`` and ``devDependencies`` from the persisted + ``reflex.lock/package.json`` (when present) so resolved version pins + survive a fresh ``reflex init``. ``scripts`` and ``overrides`` are always + refreshed from constants. The framework-managed entries in + ``constants.PackageJson.DEPENDENCIES`` / ``DEV_DEPENDENCIES`` are added + later at install time via ``bun add`` so they pick up strict pins. + + Returns: + Rendered package.json content as string. + """ + persisted = _read_persisted_package_json() return templates.package_json_template( scripts={ "dev": constants.PackageJson.Commands.DEV, "export": constants.PackageJson.Commands.EXPORT, }, - dependencies=constants.PackageJson.DEPENDENCIES, - dev_dependencies=constants.PackageJson.DEV_DEPENDENCIES, + dependencies=persisted.get("dependencies") or {}, + dev_dependencies=persisted.get("devDependencies") or {}, overrides=constants.PackageJson.OVERRIDES, ) diff --git a/reflex/utils/js_runtimes.py b/reflex/utils/js_runtimes.py index a4c0cd7a693..c38a4ead4dc 100644 --- a/reflex/utils/js_runtimes.py +++ b/reflex/utils/js_runtimes.py @@ -385,11 +385,80 @@ def _sync_root_bun_lock_for_frontend_install(): path_ops.rm(cache_file) +def _has_version_specifier(package_spec: str) -> bool: + """Check whether a package spec already includes a version specifier. + + Treats a package as pinned if it contains an ``@`` after the first + character (so scoped packages like ``@scope/pkg`` are unpinned, while + ``@scope/pkg@1.2.3`` is pinned). + + Args: + package_spec: The package spec to inspect. + + Returns: + Whether the spec carries a version specifier. + """ + return "@" in package_spec[1:] + + +def _split_by_version_specifier( + packages: set[str], +) -> tuple[set[str], set[str]]: + """Partition packages into pinned and unpinned sets. + + Args: + packages: Package specs to partition. + + Returns: + A tuple ``(pinned, unpinned)`` of disjoint package sets. + """ + pinned: set[str] = set() + unpinned: set[str] = set() + for package in packages: + if _has_version_specifier(package): + pinned.add(package) + else: + unpinned.add(package) + return pinned, unpinned + + +def _pinned_args_from_constants(deps: dict[str, str]) -> set[str]: + """Render constants-style dep dicts as ``name@version`` add args. + + Args: + deps: Mapping of package name to version string. + + Returns: + Set of ``name@version`` specs. + """ + return {f"{name}@{version}" for name, version in deps.items()} + + +def _frontend_packages_cache_payload( + packages: set[str], + config: Config, + install_package_managers: Sequence[str], +) -> str: + """Cache fingerprint for frontend package installs. + + Args: + packages: Custom packages requested by the caller. + config: The active Reflex config. + install_package_managers: The package manager paths in priority order. + + Returns: + Stable fingerprint string for the cached procedure. + """ + return ( + f"{sorted(packages)!r},{config.json()},{list(install_package_managers)!r}," + f"{sorted(constants.PackageJson.DEPENDENCIES.items())!r}," + f"{sorted(constants.PackageJson.DEV_DEPENDENCIES.items())!r}" + ) + + @cached_procedure( cache_file_path=_frontend_packages_cache_path, - payload_fn=lambda packages, config, install_package_managers: ( - f"{sorted(packages)!r},{config.json()},{list(install_package_managers)!r}" - ), + payload_fn=_frontend_packages_cache_payload, ) def _install_frontend_packages( packages: set[str], @@ -398,13 +467,26 @@ def _install_frontend_packages( ): """Installs the base and custom frontend packages. + Resolution rules: + * Framework deps in :attr:`constants.PackageJson.DEPENDENCIES` and + :attr:`constants.PackageJson.DEV_DEPENDENCIES` always carry version + specifiers and are added with strict pins so they overwrite any + existing entry in package.json. + * Plugin/custom packages with explicit version specifiers are also + added with strict pins. + * Plugin/custom packages without version specifiers are added with + ``--only-missing`` so previously resolved pins in package.json are + preserved across runs. + Args: - packages: A list of package names to be installed. - config: The config object. - install_package_managers: The package managers available for install. + packages: Custom packages requested by the caller (from + ``Config.frontend_packages`` and inferred component imports). + config: The active Reflex config. + install_package_managers: The package manager paths in priority + order (primary plus fallbacks). Example: - >>> install_frontend_packages(["react", "react-dom"], get_config()) + >>> install_frontend_packages({"react", "react-dom"}, get_config()) """ env = ( { @@ -426,32 +508,80 @@ def _install_frontend_packages( env=env, ) + # Install whatever was recovered into package.json from reflex.lock so + # the lockfile is honored before any further mutation. run_package_manager( [primary_package_manager, "install", "--legacy-peer-deps"], show_status_message="Installing base frontend packages", ) + framework_deps = _pinned_args_from_constants(constants.PackageJson.DEPENDENCIES) + if framework_deps: + run_package_manager( + [primary_package_manager, "add", "--legacy-peer-deps", *framework_deps], + show_status_message="Pinning framework frontend dependencies", + ) + + framework_dev_deps = _pinned_args_from_constants( + constants.PackageJson.DEV_DEPENDENCIES + ) + if framework_dev_deps: + run_package_manager( + [ + primary_package_manager, + "add", + "--legacy-peer-deps", + "-d", + *framework_dev_deps, + ], + show_status_message="Pinning framework frontend dev dependencies", + ) + development_deps: set[str] = set() for plugin in config.plugins: development_deps.update(plugin.get_frontend_development_dependencies()) packages.update(plugin.get_frontend_dependencies()) - if development_deps: + pinned_dev_deps, unpinned_dev_deps = _split_by_version_specifier(development_deps) + if pinned_dev_deps: run_package_manager( [ primary_package_manager, "add", "--legacy-peer-deps", "-d", - *development_deps, + *pinned_dev_deps, + ], + show_status_message="Pinning frontend development dependencies", + ) + if unpinned_dev_deps: + run_package_manager( + [ + primary_package_manager, + "add", + "--legacy-peer-deps", + "--only-missing", + "-d", + *unpinned_dev_deps, ], show_status_message="Installing frontend development dependencies", ) - # Install custom packages defined in frontend_packages - if packages: + pinned_packages, unpinned_packages = _split_by_version_specifier(packages) + if pinned_packages: run_package_manager( - [primary_package_manager, "add", "--legacy-peer-deps", *packages], + [primary_package_manager, "add", "--legacy-peer-deps", *pinned_packages], + show_status_message="Pinning frontend packages from config and components", + ) + if unpinned_packages: + run_package_manager( + [ + primary_package_manager, + "add", + "--legacy-peer-deps", + "--only-missing", + *unpinned_packages, + ], show_status_message="Installing frontend packages from config and components", ) @@ -464,3 +594,4 @@ def install_frontend_packages(packages: set[str], config: Config): _sync_root_bun_lock_for_frontend_install() _install_frontend_packages(set(packages), config, install_package_managers) frontend_skeleton.sync_web_bun_lock_to_root() + frontend_skeleton.sync_web_package_json_to_root() diff --git a/tests/units/test_prerequisites.py b/tests/units/test_prerequisites.py index 7ca16a11071..116c0c64bbb 100644 --- a/tests/units/test_prerequisites.py +++ b/tests/units/test_prerequisites.py @@ -1,3 +1,4 @@ +import json import shutil import tempfile from collections.abc import Callable, Generator @@ -58,11 +59,19 @@ class InstallPackagesEnv: web_dir: Path root_lock: Path web_lock: Path + root_package_json: Path + web_package_json: Path config: Config patch_pm: Callable[[list[str], Callable], None] install: _InstallFn +def _stub_framework_packages(monkeypatch: pytest.MonkeyPatch) -> None: + """Empty out framework deps so install call counts are predictable.""" + monkeypatch.setattr(constants.PackageJson, "DEPENDENCIES", {}) + monkeypatch.setattr(constants.PackageJson, "DEV_DEPENDENCIES", {}) + + @pytest.fixture def install_packages_env( tmp_path, monkeypatch @@ -70,8 +79,10 @@ def install_packages_env( """Isolated environment for install_frontend_packages tests. Creates the web dir, patches get_web_dir, chdirs into tmp_path, and - exposes the bun lock paths, a Config, a package-manager patch helper, - and a runner that invokes install_frontend_packages. + exposes the bun lock and package.json paths, a Config, a + package-manager patch helper, and a runner that invokes + install_frontend_packages. Framework dep constants are emptied so + tests can reason about exactly the calls they trigger. Yields: An InstallPackagesEnv with paths, config, and patch_pm/install helpers. @@ -79,6 +90,7 @@ def install_packages_env( web_dir = tmp_path / constants.Dirs.WEB web_dir.mkdir() _patch_web_dir(monkeypatch, web_dir) + _stub_framework_packages(monkeypatch) config = Config(app_name="test") def patch_pm(package_managers: list[str], run_package_manager: Callable) -> None: @@ -89,13 +101,15 @@ def patch_pm(package_managers: list[str], run_package_manager: Callable) -> None def install(packages: set[str] | None = None) -> None: js_runtimes.install_frontend_packages(packages or set(), config) - root_lock = tmp_path / constants.Bun.ROOT_LOCKFILE_DIR / constants.Bun.LOCKFILE_PATH - root_lock.parent.mkdir(parents=True, exist_ok=True) + root_lock_dir = tmp_path / constants.Bun.ROOT_LOCKFILE_DIR + root_lock_dir.mkdir(parents=True, exist_ok=True) env = InstallPackagesEnv( tmp_path=tmp_path, web_dir=web_dir, - root_lock=root_lock, + root_lock=root_lock_dir / constants.Bun.LOCKFILE_PATH, web_lock=web_dir / constants.Bun.LOCKFILE_PATH, + root_package_json=root_lock_dir / constants.PackageJson.PATH, + web_package_json=web_dir / constants.PackageJson.PATH, config=config, patch_pm=patch_pm, install=install, @@ -339,6 +353,159 @@ def run_package_manager(args, **kwargs): assert env.web_lock.read_text() == "root-lock" +def _record_calls(env: InstallPackagesEnv) -> list[list[str]]: + """Record `bun add`/`bun install` invocations into a list. + + Args: + env: The install_packages_env fixture instance. + + Returns: + A list that is appended to (in-place) on each package manager call. + """ + calls: list[list[str]] = [] + + def run_package_manager(args, **kwargs): + calls.append(list(args)) + + env.patch_pm(["bun"], run_package_manager) + return calls + + +def test_install_frontend_packages_pinned_packages_omit_only_missing( + install_packages_env: InstallPackagesEnv, +): + env = install_packages_env + calls = _record_calls(env) + + env.install({"some-pkg@1.2.3", "@scope/pkg@4.5.6"}) + + add_calls = [c for c in calls if "add" in c] + assert len(add_calls) == 1 + pinned_call = add_calls[0] + assert "--only-missing" not in pinned_call + assert "some-pkg@1.2.3" in pinned_call + assert "@scope/pkg@4.5.6" in pinned_call + + +def test_install_frontend_packages_unpinned_packages_use_only_missing( + install_packages_env: InstallPackagesEnv, +): + env = install_packages_env + calls = _record_calls(env) + + env.install({"some-pkg", "@scope/pkg"}) + + add_calls = [c for c in calls if "add" in c] + assert len(add_calls) == 1 + unpinned_call = add_calls[0] + assert "--only-missing" in unpinned_call + assert "some-pkg" in unpinned_call + assert "@scope/pkg" in unpinned_call + + +def test_install_frontend_packages_splits_pinned_and_unpinned( + install_packages_env: InstallPackagesEnv, +): + env = install_packages_env + calls = _record_calls(env) + + env.install({"pinned@1.0.0", "unpinned"}) + + add_calls = [c for c in calls if "add" in c] + assert len(add_calls) == 2 + + pinned_call = next(c for c in add_calls if "--only-missing" not in c) + unpinned_call = next(c for c in add_calls if "--only-missing" in c) + assert "pinned@1.0.0" in pinned_call + assert "unpinned" in unpinned_call + assert "unpinned" not in pinned_call + assert "pinned@1.0.0" not in unpinned_call + + +def test_install_frontend_packages_pins_framework_dependencies( + install_packages_env: InstallPackagesEnv, + monkeypatch, +): + env = install_packages_env + monkeypatch.setattr( + constants.PackageJson, "DEPENDENCIES", {"react": "19.2.5", "isbot": "5.1.39"} + ) + monkeypatch.setattr(constants.PackageJson, "DEV_DEPENDENCIES", {"vite": "8.0.9"}) + calls = _record_calls(env) + + env.install() + + add_calls = [c for c in calls if "add" in c] + pin_deps_call = next(c for c in add_calls if "react@19.2.5" in c) + assert "isbot@5.1.39" in pin_deps_call + assert "-d" not in pin_deps_call + assert "--only-missing" not in pin_deps_call + + pin_dev_deps_call = next(c for c in add_calls if "vite@8.0.9" in c) + assert "-d" in pin_dev_deps_call + assert "--only-missing" not in pin_dev_deps_call + + +def test_install_frontend_packages_persists_package_json_to_root( + install_packages_env: InstallPackagesEnv, +): + env = install_packages_env + + def run_package_manager(args, **kwargs): + env.web_package_json.write_text('{"name": "reflex", "dependencies": {}}') + + env.patch_pm(["bun"], run_package_manager) + env.install() + + assert env.root_package_json.read_text() == ( + '{"name": "reflex", "dependencies": {}}' + ) + + +def test_compile_package_json_recovers_dependencies(tmp_path, monkeypatch): + """_compile_package_json should restore deps/devDeps from reflex.lock.""" + root_pkg = tmp_path / constants.Bun.ROOT_LOCKFILE_DIR / constants.PackageJson.PATH + root_pkg.parent.mkdir(parents=True, exist_ok=True) + root_pkg.write_text( + '{"name": "reflex", "type": "module", "scripts": {"old": "x"}, ' + '"dependencies": {"react": "19.2.5"}, ' + '"devDependencies": {"vite": "8.0.9"}, ' + '"overrides": {"old-override": "1.0"}}' + ) + monkeypatch.setattr( + constants.PackageJson, + "OVERRIDES", + {"cookie": "1.1.1"}, + ) + + with chdir(tmp_path): + rendered = json.loads(frontend_skeleton._compile_package_json()) + + assert rendered["dependencies"] == {"react": "19.2.5"} + assert rendered["devDependencies"] == {"vite": "8.0.9"} + assert rendered["overrides"] == {"cookie": "1.1.1"} + assert rendered["scripts"] == { + "dev": constants.PackageJson.Commands.DEV, + "export": constants.PackageJson.Commands.EXPORT, + } + + +def test_compile_package_json_no_persisted_starts_empty(tmp_path, monkeypatch): + """Without a persisted file, deps/devDeps are empty.""" + monkeypatch.setattr( + constants.PackageJson, + "OVERRIDES", + {"cookie": "1.1.1"}, + ) + + with chdir(tmp_path): + rendered = json.loads(frontend_skeleton._compile_package_json()) + + assert rendered["dependencies"] == {} + assert rendered["devDependencies"] == {} + assert rendered["overrides"] == {"cookie": "1.1.1"} + + def test_cached_procedure(): call_count = 0 From 0759bbaf64d3523c74b8745f11a58bb83c9b9d31 Mon Sep 17 00:00:00 2001 From: Masen Furer Date: Fri, 8 May 2026 12:41:35 -0700 Subject: [PATCH 04/14] remove stale packages from reflex.lock/packages.json now that packages.json isn't being rewritten on each init, it is important to explicitly look for referenced top-level packages that are no longer used in the current app and remove them. --- reflex/utils/frontend_skeleton.py | 21 +++--- reflex/utils/js_runtimes.py | 83 +++++++++++++++++++++-- tests/units/test_prerequisites.py | 107 ++++++++++++++++++++++++++++-- 3 files changed, 193 insertions(+), 18 deletions(-) diff --git a/reflex/utils/frontend_skeleton.py b/reflex/utils/frontend_skeleton.py index 4980a41acd3..70003cebba3 100644 --- a/reflex/utils/frontend_skeleton.py +++ b/reflex/utils/frontend_skeleton.py @@ -338,20 +338,25 @@ def _compile_package_json(): Recovers ``dependencies`` and ``devDependencies`` from the persisted ``reflex.lock/package.json`` (when present) so resolved version pins - survive a fresh ``reflex init``. ``scripts`` and ``overrides`` are always - refreshed from constants. The framework-managed entries in - ``constants.PackageJson.DEPENDENCIES`` / ``DEV_DEPENDENCIES`` are added - later at install time via ``bun add`` so they pick up strict pins. + survive a fresh ``reflex init``. User-added ``scripts`` are preserved; + only the framework-owned ``dev`` and ``export`` entries are refreshed + from constants. ``overrides`` are always refreshed. The framework-managed + entries in ``constants.PackageJson.DEPENDENCIES`` / ``DEV_DEPENDENCIES`` + are added later at install time via ``bun add`` so they pick up strict + pins. Returns: Rendered package.json content as string. """ persisted = _read_persisted_package_json() + persisted_scripts = persisted.get("scripts") or {} + scripts = { + **persisted_scripts, + "dev": constants.PackageJson.Commands.DEV, + "export": constants.PackageJson.Commands.EXPORT, + } return templates.package_json_template( - scripts={ - "dev": constants.PackageJson.Commands.DEV, - "export": constants.PackageJson.Commands.EXPORT, - }, + scripts=scripts, dependencies=persisted.get("dependencies") or {}, dev_dependencies=persisted.get("devDependencies") or {}, overrides=constants.PackageJson.OVERRIDES, diff --git a/reflex/utils/js_runtimes.py b/reflex/utils/js_runtimes.py index c38a4ead4dc..30800add2d6 100644 --- a/reflex/utils/js_runtimes.py +++ b/reflex/utils/js_runtimes.py @@ -1,9 +1,10 @@ """This module provides utilities for managing JavaScript runtimes like Node.js and Bun.""" import functools +import json import os import tempfile -from collections.abc import Sequence +from collections.abc import Iterable, Sequence from pathlib import Path from packaging import version @@ -385,6 +386,53 @@ def _sync_root_bun_lock_for_frontend_install(): path_ops.rm(cache_file) +def _extract_package_name(package_spec: str) -> str: + """Strip any version suffix from a ``bun add``-style spec. + + Handles plain (``react``), pinned (``react@1.2.3``), scoped + (``@scope/pkg``), and pinned-scoped (``@scope/pkg@1.2.3``) forms. + + Args: + package_spec: The spec to parse. + + Returns: + The bare package name. + """ + if package_spec.startswith("@"): + idx = package_spec.find("@", 1) + return package_spec if idx == -1 else package_spec[:idx] + return package_spec.split("@", 1)[0] + + +def _stale_packages_in_web_package_json(needed_names: Iterable[str]) -> set[str]: + """Return packages currently in .web/package.json that are no longer needed. + + Reads ``.web/package.json``'s ``dependencies`` and ``devDependencies`` + and returns any name that is not in ``needed_names``. ``overrides`` are + excluded because they reference transitive deps. + + Args: + needed_names: Bare package names that should remain in package.json. + + Returns: + The set of bare package names that should be removed. + """ + web_pkg_json_path = frontend_skeleton.get_web_package_json_path() + if not web_pkg_json_path.exists(): + return set() + try: + data = json.loads(web_pkg_json_path.read_text()) + except (json.JSONDecodeError, OSError) as e: + console.warn( + f"Failed to read {web_pkg_json_path}: {e}; skipping stale package check." + ) + return set() + current = set(data.get("dependencies") or {}) | set( + data.get("devDependencies") or {} + ) + return current - set(needed_names) + + def _has_version_specifier(package_spec: str) -> bool: """Check whether a package spec already includes a version specifier. @@ -508,6 +556,34 @@ def _install_frontend_packages( env=env, ) + # Resolve plugin-contributed deps up front so we know the full needed + # set before deciding which entries in package.json are stale. + development_deps: set[str] = set() + for plugin in config.plugins: + development_deps.update(plugin.get_frontend_development_dependencies()) + packages.update(plugin.get_frontend_dependencies()) + + needed_names = ( + set(constants.PackageJson.DEPENDENCIES.keys()) + | set(constants.PackageJson.DEV_DEPENDENCIES.keys()) + | {_extract_package_name(p) for p in packages} + | {_extract_package_name(p) for p in development_deps} + ) + + # Drop any deps lingering in package.json that no component, plugin, or + # framework constant calls for anymore. + stale_packages = _stale_packages_in_web_package_json(needed_names) + if stale_packages: + run_package_manager( + [ + primary_package_manager, + "remove", + "--legacy-peer-deps", + *sorted(stale_packages), + ], + show_status_message="Removing unused frontend packages", + ) + # Install whatever was recovered into package.json from reflex.lock so # the lockfile is honored before any further mutation. run_package_manager( @@ -537,11 +613,6 @@ def _install_frontend_packages( show_status_message="Pinning framework frontend dev dependencies", ) - development_deps: set[str] = set() - for plugin in config.plugins: - development_deps.update(plugin.get_frontend_development_dependencies()) - packages.update(plugin.get_frontend_dependencies()) - pinned_dev_deps, unpinned_dev_deps = _split_by_version_specifier(development_deps) if pinned_dev_deps: run_package_manager( diff --git a/tests/units/test_prerequisites.py b/tests/units/test_prerequisites.py index 116c0c64bbb..06472b92beb 100644 --- a/tests/units/test_prerequisites.py +++ b/tests/units/test_prerequisites.py @@ -484,10 +484,9 @@ def test_compile_package_json_recovers_dependencies(tmp_path, monkeypatch): assert rendered["dependencies"] == {"react": "19.2.5"} assert rendered["devDependencies"] == {"vite": "8.0.9"} assert rendered["overrides"] == {"cookie": "1.1.1"} - assert rendered["scripts"] == { - "dev": constants.PackageJson.Commands.DEV, - "export": constants.PackageJson.Commands.EXPORT, - } + assert rendered["scripts"]["dev"] == constants.PackageJson.Commands.DEV + assert rendered["scripts"]["export"] == constants.PackageJson.Commands.EXPORT + assert rendered["scripts"]["old"] == "x" def test_compile_package_json_no_persisted_starts_empty(tmp_path, monkeypatch): @@ -506,6 +505,106 @@ def test_compile_package_json_no_persisted_starts_empty(tmp_path, monkeypatch): assert rendered["overrides"] == {"cookie": "1.1.1"} +def test_compile_package_json_preserves_user_scripts(tmp_path): + """User-added scripts are preserved; only dev/export are refreshed.""" + root_pkg = tmp_path / constants.Bun.ROOT_LOCKFILE_DIR / constants.PackageJson.PATH + root_pkg.parent.mkdir(parents=True, exist_ok=True) + root_pkg.write_text( + json.dumps({ + "scripts": { + "dev": "stale-dev", + "export": "stale-export", + "lint": "eslint .", + "custom": "echo hi", + }, + "dependencies": {}, + "devDependencies": {}, + }) + ) + + with chdir(tmp_path): + rendered = json.loads(frontend_skeleton._compile_package_json()) + + assert rendered["scripts"]["lint"] == "eslint ." + assert rendered["scripts"]["custom"] == "echo hi" + assert rendered["scripts"]["dev"] == constants.PackageJson.Commands.DEV + assert rendered["scripts"]["export"] == constants.PackageJson.Commands.EXPORT + + +def test_install_frontend_packages_removes_stale_dependencies( + install_packages_env: InstallPackagesEnv, +): + env = install_packages_env + env.web_package_json.write_text( + json.dumps({ + "dependencies": { + "still-needed": "1.0.0", + "stale-dep": "2.0.0", + }, + "devDependencies": { + "stale-dev-dep": "3.0.0", + }, + }) + ) + calls = _record_calls(env) + + env.install({"still-needed"}) + + remove_calls = [c for c in calls if "remove" in c] + assert len(remove_calls) == 1 + remove_call = remove_calls[0] + assert "stale-dep" in remove_call + assert "stale-dev-dep" in remove_call + assert "still-needed" not in remove_call + + +def test_install_frontend_packages_no_remove_when_all_needed( + install_packages_env: InstallPackagesEnv, +): + env = install_packages_env + env.web_package_json.write_text( + json.dumps({"dependencies": {"keep-me": "1.0.0"}, "devDependencies": {}}) + ) + calls = _record_calls(env) + + env.install({"keep-me"}) + + remove_calls = [c for c in calls if "remove" in c] + assert remove_calls == [] + + +def test_install_frontend_packages_keeps_framework_deps_during_remove( + install_packages_env: InstallPackagesEnv, + monkeypatch, +): + env = install_packages_env + monkeypatch.setattr(constants.PackageJson, "DEPENDENCIES", {"react": "19.2.5"}) + monkeypatch.setattr(constants.PackageJson, "DEV_DEPENDENCIES", {"vite": "8.0.9"}) + env.web_package_json.write_text( + json.dumps({ + "dependencies": {"react": "19.2.5", "stale-dep": "1.0.0"}, + "devDependencies": {"vite": "8.0.9"}, + }) + ) + calls = _record_calls(env) + + env.install() + + remove_calls = [c for c in calls if "remove" in c] + assert len(remove_calls) == 1 + remove_call = remove_calls[0] + assert "stale-dep" in remove_call + assert "react" not in remove_call + assert "vite" not in remove_call + + +def test_extract_package_name(): + assert js_runtimes._extract_package_name("react") == "react" + assert js_runtimes._extract_package_name("react@1.2.3") == "react" + assert js_runtimes._extract_package_name("@scope/pkg") == "@scope/pkg" + assert js_runtimes._extract_package_name("@scope/pkg@1.2.3") == "@scope/pkg" + + def test_cached_procedure(): call_count = 0 From 1d2c6858876d9b7422e1c27dc944024f1203ac49 Mon Sep 17 00:00:00 2001 From: Masen Furer Date: Fri, 8 May 2026 12:56:59 -0700 Subject: [PATCH 05/14] consolidate bun install / bun add commands Avoid extra invocations of the package manager. --- reflex/utils/js_runtimes.py | 55 ++++++++++++++----------------------- 1 file changed, 21 insertions(+), 34 deletions(-) diff --git a/reflex/utils/js_runtimes.py b/reflex/utils/js_runtimes.py index 30800add2d6..40f397535bd 100644 --- a/reflex/utils/js_runtimes.py +++ b/reflex/utils/js_runtimes.py @@ -591,37 +591,42 @@ def _install_frontend_packages( show_status_message="Installing base frontend packages", ) - framework_deps = _pinned_args_from_constants(constants.PackageJson.DEPENDENCIES) - if framework_deps: - run_package_manager( - [primary_package_manager, "add", "--legacy-peer-deps", *framework_deps], - show_status_message="Pinning framework frontend dependencies", - ) + pinned_packages, unpinned_packages = _split_by_version_specifier(packages) + pinned_dev_deps, unpinned_dev_deps = _split_by_version_specifier(development_deps) - framework_dev_deps = _pinned_args_from_constants( - constants.PackageJson.DEV_DEPENDENCIES + all_pinned = ( + _pinned_args_from_constants(constants.PackageJson.DEPENDENCIES) + | pinned_packages ) - if framework_dev_deps: + all_pinned_dev = ( + _pinned_args_from_constants(constants.PackageJson.DEV_DEPENDENCIES) + | pinned_dev_deps + ) + + if all_pinned: + run_package_manager( + [primary_package_manager, "add", "--legacy-peer-deps", *all_pinned], + show_status_message="Pinning frontend packages", + ) + if unpinned_packages: run_package_manager( [ primary_package_manager, "add", "--legacy-peer-deps", - "-d", - *framework_dev_deps, + "--only-missing", + *unpinned_packages, ], - show_status_message="Pinning framework frontend dev dependencies", + show_status_message="Installing frontend packages", ) - - pinned_dev_deps, unpinned_dev_deps = _split_by_version_specifier(development_deps) - if pinned_dev_deps: + if all_pinned_dev: run_package_manager( [ primary_package_manager, "add", "--legacy-peer-deps", "-d", - *pinned_dev_deps, + *all_pinned_dev, ], show_status_message="Pinning frontend development dependencies", ) @@ -638,24 +643,6 @@ def _install_frontend_packages( show_status_message="Installing frontend development dependencies", ) - pinned_packages, unpinned_packages = _split_by_version_specifier(packages) - if pinned_packages: - run_package_manager( - [primary_package_manager, "add", "--legacy-peer-deps", *pinned_packages], - show_status_message="Pinning frontend packages from config and components", - ) - if unpinned_packages: - run_package_manager( - [ - primary_package_manager, - "add", - "--legacy-peer-deps", - "--only-missing", - *unpinned_packages, - ], - show_status_message="Installing frontend packages from config and components", - ) - def install_frontend_packages(packages: set[str], config: Config): """Install frontend packages while respecting the canonical root bun.lock.""" From e3d626314f11a1b9727d2348c6762c7edb71db75 Mon Sep 17 00:00:00 2001 From: Masen Furer Date: Fri, 8 May 2026 13:59:31 -0700 Subject: [PATCH 06/14] Support package-lock.json from npm package manager Also sync package-lock.json to reflex.lock directory. --- .../src/reflex_base/constants/installer.py | 4 + reflex/utils/frontend_skeleton.py | 129 +++++++++++++----- reflex/utils/js_runtimes.py | 27 +--- tests/units/test_prerequisites.py | 68 +++++++++ 4 files changed, 171 insertions(+), 57 deletions(-) diff --git a/packages/reflex-base/src/reflex_base/constants/installer.py b/packages/reflex-base/src/reflex_base/constants/installer.py index 8f546b41d13..94f2aad1dd1 100644 --- a/packages/reflex-base/src/reflex_base/constants/installer.py +++ b/packages/reflex-base/src/reflex_base/constants/installer.py @@ -63,6 +63,7 @@ def DEFAULT_PATH(cls): DEFAULT_CONFIG = """ [install] registry = "{registry}" +frozenLockfile = true """ @@ -76,6 +77,9 @@ class Node(SimpleNamespace): # Path of the node config file. CONFIG_PATH = ".npmrc" + # Path of the npm lockfile. + LOCKFILE_PATH = "package-lock.json" + DEFAULT_CONFIG = """ registry={registry} fetch-retries=0 diff --git a/reflex/utils/frontend_skeleton.py b/reflex/utils/frontend_skeleton.py index 70003cebba3..74d865ac51d 100644 --- a/reflex/utils/frontend_skeleton.py +++ b/reflex/utils/frontend_skeleton.py @@ -148,34 +148,44 @@ def initialize_requirements_txt( return False -def get_root_bun_lock_path() -> Path: - """Get the canonical bun lock path in the app root. +#: Lockfiles persisted under ``reflex.lock/`` and mirrored into ``.web``. +#: Both bun and npm flows are covered so projects can be (re)built with +#: either package manager without losing pinned versions. +LOCKFILE_NAMES: tuple[str, ...] = ( + constants.Bun.LOCKFILE_PATH, + constants.Node.LOCKFILE_PATH, +) - The lockfile is stored inside a dedicated directory under the app root so - it cannot collide with a user-managed bun project that lives at the same - level as the Reflex project. This assumes the current working directory - is the Reflex app root. + +def get_root_lockfile_path(filename: str) -> Path: + """Get a persisted lockfile path under the app root's reflex.lock dir. + + Args: + filename: The lockfile basename (e.g. ``bun.lock``, ``package-lock.json``). Returns: - The canonical bun lock path in the app root. + The lockfile path inside ``/reflex.lock/``. """ - return Path.cwd() / constants.Bun.ROOT_LOCKFILE_DIR / constants.Bun.LOCKFILE_PATH + return Path.cwd() / constants.Bun.ROOT_LOCKFILE_DIR / filename -def get_web_bun_lock_path() -> Path: - """Get the mirrored bun lock path in the .web directory. +def get_web_lockfile_path(filename: str) -> Path: + """Get the mirrored lockfile path inside ``.web``. + + Args: + filename: The lockfile basename. Returns: - The mirrored bun lock path in the .web directory. + The lockfile path inside the ``.web`` directory. """ - return get_web_dir() / constants.Bun.LOCKFILE_PATH + return get_web_dir() / filename def get_root_package_json_path() -> Path: """Get the persisted package.json path in the app root. - Stored alongside ``bun.lock`` inside the dedicated lockfile directory so - resolved dependency pins survive a fresh ``reflex init``. + Stored alongside the lockfiles inside ``reflex.lock/`` so resolved + dependency pins survive a fresh ``reflex init``. Returns: The persisted package.json path in the app root. @@ -192,34 +202,81 @@ def get_web_package_json_path() -> Path: return get_web_dir() / constants.PackageJson.PATH -def sync_root_bun_lock_to_web(): - """Mirror the canonical root bun.lock into .web. +def _copy_if_exists(src: Path, dest: Path) -> bool: + """Copy ``src`` to ``dest`` (creating ``dest`` parents as needed). + + Args: + src: The source file. If absent, ``dest`` is removed when present. + dest: The destination file. - If the root lockfile is absent, remove any stale mirrored copy from .web. + Returns: + True if ``dest``'s effective contents changed (created from absence, + overwritten with different bytes, or removed because ``src`` is gone). """ - root_bun_lock_path = get_root_bun_lock_path() - web_bun_lock_path = get_web_bun_lock_path() + if not src.exists(): + if dest.exists(): + console.debug(f"Removing stale {dest}") + path_ops.rm(dest) + return True + return False - if not root_bun_lock_path.exists(): - if web_bun_lock_path.exists(): - console.debug(f"Removing stale {web_bun_lock_path}") - path_ops.rm(web_bun_lock_path) - return + if dest.exists() and dest.read_bytes() == src.read_bytes(): + return False + + changed = dest.exists() + path_ops.mkdir(dest.parent) + console.debug(f"Copying {src} to {dest}") + path_ops.cp(src, dest) + return changed + + +def sync_root_lockfile_to_web(filename: str) -> bool: + """Mirror a single persisted lockfile into ``.web``. - console.debug(f"Copying {root_bun_lock_path} to {web_bun_lock_path}") - path_ops.cp(root_bun_lock_path, web_bun_lock_path) + Args: + filename: The lockfile basename. + + Returns: + True if ``.web``'s copy was meaningfully changed (overwritten with + different bytes or removed because the root copy is gone). Initial + creation does not count as a meaningful change since no install + cache could exist yet. + """ + return _copy_if_exists( + get_root_lockfile_path(filename), get_web_lockfile_path(filename) + ) + + +def sync_root_lockfiles_to_web() -> bool: + """Mirror every persisted lockfile into ``.web``. + Returns: + True if any ``.web`` lockfile was meaningfully changed. + """ + # Materialize results so every lockfile is synced + changed = [sync_root_lockfile_to_web(name) for name in LOCKFILE_NAMES] + return any(changed) + + +def sync_web_lockfile_to_root(filename: str): + """Persist a single ``.web`` lockfile back to the app root. -def sync_web_bun_lock_to_root(): - """Persist the mirrored .web bun.lock back to the app root.""" - web_bun_lock_path = get_web_bun_lock_path() - if not web_bun_lock_path.exists(): + Args: + filename: The lockfile basename. + """ + web = get_web_lockfile_path(filename) + if not web.exists(): return + root = get_root_lockfile_path(filename) + path_ops.mkdir(root.parent) + console.debug(f"Copying {web} to {root}") + path_ops.cp(web, root) + - root_bun_lock_path = get_root_bun_lock_path() - path_ops.mkdir(root_bun_lock_path.parent) - console.debug(f"Copying {web_bun_lock_path} to {root_bun_lock_path}") - path_ops.cp(web_bun_lock_path, root_bun_lock_path) +def sync_web_lockfiles_to_root(): + """Persist every ``.web`` lockfile back to the app root.""" + for name in LOCKFILE_NAMES: + sync_web_lockfile_to_root(name) def sync_web_package_json_to_root(): @@ -268,8 +325,8 @@ def initialize_web_directory(): console.debug(f"Copying {constants.Templates.Dirs.WEB_TEMPLATE} to {get_web_dir()}") path_ops.copy_tree(constants.Templates.Dirs.WEB_TEMPLATE, str(get_web_dir())) - console.debug("Restoring the bun lock file.") - sync_root_bun_lock_to_web() + console.debug("Restoring lockfiles.") + sync_root_lockfiles_to_web() console.debug("Initializing the web directory.") initialize_package_json() diff --git a/reflex/utils/js_runtimes.py b/reflex/utils/js_runtimes.py index 40f397535bd..e6c92d3c244 100644 --- a/reflex/utils/js_runtimes.py +++ b/reflex/utils/js_runtimes.py @@ -363,25 +363,10 @@ def _frontend_packages_cache_path() -> Path: return get_web_dir() / "reflex.install_frontend_packages.cached" -def _sync_root_bun_lock_for_frontend_install(): - """Sync the canonical bun.lock into .web and invalidate the install cache when needed.""" - root_bun_lock_path = frontend_skeleton.get_root_bun_lock_path() - web_bun_lock_path = frontend_skeleton.get_web_bun_lock_path() - cache_file = _frontend_packages_cache_path() - - if not root_bun_lock_path.exists(): - if web_bun_lock_path.exists(): - frontend_skeleton.sync_root_bun_lock_to_web() - if cache_file.exists(): - path_ops.rm(cache_file) - return - - if not web_bun_lock_path.exists(): - frontend_skeleton.sync_root_bun_lock_to_web() - return - - if web_bun_lock_path.read_bytes() != root_bun_lock_path.read_bytes(): - frontend_skeleton.sync_root_bun_lock_to_web() +def _sync_root_lockfiles_for_frontend_install(): + """Sync persisted lockfiles into .web and invalidate the install cache when needed.""" + if frontend_skeleton.sync_root_lockfiles_to_web(): + cache_file = _frontend_packages_cache_path() if cache_file.exists(): path_ops.rm(cache_file) @@ -649,7 +634,7 @@ def install_frontend_packages(packages: set[str], config: Config): install_package_managers = tuple( get_nodejs_compatible_package_managers(raise_on_none=True) ) - _sync_root_bun_lock_for_frontend_install() + _sync_root_lockfiles_for_frontend_install() _install_frontend_packages(set(packages), config, install_package_managers) - frontend_skeleton.sync_web_bun_lock_to_root() + frontend_skeleton.sync_web_lockfiles_to_root() frontend_skeleton.sync_web_package_json_to_root() diff --git a/tests/units/test_prerequisites.py b/tests/units/test_prerequisites.py index 06472b92beb..78c2809ee41 100644 --- a/tests/units/test_prerequisites.py +++ b/tests/units/test_prerequisites.py @@ -598,6 +598,74 @@ def test_install_frontend_packages_keeps_framework_deps_during_remove( assert "vite" not in remove_call +def test_install_frontend_packages_persists_npm_lock( + install_packages_env: InstallPackagesEnv, +): + env = install_packages_env + web_npm_lock = env.web_dir / constants.Node.LOCKFILE_PATH + root_npm_lock = ( + env.tmp_path / constants.Bun.ROOT_LOCKFILE_DIR / constants.Node.LOCKFILE_PATH + ) + + def run_package_manager(args, **kwargs): + web_npm_lock.write_text("npm-lock-content") + + env.patch_pm(["npm"], run_package_manager) + env.install() + + assert root_npm_lock.read_text() == "npm-lock-content" + + +def test_install_frontend_packages_restores_npm_lock( + install_packages_env: InstallPackagesEnv, +): + env = install_packages_env + root_npm_lock = ( + env.tmp_path / constants.Bun.ROOT_LOCKFILE_DIR / constants.Node.LOCKFILE_PATH + ) + root_npm_lock.write_text("persisted-npm-lock") + web_npm_lock = env.web_dir / constants.Node.LOCKFILE_PATH + seen_contents: list[str] = [] + + def run_package_manager(args, **kwargs): + seen_contents.append(web_npm_lock.read_text()) + + env.patch_pm(["npm"], run_package_manager) + env.install() + + assert seen_contents + assert seen_contents[0] == "persisted-npm-lock" + + +def test_install_frontend_packages_npm_lock_change_invalidates_cache( + install_packages_env: InstallPackagesEnv, +): + env = install_packages_env + root_npm_lock = ( + env.tmp_path / constants.Bun.ROOT_LOCKFILE_DIR / constants.Node.LOCKFILE_PATH + ) + root_npm_lock.write_text("npm-lock-v1") + web_npm_lock = env.web_dir / constants.Node.LOCKFILE_PATH + call_count = 0 + + def run_package_manager(args, **kwargs): + nonlocal call_count + call_count += 1 + if root_npm_lock.exists(): + web_npm_lock.write_text(root_npm_lock.read_text()) + + env.patch_pm(["npm"], run_package_manager) + + env.install() + first_run_calls = call_count + env.install() + assert call_count == first_run_calls # cache hit, no extra calls + + root_npm_lock.write_text("npm-lock-v2") + env.install() + assert call_count > first_run_calls # cache invalidated, install ran again + + def test_extract_package_name(): assert js_runtimes._extract_package_name("react") == "react" assert js_runtimes._extract_package_name("react@1.2.3") == "react" From 4b2bce9e499c3159864ec1818ed4191f20dc08f3 Mon Sep 17 00:00:00 2001 From: Masen Furer Date: Fri, 8 May 2026 14:06:05 -0700 Subject: [PATCH 07/14] skip initial frontend package "install" step when missing lockfile The first `install` command only does something if there is an existing package.json / lockfile; in the absense of the lock file, there is nothing to install, so skip it. --- reflex/utils/js_runtimes.py | 18 +++++++---- tests/units/test_prerequisites.py | 52 +++++++++++++++++++++++++------ 2 files changed, 55 insertions(+), 15 deletions(-) diff --git a/reflex/utils/js_runtimes.py b/reflex/utils/js_runtimes.py index e6c92d3c244..949da6c107b 100644 --- a/reflex/utils/js_runtimes.py +++ b/reflex/utils/js_runtimes.py @@ -569,12 +569,18 @@ def _install_frontend_packages( show_status_message="Removing unused frontend packages", ) - # Install whatever was recovered into package.json from reflex.lock so - # the lockfile is honored before any further mutation. - run_package_manager( - [primary_package_manager, "install", "--legacy-peer-deps"], - show_status_message="Installing base frontend packages", - ) + # Install against the recovered lockfile so its pins are honored + # before any further mutation. Skip on brand-new projects where no + # lockfile exists yet — ``frozenLockfile`` would error, and the + # subsequent ``bun add`` calls will generate a fresh lockfile. + if any( + frontend_skeleton.get_web_lockfile_path(name).exists() + for name in frontend_skeleton.LOCKFILE_NAMES + ): + run_package_manager( + [primary_package_manager, "install", "--legacy-peer-deps"], + show_status_message="Installing base frontend packages", + ) pinned_packages, unpinned_packages = _split_by_version_specifier(packages) pinned_dev_deps, unpinned_dev_deps = _split_by_version_specifier(development_deps) diff --git a/tests/units/test_prerequisites.py b/tests/units/test_prerequisites.py index 78c2809ee41..d3a697f04e8 100644 --- a/tests/units/test_prerequisites.py +++ b/tests/units/test_prerequisites.py @@ -254,7 +254,7 @@ def run_package_manager(args, **kwargs): env.web_lock.write_text("generated-lock") env.patch_pm(["bun"], run_package_manager) - env.install() + env.install({"some-pkg@1.0.0"}) assert env.root_lock.read_text() == "generated-lock" @@ -289,11 +289,13 @@ def test_install_frontend_packages_cache_respects_root_bun_lock( ): env = install_packages_env env.root_lock.write_text("lock-v1") - call_count = 0 + install_runs = 0 def run_package_manager(args, **kwargs): - nonlocal call_count - call_count += 1 + nonlocal install_runs + # Count distinct cached-procedure invocations by detecting `install`. + if "install" in args and "add" not in args and "remove" not in args: + install_runs += 1 if env.root_lock.exists(): env.web_lock.write_text(env.root_lock.read_text()) else: @@ -308,7 +310,10 @@ def run_package_manager(args, **kwargs): env.root_lock.unlink() env.install() - assert call_count == 3 + # 4 install() calls minus one cache hit = 3 actual runs. The final run + # sees no root lock so the web copy is removed before the run; the + # initial `bun install` is then skipped, leaving 2 install invocations. + assert install_runs == 2 def test_install_frontend_packages_npm_does_not_create_bogus_bun_lock( @@ -324,9 +329,9 @@ def run_package_manager(args, **kwargs): assert not env.web_lock.exists() env.patch_pm(["npm"], run_package_manager) - env.install() + env.install({"some-pkg@1.0.0"}) - assert call_count == 1 + assert call_count >= 1 assert not env.root_lock.exists() assert not env.web_lock.exists() @@ -455,7 +460,7 @@ def run_package_manager(args, **kwargs): env.web_package_json.write_text('{"name": "reflex", "dependencies": {}}') env.patch_pm(["bun"], run_package_manager) - env.install() + env.install({"some-pkg@1.0.0"}) assert env.root_package_json.read_text() == ( '{"name": "reflex", "dependencies": {}}' @@ -598,6 +603,35 @@ def test_install_frontend_packages_keeps_framework_deps_during_remove( assert "vite" not in remove_call +def test_install_frontend_packages_skips_initial_install_on_fresh_project( + install_packages_env: InstallPackagesEnv, +): + """No lockfile yet → skip ``bun install`` (would fail under frozenLockfile).""" + env = install_packages_env + calls = _record_calls(env) + + env.install({"some-pkg@1.0.0"}) + + install_calls = [c for c in calls if "install" in c and "add" not in c] + assert install_calls == [] + add_calls = [c for c in calls if "add" in c] + assert len(add_calls) == 1 + + +def test_install_frontend_packages_runs_initial_install_when_lockfile_present( + install_packages_env: InstallPackagesEnv, +): + """Lockfile recovered → initial ``bun install`` runs to honor pins.""" + env = install_packages_env + env.root_lock.write_text("persisted-lock") + calls = _record_calls(env) + + env.install({"some-pkg@1.0.0"}) + + install_calls = [c for c in calls if "install" in c and "add" not in c] + assert len(install_calls) == 1 + + def test_install_frontend_packages_persists_npm_lock( install_packages_env: InstallPackagesEnv, ): @@ -611,7 +645,7 @@ def run_package_manager(args, **kwargs): web_npm_lock.write_text("npm-lock-content") env.patch_pm(["npm"], run_package_manager) - env.install() + env.install({"some-pkg@1.0.0"}) assert root_npm_lock.read_text() == "npm-lock-content" From 1a16ed71df1f5baf04efa2a28ba44e3a75fefc27 Mon Sep 17 00:00:00 2001 From: Masen Furer Date: Fri, 8 May 2026 14:39:11 -0700 Subject: [PATCH 08/14] remove frontend package manager fallback To better respect checked-in lock files, do not automatically try using a different package manager which may NOT have a lock file, resulting in unexpected packages being downloaded. --- .../src/reflex_base/constants/installer.py | 1 - reflex/utils/js_runtimes.py | 53 ++++++++++++--- tests/units/test_prerequisites.py | 67 +++++++++++++++++++ 3 files changed, 111 insertions(+), 10 deletions(-) diff --git a/packages/reflex-base/src/reflex_base/constants/installer.py b/packages/reflex-base/src/reflex_base/constants/installer.py index 94f2aad1dd1..99db9bb67d6 100644 --- a/packages/reflex-base/src/reflex_base/constants/installer.py +++ b/packages/reflex-base/src/reflex_base/constants/installer.py @@ -63,7 +63,6 @@ def DEFAULT_PATH(cls): DEFAULT_CONFIG = """ [install] registry = "{registry}" -frozenLockfile = true """ diff --git a/reflex/utils/js_runtimes.py b/reflex/utils/js_runtimes.py index 949da6c107b..0e5d23083e6 100644 --- a/reflex/utils/js_runtimes.py +++ b/reflex/utils/js_runtimes.py @@ -92,15 +92,40 @@ def npm_escape_hatch() -> bool: return environment.REFLEX_USE_NPM.get() +def _persisted_lockfile_implies_npm() -> bool: + """Whether the persisted lockfiles imply the project is npm-managed. + + A project is treated as npm-managed when ``reflex.lock/`` carries an + npm lockfile but no bun lockfile, so committing only ``package-lock.json`` + is enough to opt in without setting ``REFLEX_USE_NPM=1``. + + Returns: + Whether the persisted state implies npm. + """ + root_dir = Path.cwd() / constants.Bun.ROOT_LOCKFILE_DIR + return (root_dir / constants.Node.LOCKFILE_PATH).exists() and not ( + root_dir / constants.Bun.LOCKFILE_PATH + ).exists() + + def prefer_npm_over_bun() -> bool: """Check if npm should be preferred over bun. + Order of precedence: + 1. Windows + OneDrive — always npm (bun is broken there). + 2. ``REFLEX_USE_NPM`` set — honor the explicit value. + 3. Persisted lockfile state — implicit npm if only a npm lock is + present in ``reflex.lock/``. + Returns: If npm should be preferred over bun. """ - return npm_escape_hatch() or ( - constants.IS_WINDOWS and windows_check_onedrive_in_path() - ) + if constants.IS_WINDOWS and windows_check_onedrive_in_path(): + return True + explicit = environment.REFLEX_USE_NPM.getenv() + if explicit is not None: + return explicit + return _persisted_lockfile_implies_npm() def get_nodejs_compatible_package_managers( @@ -530,11 +555,14 @@ def _install_frontend_packages( ) primary_package_manager = install_package_managers[0] - fallbacks = install_package_managers[1:] + # No fallback to a different package manager: switching mid-flow could + # bypass the persisted lockfile (e.g. on a package-integrity failure + # the alternate manager would happily fetch a new version), defeating + # the whole point of pinning. A failure here must surface as a failure. run_package_manager = functools.partial( processes.run_process_with_fallbacks, - fallbacks=fallbacks, + fallbacks=None, analytics_enabled=True, cwd=get_web_dir(), shell=constants.IS_WINDOWS, @@ -570,15 +598,22 @@ def _install_frontend_packages( ) # Install against the recovered lockfile so its pins are honored - # before any further mutation. Skip on brand-new projects where no - # lockfile exists yet — ``frozenLockfile`` would error, and the - # subsequent ``bun add`` calls will generate a fresh lockfile. + # before any further mutation. ``--frozen-lockfile`` ensures bun + # refuses to silently rewrite the lockfile here; npm ignores the + # flag. Skip on brand-new projects where no lockfile exists yet — + # ``--frozen-lockfile`` would error, and the subsequent ``bun add`` + # calls will generate a fresh lockfile. if any( frontend_skeleton.get_web_lockfile_path(name).exists() for name in frontend_skeleton.LOCKFILE_NAMES ): run_package_manager( - [primary_package_manager, "install", "--legacy-peer-deps"], + [ + primary_package_manager, + "install", + "--legacy-peer-deps", + "--frozen-lockfile", + ], show_status_message="Installing base frontend packages", ) diff --git a/tests/units/test_prerequisites.py b/tests/units/test_prerequisites.py index d3a697f04e8..767e835f918 100644 --- a/tests/units/test_prerequisites.py +++ b/tests/units/test_prerequisites.py @@ -700,6 +700,73 @@ def run_package_manager(args, **kwargs): assert call_count > first_run_calls # cache invalidated, install ran again +def test_prefer_npm_over_bun_implicit_from_npm_lock(tmp_path, monkeypatch): + """A persisted package-lock.json with no bun.lock implies npm preference.""" + monkeypatch.delenv("REFLEX_USE_NPM", raising=False) + monkeypatch.setattr(js_runtimes.constants, "IS_WINDOWS", False) + root_dir = tmp_path / constants.Bun.ROOT_LOCKFILE_DIR + root_dir.mkdir(parents=True) + (root_dir / constants.Node.LOCKFILE_PATH).write_text("{}") + + with chdir(tmp_path): + assert js_runtimes.prefer_npm_over_bun() is True + + +def test_prefer_npm_over_bun_implicit_disabled_when_bun_lock_present( + tmp_path, monkeypatch +): + """If both lockfiles are persisted, no implicit npm preference.""" + monkeypatch.delenv("REFLEX_USE_NPM", raising=False) + monkeypatch.setattr(js_runtimes.constants, "IS_WINDOWS", False) + root_dir = tmp_path / constants.Bun.ROOT_LOCKFILE_DIR + root_dir.mkdir(parents=True) + (root_dir / constants.Node.LOCKFILE_PATH).write_text("{}") + (root_dir / constants.Bun.LOCKFILE_PATH).write_text("") + + with chdir(tmp_path): + assert js_runtimes.prefer_npm_over_bun() is False + + +def test_prefer_npm_over_bun_explicit_false_overrides_implicit(tmp_path, monkeypatch): + """REFLEX_USE_NPM=0 overrides the implicit npm-lock-only detection.""" + monkeypatch.setenv("REFLEX_USE_NPM", "0") + monkeypatch.setattr(js_runtimes.constants, "IS_WINDOWS", False) + root_dir = tmp_path / constants.Bun.ROOT_LOCKFILE_DIR + root_dir.mkdir(parents=True) + (root_dir / constants.Node.LOCKFILE_PATH).write_text("{}") + + with chdir(tmp_path): + assert js_runtimes.prefer_npm_over_bun() is False + + +def test_prefer_npm_over_bun_explicit_true_wins(tmp_path, monkeypatch): + """REFLEX_USE_NPM=1 forces npm regardless of persisted state.""" + monkeypatch.setenv("REFLEX_USE_NPM", "1") + monkeypatch.setattr(js_runtimes.constants, "IS_WINDOWS", False) + + with chdir(tmp_path): + assert js_runtimes.prefer_npm_over_bun() is True + + +def test_install_frontend_packages_does_not_fall_back( + install_packages_env: InstallPackagesEnv, +): + """A failing primary package manager surfaces the error, no fallback.""" + env = install_packages_env + env.root_lock.write_text("persisted-lock") + error_message = "primary failed" + + def run_package_manager(args, **kwargs): + # If a fallback ever ran the test would not raise. + assert kwargs.get("fallbacks") is None + raise RuntimeError(error_message) + + env.patch_pm(["bun"], run_package_manager) + + with pytest.raises(RuntimeError, match=error_message): + env.install({"some-pkg@1.0.0"}) + + def test_extract_package_name(): assert js_runtimes._extract_package_name("react") == "react" assert js_runtimes._extract_package_name("react@1.2.3") == "react" From 30529561d7da6d0216281d3089839e41710adea0 Mon Sep 17 00:00:00 2001 From: Masen Furer Date: Fri, 8 May 2026 15:32:29 -0700 Subject: [PATCH 09/14] friendly message for bun frozen lockfile issues --- reflex/utils/js_runtimes.py | 64 ++++++++++++++++++++++---- tests/units/test_prerequisites.py | 75 +++++++++++++++++++++++++++++++ 2 files changed, 130 insertions(+), 9 deletions(-) diff --git a/reflex/utils/js_runtimes.py b/reflex/utils/js_runtimes.py index 0e5d23083e6..c06458fac2e 100644 --- a/reflex/utils/js_runtimes.py +++ b/reflex/utils/js_runtimes.py @@ -443,6 +443,60 @@ def _stale_packages_in_web_package_json(needed_names: Iterable[str]) -> set[str] return current - set(needed_names) +def _run_initial_install(primary_package_manager: str, env: dict) -> None: + """Run the initial frozen-lockfile install with a friendly recovery hint. + + bun reports ``error: lockfile had changes, but lockfile is frozen`` when + the persisted lockfile cannot satisfy the recovered package.json. When + that happens, point the user at ``reflex.lock/package.json`` so they can + delete it and let Reflex regenerate the dep set from scratch on the + next run. + + Args: + primary_package_manager: Path to the package manager executable. + env: Extra environment variables for the subprocess. + + Raises: + SystemExit: If the install fails. The exit message tells the user + how to recover from a frozen-lockfile mismatch when applicable. + """ + args = processes.get_command_with_loglevel([ + primary_package_manager, + "install", + "--legacy-peer-deps", + "--frozen-lockfile", + ]) + process = processes.new_process( + args, + cwd=get_web_dir(), + shell=constants.IS_WINDOWS, + env=env, + ) + logs = processes.show_status( + "Installing base frontend packages", + process, + suppress_errors=True, + ) + if process.returncode == 0: + return + + if any("lockfile had changes, but lockfile is frozen" in line for line in logs): + root_dir = Path.cwd() / constants.Bun.ROOT_LOCKFILE_DIR + console.error( + "The persisted lockfile is out of sync with the recovered " + f"package.json. Delete the [bold]{root_dir}[/bold] directory " + "and rerun so Reflex regenerates it from scratch." + ) + raise SystemExit(1) + + # Replay captured logs so the user can diagnose other failures (mirrors + # show_status's default error path, which we suppressed above). + for line in logs: + console.error(line, end="") + console.error("\nRun with [bold]--loglevel debug[/bold] for the full log.") + raise SystemExit(1) + + def _has_version_specifier(package_spec: str) -> bool: """Check whether a package spec already includes a version specifier. @@ -607,15 +661,7 @@ def _install_frontend_packages( frontend_skeleton.get_web_lockfile_path(name).exists() for name in frontend_skeleton.LOCKFILE_NAMES ): - run_package_manager( - [ - primary_package_manager, - "install", - "--legacy-peer-deps", - "--frozen-lockfile", - ], - show_status_message="Installing base frontend packages", - ) + _run_initial_install(primary_package_manager, env) pinned_packages, unpinned_packages = _split_by_version_specifier(packages) pinned_dev_deps, unpinned_dev_deps = _split_by_version_specifier(development_deps) diff --git a/tests/units/test_prerequisites.py b/tests/units/test_prerequisites.py index 767e835f918..ca5a68c37bf 100644 --- a/tests/units/test_prerequisites.py +++ b/tests/units/test_prerequisites.py @@ -46,6 +46,21 @@ def _patch_frontend_package_manager( run_package_manager, ) + # Forward the initial-install helper through the same stub so tests can + # inspect the install args without mocking subprocess primitives. + def _stub_initial_install(primary_pm, env): + run_package_manager( + [ + primary_pm, + "install", + "--legacy-peer-deps", + "--frozen-lockfile", + ], + show_status_message="Installing base frontend packages", + ) + + monkeypatch.setattr(js_runtimes, "_run_initial_install", _stub_initial_install) + class _InstallFn(Protocol): def __call__(self, packages: set[str] | None = ...) -> None: ... @@ -767,6 +782,66 @@ def run_package_manager(args, **kwargs): env.install({"some-pkg@1.0.0"}) +@pytest.mark.usefixtures("install_packages_env") +def test_run_initial_install_frozen_lockfile_error_helpful_message(monkeypatch, capsys): + """A frozen-lockfile mismatch surfaces a 'delete reflex.lock/package.json' hint.""" + + class _FakeProcess: + returncode = 1 + + monkeypatch.setattr( + js_runtimes.processes, + "new_process", + lambda *args, **kwargs: _FakeProcess(), + ) + monkeypatch.setattr( + js_runtimes.processes, + "show_status", + lambda message, process, suppress_errors=False: [ + "error: lockfile had changes, but lockfile is frozen\n", + ], + ) + + with pytest.raises(SystemExit): + js_runtimes._run_initial_install("bun", env={}) + + captured = capsys.readouterr() + output = captured.out + captured.err + assert "out of sync" in output + # The message points at the whole reflex.lock dir, not just bun.lock, + # so deleting it doesn't accidentally leave behind a package-lock.json. + assert constants.Bun.ROOT_LOCKFILE_DIR in output + assert "bun.lock" in output + assert "package-lock.json" in output + + +@pytest.mark.usefixtures("install_packages_env") +def test_run_initial_install_other_error_replays_logs(monkeypatch, capsys): + """Non-frozen-lockfile failures replay the captured logs.""" + + class _FakeProcess: + returncode = 1 + + monkeypatch.setattr( + js_runtimes.processes, + "new_process", + lambda *args, **kwargs: _FakeProcess(), + ) + monkeypatch.setattr( + js_runtimes.processes, + "show_status", + lambda message, process, suppress_errors=False: [ + "error: network unreachable\n", + ], + ) + + with pytest.raises(SystemExit): + js_runtimes._run_initial_install("bun", env={}) + + captured = capsys.readouterr() + assert "network unreachable" in captured.out + captured.err + + def test_extract_package_name(): assert js_runtimes._extract_package_name("react") == "react" assert js_runtimes._extract_package_name("react@1.2.3") == "react" From 44f5f9daa4359c064d1658799e86339a9140238b Mon Sep 17 00:00:00 2001 From: Masen Furer Date: Fri, 8 May 2026 16:35:42 -0700 Subject: [PATCH 10/14] remove bad test assertion --- tests/units/test_prerequisites.py | 4 ---- 1 file changed, 4 deletions(-) diff --git a/tests/units/test_prerequisites.py b/tests/units/test_prerequisites.py index ca5a68c37bf..0babf5375c4 100644 --- a/tests/units/test_prerequisites.py +++ b/tests/units/test_prerequisites.py @@ -808,11 +808,7 @@ class _FakeProcess: captured = capsys.readouterr() output = captured.out + captured.err assert "out of sync" in output - # The message points at the whole reflex.lock dir, not just bun.lock, - # so deleting it doesn't accidentally leave behind a package-lock.json. assert constants.Bun.ROOT_LOCKFILE_DIR in output - assert "bun.lock" in output - assert "package-lock.json" in output @pytest.mark.usefixtures("install_packages_env") From 891630f82db06b8659e362cd2611e2a620460646 Mon Sep 17 00:00:00 2001 From: Masen Furer Date: Fri, 8 May 2026 17:32:34 -0700 Subject: [PATCH 11/14] Determine whether unpinned packages are missing Avoid adding unpinned packages that are already accounted for, to avoid resetting the version specified in the lock file. --- reflex/utils/js_runtimes.py | 106 +++++++++++---------- tests/units/test_prerequisites.py | 152 ++++++++++++++++++++++++++---- 2 files changed, 190 insertions(+), 68 deletions(-) diff --git a/reflex/utils/js_runtimes.py b/reflex/utils/js_runtimes.py index c06458fac2e..2ebc72fe531 100644 --- a/reflex/utils/js_runtimes.py +++ b/reflex/utils/js_runtimes.py @@ -4,7 +4,7 @@ import json import os import tempfile -from collections.abc import Iterable, Sequence +from collections.abc import Sequence from pathlib import Path from packaging import version @@ -414,18 +414,15 @@ def _extract_package_name(package_spec: str) -> str: return package_spec.split("@", 1)[0] -def _stale_packages_in_web_package_json(needed_names: Iterable[str]) -> set[str]: - """Return packages currently in .web/package.json that are no longer needed. +def _existing_web_package_names() -> set[str]: + """Return packages currently declared in .web/package.json. - Reads ``.web/package.json``'s ``dependencies`` and ``devDependencies`` - and returns any name that is not in ``needed_names``. ``overrides`` are - excluded because they reference transitive deps. - - Args: - needed_names: Bare package names that should remain in package.json. + Reads ``.web/package.json``'s ``dependencies`` and ``devDependencies``. + ``overrides`` are excluded because they reference transitive deps. Returns: - The set of bare package names that should be removed. + The set of bare package names already in package.json. Empty if the + file is missing or unreadable. """ web_pkg_json_path = frontend_skeleton.get_web_package_json_path() if not web_pkg_json_path.exists(): @@ -434,13 +431,26 @@ def _stale_packages_in_web_package_json(needed_names: Iterable[str]) -> set[str] data = json.loads(web_pkg_json_path.read_text()) except (json.JSONDecodeError, OSError) as e: console.warn( - f"Failed to read {web_pkg_json_path}: {e}; skipping stale package check." + f"Failed to read {web_pkg_json_path}: {e}; skipping existing package check." ) return set() - current = set(data.get("dependencies") or {}) | set( - data.get("devDependencies") or {} - ) - return current - set(needed_names) + return set(data.get("dependencies") or {}) | set(data.get("devDependencies") or {}) + + +def _is_bun_package_manager(package_manager: str) -> bool: + """Whether the given package manager path refers to bun. + + bun-specific CLI flags (``--frozen-lockfile``, ``--only-missing``) are + not understood by npm and will fail outright on upcoming npm versions + that reject unknown options, so callers gate those flags on this check. + + Args: + package_manager: Path or bare name of the package manager executable. + + Returns: + Whether the executable is bun. + """ + return Path(package_manager).stem.lower() == "bun" def _run_initial_install(primary_package_manager: str, env: dict) -> None: @@ -460,12 +470,16 @@ def _run_initial_install(primary_package_manager: str, env: dict) -> None: SystemExit: If the install fails. The exit message tells the user how to recover from a frozen-lockfile mismatch when applicable. """ - args = processes.get_command_with_loglevel([ + install_args = [ primary_package_manager, "install", "--legacy-peer-deps", - "--frozen-lockfile", - ]) + ] + if _is_bun_package_manager(primary_package_manager): + # ``--frozen-lockfile`` is bun-only; npm ignores it today and the + # next major rejects unknown flags outright. + install_args.append("--frozen-lockfile") + args = processes.get_command_with_loglevel(install_args) process = processes.new_process( args, cwd=get_web_dir(), @@ -586,9 +600,11 @@ def _install_frontend_packages( existing entry in package.json. * Plugin/custom packages with explicit version specifiers are also added with strict pins. - * Plugin/custom packages without version specifiers are added with - ``--only-missing`` so previously resolved pins in package.json are - preserved across runs. + * Plugin/custom packages without version specifiers are skipped + entirely if package.json already declares them, so previously + resolved pins are preserved across runs without relying on + package-manager-specific flags. Otherwise they are added without + a version so the manager picks one. Args: packages: Custom packages requested by the caller (from @@ -637,9 +653,11 @@ def _install_frontend_packages( | {_extract_package_name(p) for p in development_deps} ) + existing_names = _existing_web_package_names() + # Drop any deps lingering in package.json that no component, plugin, or # framework constant calls for anymore. - stale_packages = _stale_packages_in_web_package_json(needed_names) + stale_packages = existing_names - needed_names if stale_packages: run_package_manager( [ @@ -666,51 +684,37 @@ def _install_frontend_packages( pinned_packages, unpinned_packages = _split_by_version_specifier(packages) pinned_dev_deps, unpinned_dev_deps = _split_by_version_specifier(development_deps) - all_pinned = ( + # Skip unpinned entries that already appear in package.json so the + # package manager doesn't churn the previously resolved version. This + # replaces bun's ``--only-missing`` flag with package-manager-agnostic + # logic that also works on npm. + new_unpinned_packages = unpinned_packages - existing_names + new_unpinned_dev_deps = unpinned_dev_deps - existing_names + + deps_to_add = ( _pinned_args_from_constants(constants.PackageJson.DEPENDENCIES) | pinned_packages + | new_unpinned_packages ) - all_pinned_dev = ( + dev_deps_to_add = ( _pinned_args_from_constants(constants.PackageJson.DEV_DEPENDENCIES) | pinned_dev_deps + | new_unpinned_dev_deps ) - if all_pinned: - run_package_manager( - [primary_package_manager, "add", "--legacy-peer-deps", *all_pinned], - show_status_message="Pinning frontend packages", - ) - if unpinned_packages: + if deps_to_add: run_package_manager( - [ - primary_package_manager, - "add", - "--legacy-peer-deps", - "--only-missing", - *unpinned_packages, - ], + [primary_package_manager, "add", "--legacy-peer-deps", *deps_to_add], show_status_message="Installing frontend packages", ) - if all_pinned_dev: - run_package_manager( - [ - primary_package_manager, - "add", - "--legacy-peer-deps", - "-d", - *all_pinned_dev, - ], - show_status_message="Pinning frontend development dependencies", - ) - if unpinned_dev_deps: + if dev_deps_to_add: run_package_manager( [ primary_package_manager, "add", "--legacy-peer-deps", - "--only-missing", "-d", - *unpinned_dev_deps, + *dev_deps_to_add, ], show_status_message="Installing frontend development dependencies", ) diff --git a/tests/units/test_prerequisites.py b/tests/units/test_prerequisites.py index 0babf5375c4..fb40b45c7a7 100644 --- a/tests/units/test_prerequisites.py +++ b/tests/units/test_prerequisites.py @@ -49,13 +49,11 @@ def _patch_frontend_package_manager( # Forward the initial-install helper through the same stub so tests can # inspect the install args without mocking subprocess primitives. def _stub_initial_install(primary_pm, env): + args = [primary_pm, "install", "--legacy-peer-deps"] + if js_runtimes._is_bun_package_manager(primary_pm): + args.append("--frozen-lockfile") run_package_manager( - [ - primary_pm, - "install", - "--legacy-peer-deps", - "--frozen-lockfile", - ], + args, show_status_message="Installing base frontend packages", ) @@ -391,9 +389,10 @@ def run_package_manager(args, **kwargs): return calls -def test_install_frontend_packages_pinned_packages_omit_only_missing( +def test_install_frontend_packages_pinned_packages_single_call( install_packages_env: InstallPackagesEnv, ): + """All-pinned packages produce a single add call without ``--only-missing``.""" env = install_packages_env calls = _record_calls(env) @@ -407,9 +406,10 @@ def test_install_frontend_packages_pinned_packages_omit_only_missing( assert "@scope/pkg@4.5.6" in pinned_call -def test_install_frontend_packages_unpinned_packages_use_only_missing( +def test_install_frontend_packages_unpinned_packages_single_call( install_packages_env: InstallPackagesEnv, ): + """Unpinned packages are added without ``--only-missing`` when not present.""" env = install_packages_env calls = _record_calls(env) @@ -418,34 +418,101 @@ def test_install_frontend_packages_unpinned_packages_use_only_missing( add_calls = [c for c in calls if "add" in c] assert len(add_calls) == 1 unpinned_call = add_calls[0] - assert "--only-missing" in unpinned_call + assert "--only-missing" not in unpinned_call assert "some-pkg" in unpinned_call assert "@scope/pkg" in unpinned_call -def test_install_frontend_packages_splits_pinned_and_unpinned( +def test_install_frontend_packages_combines_pinned_and_unpinned( install_packages_env: InstallPackagesEnv, ): + """Pinned and unpinned packages are batched into one add call.""" env = install_packages_env calls = _record_calls(env) env.install({"pinned@1.0.0", "unpinned"}) add_calls = [c for c in calls if "add" in c] - assert len(add_calls) == 2 + assert len(add_calls) == 1 + add_call = add_calls[0] + assert "--only-missing" not in add_call + assert "pinned@1.0.0" in add_call + assert "unpinned" in add_call + + +def test_install_frontend_packages_skips_unpinned_already_in_package_json( + install_packages_env: InstallPackagesEnv, +): + """An unpinned package already in package.json is not re-added.""" + env = install_packages_env + env.web_package_json.write_text( + json.dumps({"dependencies": {"already-installed": "2.3.4"}}) + ) + calls = _record_calls(env) + + env.install({"already-installed", "fresh-pkg"}) + + add_calls = [c for c in calls if "add" in c] + assert len(add_calls) == 1 + add_call = add_calls[0] + assert "fresh-pkg" in add_call + assert "already-installed" not in add_call + + +def test_install_frontend_packages_skips_unpinned_dev_dep_already_in_package_json( + install_packages_env: InstallPackagesEnv, + monkeypatch, +): + """An unpinned dev dep already in package.json is not re-added.""" + env = install_packages_env + env.web_package_json.write_text( + json.dumps({ + "devDependencies": { + "already-dev": "1.2.3", + } + }) + ) + + class FakePlugin: + def get_frontend_dependencies(self): + return set() + + def get_frontend_development_dependencies(self): + return {"already-dev", "fresh-dev"} + + monkeypatch.setattr(env.config, "plugins", [FakePlugin()]) + calls = _record_calls(env) + + env.install() + + dev_add_calls = [c for c in calls if "add" in c and "-d" in c] + assert len(dev_add_calls) == 1 + dev_call = dev_add_calls[0] + assert "fresh-dev" in dev_call + assert "already-dev" not in dev_call + + +def test_install_frontend_packages_unpinned_already_present_makes_no_add_call( + install_packages_env: InstallPackagesEnv, +): + """If every requested unpinned package is already present, no add call runs.""" + env = install_packages_env + env.web_package_json.write_text( + json.dumps({"dependencies": {"some-pkg": "1.0.0", "@scope/pkg": "2.0.0"}}) + ) + calls = _record_calls(env) - pinned_call = next(c for c in add_calls if "--only-missing" not in c) - unpinned_call = next(c for c in add_calls if "--only-missing" in c) - assert "pinned@1.0.0" in pinned_call - assert "unpinned" in unpinned_call - assert "unpinned" not in pinned_call - assert "pinned@1.0.0" not in unpinned_call + env.install({"some-pkg", "@scope/pkg"}) + + add_calls = [c for c in calls if "add" in c] + assert add_calls == [] def test_install_frontend_packages_pins_framework_dependencies( install_packages_env: InstallPackagesEnv, monkeypatch, ): + """Framework dep constants are emitted as pinned ``name@version`` specs.""" env = install_packages_env monkeypatch.setattr( constants.PackageJson, "DEPENDENCIES", {"react": "19.2.5", "isbot": "5.1.39"} @@ -466,6 +533,57 @@ def test_install_frontend_packages_pins_framework_dependencies( assert "--only-missing" not in pin_dev_deps_call +def _record_calls_with_pm( + env: InstallPackagesEnv, package_manager: str +) -> list[list[str]]: + """Record package-manager invocations for an arbitrary primary PM. + + Args: + env: The install_packages_env fixture instance. + package_manager: The primary package manager path to patch in. + + Returns: + A list that is appended to (in-place) on each package manager call. + """ + calls: list[list[str]] = [] + + def run_package_manager(args, **kwargs): + calls.append(list(args)) + + env.patch_pm([package_manager], run_package_manager) + return calls + + +def test_install_frontend_packages_npm_skips_frozen_lockfile( + install_packages_env: InstallPackagesEnv, +): + """``--frozen-lockfile`` is bun-only and must not be passed to npm.""" + env = install_packages_env + env.root_lock.write_text("npm-lock") + calls = _record_calls_with_pm(env, "npm") + + env.install({"some-pkg@1.0.0"}) + + install_calls = [c for c in calls if "install" in c] + assert install_calls, "expected an initial `npm install` call" + for call in install_calls: + assert "--frozen-lockfile" not in call + + +def test_install_frontend_packages_bun_keeps_frozen_lockfile( + install_packages_env: InstallPackagesEnv, +): + """Bun still receives ``--frozen-lockfile`` on the initial install.""" + env = install_packages_env + env.root_lock.write_text("bun-lock") + calls = _record_calls_with_pm(env, "bun") + + env.install({"some-pkg"}) + + install_calls = [c for c in calls if "install" in c] + assert any("--frozen-lockfile" in c for c in install_calls) + + def test_install_frontend_packages_persists_package_json_to_root( install_packages_env: InstallPackagesEnv, ): From 3f79167aeab7b74015d582c3f36d8fb32f08bd36 Mon Sep 17 00:00:00 2001 From: Masen Furer Date: Sun, 10 May 2026 12:52:21 -0700 Subject: [PATCH 12/14] Apply suggestion from @masenf --- reflex/utils/js_runtimes.py | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/reflex/utils/js_runtimes.py b/reflex/utils/js_runtimes.py index 2ebc72fe531..d7d723b79eb 100644 --- a/reflex/utils/js_runtimes.py +++ b/reflex/utils/js_runtimes.py @@ -670,11 +670,7 @@ def _install_frontend_packages( ) # Install against the recovered lockfile so its pins are honored - # before any further mutation. ``--frozen-lockfile`` ensures bun - # refuses to silently rewrite the lockfile here; npm ignores the - # flag. Skip on brand-new projects where no lockfile exists yet — - # ``--frozen-lockfile`` would error, and the subsequent ``bun add`` - # calls will generate a fresh lockfile. + # before any further mutation. if any( frontend_skeleton.get_web_lockfile_path(name).exists() for name in frontend_skeleton.LOCKFILE_NAMES From 41d17fcdc4989d0bd254f58a60cdf4772158f89e Mon Sep 17 00:00:00 2001 From: Masen Furer Date: Sun, 10 May 2026 13:13:04 -0700 Subject: [PATCH 13/14] handle the case where a package moves between deps or dev deps --- reflex/utils/js_runtimes.py | 83 ++++++++++++++--------- tests/units/test_prerequisites.py | 109 ++++++++++++++++++++++++++++++ 2 files changed, 160 insertions(+), 32 deletions(-) diff --git a/reflex/utils/js_runtimes.py b/reflex/utils/js_runtimes.py index d7d723b79eb..47384908cb3 100644 --- a/reflex/utils/js_runtimes.py +++ b/reflex/utils/js_runtimes.py @@ -414,27 +414,32 @@ def _extract_package_name(package_spec: str) -> str: return package_spec.split("@", 1)[0] -def _existing_web_package_names() -> set[str]: - """Return packages currently declared in .web/package.json. +def _existing_web_package_sections() -> tuple[set[str], set[str]]: + """Return packages currently declared in .web/package.json by section. - Reads ``.web/package.json``'s ``dependencies`` and ``devDependencies``. - ``overrides`` are excluded because they reference transitive deps. + Reads ``.web/package.json``'s ``dependencies`` and ``devDependencies`` + separately so callers can detect packages declared in the wrong + section. ``overrides`` are excluded because they reference transitive + deps. Returns: - The set of bare package names already in package.json. Empty if the - file is missing or unreadable. + A tuple ``(deps, dev_deps)`` of bare package names. Both empty if + the file is missing or unreadable. """ web_pkg_json_path = frontend_skeleton.get_web_package_json_path() if not web_pkg_json_path.exists(): - return set() + return set(), set() try: data = json.loads(web_pkg_json_path.read_text()) except (json.JSONDecodeError, OSError) as e: console.warn( f"Failed to read {web_pkg_json_path}: {e}; skipping existing package check." ) - return set() - return set(data.get("dependencies") or {}) | set(data.get("devDependencies") or {}) + return set(), set() + return ( + set(data.get("dependencies") or {}), + set(data.get("devDependencies") or {}), + ) def _is_bun_package_manager(package_manager: str) -> bool: @@ -601,10 +606,14 @@ def _install_frontend_packages( * Plugin/custom packages with explicit version specifiers are also added with strict pins. * Plugin/custom packages without version specifiers are skipped - entirely if package.json already declares them, so previously - resolved pins are preserved across runs without relying on - package-manager-specific flags. Otherwise they are added without - a version so the manager picks one. + entirely if package.json already declares them in the correct + section, so previously resolved pins are preserved across runs + without relying on package-manager-specific flags. Otherwise they + are added without a version so the manager picks one. + * Packages declared in the wrong section (e.g. a regular dep + listed under ``devDependencies``) are removed first and re-added + so they land in the section the framework/plugin/import-graph + actually intends. Args: packages: Custom packages requested by the caller (from @@ -646,25 +655,33 @@ def _install_frontend_packages( development_deps.update(plugin.get_frontend_development_dependencies()) packages.update(plugin.get_frontend_dependencies()) - needed_names = ( - set(constants.PackageJson.DEPENDENCIES.keys()) - | set(constants.PackageJson.DEV_DEPENDENCIES.keys()) - | {_extract_package_name(p) for p in packages} - | {_extract_package_name(p) for p in development_deps} - ) - - existing_names = _existing_web_package_names() - - # Drop any deps lingering in package.json that no component, plugin, or - # framework constant calls for anymore. + wanted_dep_names = set(constants.PackageJson.DEPENDENCIES.keys()) | { + _extract_package_name(p) for p in packages + } + wanted_dev_dep_names = set(constants.PackageJson.DEV_DEPENDENCIES.keys()) | { + _extract_package_name(p) for p in development_deps + } + needed_names = wanted_dep_names | wanted_dev_dep_names + + existing_deps, existing_dev_deps = _existing_web_package_sections() + existing_names = existing_deps | existing_dev_deps + + # Drop deps lingering in package.json that no component, plugin, or + # framework constant calls for anymore, plus any package declared in + # the wrong section. bun and npm both update the existing entry + # in-place on a re-add and won't move it across sections, so misplaced + # entries must be removed first to land in the correct one. stale_packages = existing_names - needed_names - if stale_packages: + misplaced_in_dev = (wanted_dep_names & existing_dev_deps) - existing_deps + misplaced_in_deps = (wanted_dev_dep_names & existing_deps) - existing_dev_deps + to_remove = stale_packages | misplaced_in_dev | misplaced_in_deps + if to_remove: run_package_manager( [ primary_package_manager, "remove", "--legacy-peer-deps", - *sorted(stale_packages), + *sorted(to_remove), ], show_status_message="Removing unused frontend packages", ) @@ -680,12 +697,14 @@ def _install_frontend_packages( pinned_packages, unpinned_packages = _split_by_version_specifier(packages) pinned_dev_deps, unpinned_dev_deps = _split_by_version_specifier(development_deps) - # Skip unpinned entries that already appear in package.json so the - # package manager doesn't churn the previously resolved version. This - # replaces bun's ``--only-missing`` flag with package-manager-agnostic - # logic that also works on npm. - new_unpinned_packages = unpinned_packages - existing_names - new_unpinned_dev_deps = unpinned_dev_deps - existing_names + # Skip unpinned entries that already appear in the correct section so + # the package manager doesn't churn the previously resolved version. + # Misplaced entries fall through here and get re-added (after the + # remove step above) into the right section. This replaces bun's + # ``--only-missing`` flag with package-manager-agnostic logic that + # also works on npm. + new_unpinned_packages = unpinned_packages - existing_deps + new_unpinned_dev_deps = unpinned_dev_deps - existing_dev_deps deps_to_add = ( _pinned_args_from_constants(constants.PackageJson.DEPENDENCIES) diff --git a/tests/units/test_prerequisites.py b/tests/units/test_prerequisites.py index fb40b45c7a7..cc8cc29ee62 100644 --- a/tests/units/test_prerequisites.py +++ b/tests/units/test_prerequisites.py @@ -508,6 +508,115 @@ def test_install_frontend_packages_unpinned_already_present_makes_no_add_call( assert add_calls == [] +def test_install_frontend_packages_moves_misplaced_unpinned_dep_to_deps( + install_packages_env: InstallPackagesEnv, +): + """A regular dep currently sitting under devDependencies gets relocated.""" + env = install_packages_env + env.web_package_json.write_text( + json.dumps({"devDependencies": {"some-pkg": "1.2.3"}}) + ) + calls = _record_calls(env) + + env.install({"some-pkg"}) + + remove_calls = [c for c in calls if "remove" in c] + assert len(remove_calls) == 1 + assert "some-pkg" in remove_calls[0] + + add_calls = [c for c in calls if "add" in c] + assert len(add_calls) == 1 + add_call = add_calls[0] + assert "some-pkg" in add_call + assert "-d" not in add_call + + +def test_install_frontend_packages_moves_misplaced_unpinned_dev_dep_to_dev( + install_packages_env: InstallPackagesEnv, + monkeypatch, +): + """A dev dep currently sitting under dependencies gets relocated.""" + env = install_packages_env + env.web_package_json.write_text(json.dumps({"dependencies": {"some-dev": "1.2.3"}})) + + class FakePlugin: + def get_frontend_dependencies(self): + return set() + + def get_frontend_development_dependencies(self): + return {"some-dev"} + + monkeypatch.setattr(env.config, "plugins", [FakePlugin()]) + calls = _record_calls(env) + + env.install() + + remove_calls = [c for c in calls if "remove" in c] + assert len(remove_calls) == 1 + assert "some-dev" in remove_calls[0] + + dev_add_calls = [c for c in calls if "add" in c and "-d" in c] + assert len(dev_add_calls) == 1 + assert "some-dev" in dev_add_calls[0] + + deps_add_calls = [c for c in calls if "add" in c and "-d" not in c] + assert deps_add_calls == [] + + +def test_install_frontend_packages_moves_misplaced_pinned_framework_dep( + install_packages_env: InstallPackagesEnv, + monkeypatch, +): + """A framework dep listed in the wrong section gets relocated and re-pinned.""" + env = install_packages_env + monkeypatch.setattr(constants.PackageJson, "DEPENDENCIES", {"react": "19.2.5"}) + env.web_package_json.write_text( + json.dumps({"devDependencies": {"react": "18.0.0"}}) + ) + calls = _record_calls(env) + + env.install() + + remove_calls = [c for c in calls if "remove" in c] + assert len(remove_calls) == 1 + assert "react" in remove_calls[0] + + add_calls = [c for c in calls if "add" in c] + assert len(add_calls) == 1 + add_call = add_calls[0] + assert "react@19.2.5" in add_call + assert "-d" not in add_call + + +def test_install_frontend_packages_does_not_move_correctly_placed_packages( + install_packages_env: InstallPackagesEnv, + monkeypatch, +): + """Packages already in the right section trigger no remove/add.""" + env = install_packages_env + env.web_package_json.write_text( + json.dumps({ + "dependencies": {"regular": "1.0.0"}, + "devDependencies": {"dev-only": "2.0.0"}, + }) + ) + + class FakePlugin: + def get_frontend_dependencies(self): + return set() + + def get_frontend_development_dependencies(self): + return {"dev-only"} + + monkeypatch.setattr(env.config, "plugins", [FakePlugin()]) + calls = _record_calls(env) + + env.install({"regular"}) + + assert [c for c in calls if "remove" in c] == [] + assert [c for c in calls if "add" in c] == [] + + def test_install_frontend_packages_pins_framework_dependencies( install_packages_env: InstallPackagesEnv, monkeypatch, From cb9cf8db99588bee7336d0c70d404776da197e3f Mon Sep 17 00:00:00 2001 From: Masen Furer Date: Sun, 10 May 2026 13:29:48 -0700 Subject: [PATCH 14/14] prefer dependencies over dev dependencies if package appears in both dependencies are always installed, so it makes sense to ensure any potential runtime dependencies definitely go into the dependencies list. for practical purposes maybe it doesn't matter since there is no existing reflex run mode that does not install dev dependencies. --- reflex/utils/js_runtimes.py | 40 ++++++++++----- tests/units/test_prerequisites.py | 85 +++++++++++++++++++++++++++++++ 2 files changed, 112 insertions(+), 13 deletions(-) diff --git a/reflex/utils/js_runtimes.py b/reflex/utils/js_runtimes.py index 47384908cb3..9d1cc9de557 100644 --- a/reflex/utils/js_runtimes.py +++ b/reflex/utils/js_runtimes.py @@ -658,9 +658,15 @@ def _install_frontend_packages( wanted_dep_names = set(constants.PackageJson.DEPENDENCIES.keys()) | { _extract_package_name(p) for p in packages } - wanted_dev_dep_names = set(constants.PackageJson.DEV_DEPENDENCIES.keys()) | { - _extract_package_name(p) for p in development_deps - } + # If the same package is requested as both a regular and a development + # dependency (e.g. two plugins disagree on the section), prefer the + # regular-dep section. This keeps the placement deterministic instead + # of depending on the order the package manager processes the two + # add calls. + wanted_dev_dep_names = ( + set(constants.PackageJson.DEV_DEPENDENCIES.keys()) + | {_extract_package_name(p) for p in development_deps} + ) - wanted_dep_names needed_names = wanted_dep_names | wanted_dev_dep_names existing_deps, existing_dev_deps = _existing_web_package_sections() @@ -711,17 +717,20 @@ def _install_frontend_packages( | pinned_packages | new_unpinned_packages ) - dev_deps_to_add = ( - _pinned_args_from_constants(constants.PackageJson.DEV_DEPENDENCIES) - | pinned_dev_deps - | new_unpinned_dev_deps - ) - - if deps_to_add: - run_package_manager( - [primary_package_manager, "add", "--legacy-peer-deps", *deps_to_add], - show_status_message="Installing frontend packages", + deps_names_to_add = {_extract_package_name(p) for p in deps_to_add} + dev_deps_to_add = { + spec + for spec in ( + _pinned_args_from_constants(constants.PackageJson.DEV_DEPENDENCIES) + | pinned_dev_deps + | new_unpinned_dev_deps ) + if _extract_package_name(spec) not in deps_names_to_add + } + + # Add dev dependencies first so that any subsequent regular-dep add + # for an overlapping name lands in ``dependencies`` regardless of + # whether the package manager moves entries between sections. if dev_deps_to_add: run_package_manager( [ @@ -733,6 +742,11 @@ def _install_frontend_packages( ], show_status_message="Installing frontend development dependencies", ) + if deps_to_add: + run_package_manager( + [primary_package_manager, "add", "--legacy-peer-deps", *deps_to_add], + show_status_message="Installing frontend packages", + ) def install_frontend_packages(packages: set[str], config: Config): diff --git a/tests/units/test_prerequisites.py b/tests/units/test_prerequisites.py index cc8cc29ee62..edd3140383e 100644 --- a/tests/units/test_prerequisites.py +++ b/tests/units/test_prerequisites.py @@ -588,6 +588,91 @@ def test_install_frontend_packages_moves_misplaced_pinned_framework_dep( assert "-d" not in add_call +def test_install_frontend_packages_conflict_prefers_regular_section( + install_packages_env: InstallPackagesEnv, + monkeypatch, +): + """A package wanted by both deps and dev-deps lands in deps only.""" + env = install_packages_env + + class FakePlugin: + def get_frontend_dependencies(self): + return {"shared-pkg"} + + def get_frontend_development_dependencies(self): + return {"shared-pkg"} + + monkeypatch.setattr(env.config, "plugins", [FakePlugin()]) + calls = _record_calls(env) + + env.install() + + add_calls = [c for c in calls if "add" in c] + assert len(add_calls) == 1 + add_call = add_calls[0] + assert "shared-pkg" in add_call + assert "-d" not in add_call + + +def test_install_frontend_packages_dev_deps_added_before_regular_deps( + install_packages_env: InstallPackagesEnv, + monkeypatch, +): + """When both sections have additions, the dev-deps add runs first.""" + env = install_packages_env + + class FakePlugin: + def get_frontend_dependencies(self): + return set() + + def get_frontend_development_dependencies(self): + return {"some-dev"} + + monkeypatch.setattr(env.config, "plugins", [FakePlugin()]) + calls = _record_calls(env) + + env.install({"some-pkg"}) + + add_calls = [c for c in calls if "add" in c] + assert len(add_calls) == 2 + assert "-d" in add_calls[0] + assert "some-dev" in add_calls[0] + assert "-d" not in add_calls[1] + assert "some-pkg" in add_calls[1] + + +def test_install_frontend_packages_conflict_with_misplaced_existing_entry( + install_packages_env: InstallPackagesEnv, + monkeypatch, +): + """A conflicting name currently in devDeps is removed and re-added to deps.""" + env = install_packages_env + env.web_package_json.write_text( + json.dumps({"devDependencies": {"shared-pkg": "1.0.0"}}) + ) + + class FakePlugin: + def get_frontend_dependencies(self): + return {"shared-pkg"} + + def get_frontend_development_dependencies(self): + return {"shared-pkg"} + + monkeypatch.setattr(env.config, "plugins", [FakePlugin()]) + calls = _record_calls(env) + + env.install() + + remove_calls = [c for c in calls if "remove" in c] + assert len(remove_calls) == 1 + assert "shared-pkg" in remove_calls[0] + + add_calls = [c for c in calls if "add" in c] + assert len(add_calls) == 1 + assert "shared-pkg" in add_calls[0] + assert "-d" not in add_calls[0] + + def test_install_frontend_packages_does_not_move_correctly_placed_packages( install_packages_env: InstallPackagesEnv, monkeypatch,