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..99db9bb67d6 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): @@ -71,6 +76,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/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( diff --git a/reflex/utils/frontend_skeleton.py b/reflex/utils/frontend_skeleton.py index 12054b9a2d3..74d865ac51d 100644 --- a/reflex/utils/frontend_skeleton.py +++ b/reflex/utils/frontend_skeleton.py @@ -148,53 +148,171 @@ 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, +) - 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 lockfile path inside ``/reflex.lock/``. + """ + return Path.cwd() / constants.Bun.ROOT_LOCKFILE_DIR / filename + + +def get_web_lockfile_path(filename: str) -> Path: + """Get the mirrored lockfile path inside ``.web``. + + Args: + filename: The lockfile basename. + + Returns: + The lockfile path inside the ``.web`` directory. + """ + return get_web_dir() / filename + + +def get_root_package_json_path() -> Path: + """Get the persisted package.json path in the app root. + + 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. + """ + 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 _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. Returns: - The canonical bun lock path in the app root. + True if ``dest``'s effective contents changed (created from absence, + overwritten with different bytes, or removed because ``src`` is gone). """ - return Path.cwd() / constants.Bun.LOCKFILE_PATH + if not src.exists(): + if dest.exists(): + console.debug(f"Removing stale {dest}") + path_ops.rm(dest) + return True + return False + + 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 get_web_bun_lock_path() -> Path: - """Get the mirrored bun lock path in the .web directory. +def sync_root_lockfile_to_web(filename: str) -> bool: + """Mirror a single persisted lockfile into ``.web``. + + Args: + filename: The lockfile basename. Returns: - The mirrored bun lock path in the .web directory. + 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 get_web_dir() / constants.Bun.LOCKFILE_PATH + return _copy_if_exists( + get_root_lockfile_path(filename), get_web_lockfile_path(filename) + ) -def sync_root_bun_lock_to_web(): - """Mirror the canonical root bun.lock into .web. +def sync_root_lockfiles_to_web() -> bool: + """Mirror every persisted lockfile into ``.web``. - If the root lockfile is absent, remove any stale mirrored copy from .web. + Returns: + True if any ``.web`` lockfile was meaningfully changed. """ - root_bun_lock_path = get_root_bun_lock_path() - web_bun_lock_path = get_web_bun_lock_path() + # Materialize results so every lockfile is synced + changed = [sync_root_lockfile_to_web(name) for name in LOCKFILE_NAMES] + return any(changed) + - 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) +def sync_web_lockfile_to_root(filename: str): + """Persist a single ``.web`` lockfile back to the app root. + + 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) + + +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) - console.debug(f"Copying {root_bun_lock_path} to {web_bun_lock_path}") - path_ops.cp(root_bun_lock_path, web_bun_lock_path) +def sync_web_package_json_to_root(): + """Persist the resolved .web package.json 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(): + 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_bun_lock_path = get_root_bun_lock_path() - console.debug(f"Copying {web_bun_lock_path} to {root_bun_lock_path}") - path_ops.cp(web_bun_lock_path, root_bun_lock_path) + 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(): @@ -207,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() @@ -273,13 +391,31 @@ 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``. 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, - }, - dependencies=constants.PackageJson.DEPENDENCIES, - dev_dependencies=constants.PackageJson.DEV_DEPENDENCIES, + 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 a4c0cd7a693..9d1cc9de557 100644 --- a/reflex/utils/js_runtimes.py +++ b/reflex/utils/js_runtimes.py @@ -1,6 +1,7 @@ """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 @@ -91,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( @@ -362,34 +388,208 @@ 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() +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) + - 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 +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 _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`` + separately so callers can detect packages declared in the wrong + section. ``overrides`` are excluded because they reference transitive + deps. + + Returns: + 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(), 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(), set() + 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: + """Run the initial frozen-lockfile install with a friendly recovery hint. - if not web_bun_lock_path.exists(): - frontend_skeleton.sync_root_bun_lock_to_web() + 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. + """ + install_args = [ + primary_package_manager, + "install", + "--legacy-peer-deps", + ] + 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(), + shell=constants.IS_WINDOWS, + env=env, + ) + logs = processes.show_status( + "Installing base frontend packages", + process, + suppress_errors=True, + ) + if process.returncode == 0: return - if web_bun_lock_path.read_bytes() != root_bun_lock_path.read_bytes(): - frontend_skeleton.sync_root_bun_lock_to_web() - if cache_file.exists(): - path_ops.rm(cache_file) + 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. + + 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 +598,32 @@ 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 skipped + 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: 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 = ( { @@ -415,44 +634,118 @@ 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, env=env, ) - run_package_manager( - [primary_package_manager, "install", "--legacy-peer-deps"], - show_status_message="Installing base frontend packages", - ) - + # 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()) - if development_deps: + wanted_dep_names = set(constants.PackageJson.DEPENDENCIES.keys()) | { + _extract_package_name(p) for p in packages + } + # 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() + 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 + 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(to_remove), + ], + show_status_message="Removing unused frontend packages", + ) + + # Install against the recovered lockfile so its pins are honored + # before any further mutation. + if any( + frontend_skeleton.get_web_lockfile_path(name).exists() + for name in frontend_skeleton.LOCKFILE_NAMES + ): + _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) + + # 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) + | pinned_packages + | new_unpinned_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( [ primary_package_manager, "add", "--legacy-peer-deps", "-d", - *development_deps, + *dev_deps_to_add, ], show_status_message="Installing frontend development dependencies", ) - - # Install custom packages defined in frontend_packages - if packages: + if deps_to_add: run_package_manager( - [primary_package_manager, "add", "--legacy-peer-deps", *packages], - show_status_message="Installing frontend packages from config and components", + [primary_package_manager, "add", "--legacy-peer-deps", *deps_to_add], + show_status_message="Installing frontend packages", ) @@ -461,6 +754,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 0fc6321e3d4..edd3140383e 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 @@ -45,6 +46,19 @@ 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): + args = [primary_pm, "install", "--legacy-peer-deps"] + if js_runtimes._is_bun_package_manager(primary_pm): + args.append("--frozen-lockfile") + run_package_manager( + args, + 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: ... @@ -58,11 +72,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 +92,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 +103,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,11 +114,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_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=tmp_path / constants.Bun.LOCKFILE_PATH, + 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, @@ -200,7 +229,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): @@ -236,7 +267,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" @@ -271,11 +302,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: @@ -290,7 +323,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( @@ -306,9 +342,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() @@ -335,6 +371,792 @@ 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_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) + + 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_single_call( + install_packages_env: InstallPackagesEnv, +): + """Unpinned packages are added without ``--only-missing`` when not present.""" + 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" not in unpinned_call + assert "some-pkg" in unpinned_call + assert "@scope/pkg" in unpinned_call + + +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) == 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) + + 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_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_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, +): + """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, +): + """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"} + ) + 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 _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, +): + 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({"some-pkg@1.0.0"}) + + 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 + 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): + """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_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_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, +): + 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({"some-pkg@1.0.0"}) + + 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_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"}) + + +@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 + assert constants.Bun.ROOT_LOCKFILE_DIR 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" + 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