diff --git a/news/6658.bugfix.md b/news/6658.bugfix.md new file mode 100644 index 00000000000..af20adbe428 --- /dev/null +++ b/news/6658.bugfix.md @@ -0,0 +1 @@ +Sync `reflex.lock/package.json` to `.web/package.json` before installing packages to ensure lock file and package.json are aligned. \ No newline at end of file diff --git a/packages/reflex-base/news/6658.feature.md b/packages/reflex-base/news/6658.feature.md new file mode 100644 index 00000000000..6c32bf8775f --- /dev/null +++ b/packages/reflex-base/news/6658.feature.md @@ -0,0 +1 @@ +`package_json_template` accepts `**additional_keys` to include extra fields (e.g. `name`, `packageManager`, `engines`) in the rendered package.json. diff --git a/packages/reflex-base/src/reflex_base/compiler/templates.py b/packages/reflex-base/src/reflex_base/compiler/templates.py index a380aaac4b9..8e6e63a2bb3 100644 --- a/packages/reflex-base/src/reflex_base/compiler/templates.py +++ b/packages/reflex-base/src/reflex_base/compiler/templates.py @@ -516,6 +516,7 @@ def package_json_template( dependencies: dict[str, str], dev_dependencies: dict[str, str], overrides: dict[str, str], + **additional_keys: Any, ): """Template for package.json. @@ -524,17 +525,21 @@ def package_json_template( dependencies: The dependencies to include in the package.json file. dev_dependencies: The devDependencies to include in the package.json file. overrides: The overrides to include in the package.json file. + additional_keys: Additional keys to include in the package.json file. Returns: Rendered package.json content as string. """ + # Ensure "type" is not duplicated since it's always set to "module" + additional_keys.pop("type", None) return json.dumps({ - "name": "reflex", + "name": additional_keys.pop("name", "reflex"), "type": "module", "scripts": scripts, "dependencies": dependencies, "devDependencies": dev_dependencies, "overrides": overrides, + **additional_keys, }) diff --git a/reflex/utils/frontend_skeleton.py b/reflex/utils/frontend_skeleton.py index 966e14e2686..de9f29e7bbd 100644 --- a/reflex/utils/frontend_skeleton.py +++ b/reflex/utils/frontend_skeleton.py @@ -271,6 +271,7 @@ def initialize_requirements_txt( constants.Bun.LOCKFILE_PATH, constants.Node.LOCKFILE_PATH, ) +NO_PRUNE_LOCKFILE_NAMES: tuple[str, ...] = (constants.PackageJson.PATH,) def get_root_lockfile_path(filename: str) -> Path: @@ -297,40 +298,20 @@ def get_web_lockfile_path(filename: str) -> 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 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: +def _copy_if_exists(src: Path, dest: Path, prune: bool = True) -> 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. + prune: Remove destination file that does not exist in source. Returns: True if ``dest``'s effective contents changed (created from absence, overwritten with different bytes, or removed because ``src`` is gone). """ if not src.exists(): - if dest.exists(): + if dest.exists() and prune: console.debug(f"Removing stale {dest}") path_ops.rm(dest) return True @@ -346,11 +327,12 @@ def _copy_if_exists(src: Path, dest: Path) -> bool: return changed -def sync_root_lockfile_to_web(filename: str) -> bool: +def sync_root_lockfile_to_web(filename: str, prune: bool = True) -> bool: """Mirror a single persisted lockfile into ``.web``. Args: filename: The lockfile basename. + prune: Remove destination file that does not exist in source. Returns: True if ``.web``'s copy was meaningfully changed (overwritten with @@ -359,7 +341,7 @@ def sync_root_lockfile_to_web(filename: str) -> bool: cache could exist yet. """ return _copy_if_exists( - get_root_lockfile_path(filename), get_web_lockfile_path(filename) + get_root_lockfile_path(filename), get_web_lockfile_path(filename), prune=prune ) @@ -370,7 +352,9 @@ def sync_root_lockfiles_to_web() -> bool: 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] + changed = [sync_root_lockfile_to_web(name) for name in LOCKFILE_NAMES] + [ + sync_root_lockfile_to_web(name, prune=False) for name in NO_PRUNE_LOCKFILE_NAMES + ] return any(changed) @@ -391,44 +375,34 @@ def sync_web_lockfile_to_root(filename: str): def sync_web_lockfiles_to_root(): """Persist every ``.web`` lockfile back to the app root.""" - for name in LOCKFILE_NAMES: + for name in LOCKFILE_NAMES + NO_PRUNE_LOCKFILE_NAMES: sync_web_lockfile_to_root(name) -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. + The parsed JSON object, or an empty dict if the file is missing, + cannot be parsed, or is not a JSON object. """ - root_package_json_path = get_root_package_json_path() + root_package_json_path = get_root_lockfile_path(constants.PackageJson.PATH) if not root_package_json_path.exists(): return {} try: - return json.loads(root_package_json_path.read_text()) + parsed = 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 {} + if not isinstance(parsed, dict): + console.warn( + f"Expected {root_package_json_path} to contain a JSON object, " + f"got {type(parsed).__name__}; starting with empty dependency lists." + ) + return {} + return parsed def initialize_web_directory(): @@ -446,6 +420,7 @@ def initialize_web_directory(): console.debug("Initializing the web directory.") initialize_package_json() + sync_web_lockfiles_to_root() console.debug("Initializing the bun config file.") initialize_bun_config() @@ -519,26 +494,31 @@ def _compile_package_json(): ``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. + from constants. User-added ``overrides`` are kept, with the + framework-owned entries refreshed on top. Any other persisted fields + (e.g. ``packageManager``, ``engines``) are passed through unchanged. + 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, + **(persisted.pop("scripts", None) or {}), "dev": constants.PackageJson.Commands.DEV, "export": constants.PackageJson.Commands.EXPORT, } return templates.package_json_template( scripts=scripts, - dependencies=persisted.get("dependencies") or {}, - dev_dependencies=persisted.get("devDependencies") or {}, - overrides=constants.PackageJson.OVERRIDES, + dependencies=persisted.pop("dependencies", None) or {}, + dev_dependencies=persisted.pop("devDependencies", None) or {}, + overrides={ + **(persisted.pop("overrides", None) or {}), + **constants.PackageJson.OVERRIDES, + }, + **persisted, ) diff --git a/reflex/utils/js_runtimes.py b/reflex/utils/js_runtimes.py index 9d1cc9de557..4ef2571ec63 100644 --- a/reflex/utils/js_runtimes.py +++ b/reflex/utils/js_runtimes.py @@ -426,7 +426,9 @@ def _existing_web_package_sections() -> tuple[set[str], set[str]]: 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() + web_pkg_json_path = frontend_skeleton.get_web_lockfile_path( + constants.PackageJson.PATH + ) if not web_pkg_json_path.exists(): return set(), set() try: @@ -757,4 +759,3 @@ def install_frontend_packages(packages: set[str], config: Config): _sync_root_lockfiles_for_frontend_install() _install_frontend_packages(set(packages), config, install_package_managers) 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 0079366ce97..63a298f78f6 100644 --- a/tests/units/test_prerequisites.py +++ b/tests/units/test_prerequisites.py @@ -132,23 +132,39 @@ def install(packages: set[str] | None = None) -> None: yield env -@pytest.fixture -def _stub_skeleton_initializers(monkeypatch): - """Stub the frontend_skeleton initialize_* helpers to no-ops.""" - for name in ( - "initialize_package_json", - "initialize_bun_config", - "initialize_npmrc", - "update_react_router_config", - "initialize_vite_config", - ): - monkeypatch.setattr(frontend_skeleton, name, lambda: None) +_SKELETON_INITIALIZERS = ( + "initialize_package_json", + "initialize_bun_config", + "initialize_npmrc", + "update_react_router_config", + "initialize_vite_config", +) + + +def _stub_skeleton_initializers_except( + monkeypatch: pytest.MonkeyPatch, keep: tuple[str, ...] = () +): + """Stub the frontend_skeleton initialize_* helpers to no-ops, except ``keep``. + + Args: + monkeypatch: The pytest monkeypatch fixture. + keep: Initializer names to leave running for real. + """ + for name in _SKELETON_INITIALIZERS: + if name not in keep: + monkeypatch.setattr(frontend_skeleton, name, lambda: None) monkeypatch.setattr(frontend_skeleton, "get_project_hash", lambda: None) monkeypatch.setattr( frontend_skeleton, "init_reflex_json", lambda project_hash: None ) +@pytest.fixture +def _stub_skeleton_initializers(monkeypatch): + """Stub the frontend_skeleton initialize_* helpers to no-ops.""" + _stub_skeleton_initializers_except(monkeypatch) + + @pytest.mark.parametrize( ("config", "export", "expected_output"), [ @@ -241,6 +257,83 @@ def test_initialize_web_directory_restores_root_bun_lock(tmp_path, monkeypatch): assert (web_dir / constants.Bun.LOCKFILE_PATH).read_text() == "root-lock" +def test_initialize_web_directory_persists_package_json_to_root(tmp_path, monkeypatch): + """initialize_web_directory persists the compiled package.json back to root.""" + template_dir = tmp_path / "template" + template_dir.mkdir() + (template_dir / ".gitignore").write_text(".web\n") + monkeypatch.setattr( + frontend_skeleton.constants.Templates.Dirs, "WEB_TEMPLATE", template_dir + ) + + web_dir = tmp_path / constants.Dirs.WEB + _patch_web_dir(monkeypatch, web_dir) + + # Stub every initializer except package.json so only its round-trip runs. + _stub_skeleton_initializers_except(monkeypatch, keep=("initialize_package_json",)) + + # A user override persisted in reflex.lock/package.json should round-trip. + 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({"overrides": {"user-pkg": "2.0.0"}})) + + with chdir(tmp_path): + frontend_skeleton.initialize_web_directory() + + web_pkg = web_dir / constants.PackageJson.PATH + # The compiled .web/package.json is persisted back to reflex.lock verbatim. + assert root_pkg.read_text() == web_pkg.read_text() + assert json.loads(root_pkg.read_text())["overrides"]["user-pkg"] == "2.0.0" + + +def test_sync_root_lockfile_to_web_prune_false_preserves_web_copy( + tmp_path, monkeypatch +): + """prune=False keeps the .web copy when the root copy is absent; prune=True removes it.""" + web_dir = tmp_path / constants.Dirs.WEB + web_dir.mkdir() + _patch_web_dir(monkeypatch, web_dir) + web_pkg = web_dir / constants.PackageJson.PATH + web_pkg.write_text('{"name": "reflex"}') + + with chdir(tmp_path): + # No root copy exists, so prune=False must not delete the .web copy. + assert ( + frontend_skeleton.sync_root_lockfile_to_web( + constants.PackageJson.PATH, prune=False + ) + is False + ) + assert web_pkg.read_text() == '{"name": "reflex"}' + + # The default (prune=True) removes the orphaned .web copy. + assert ( + frontend_skeleton.sync_root_lockfile_to_web(constants.PackageJson.PATH) + is True + ) + assert not web_pkg.exists() + + +def test_sync_root_lockfiles_to_web_keeps_web_package_json(tmp_path, monkeypatch): + """package.json (NO_PRUNE) survives an absent root copy while lockfiles prune.""" + web_dir = tmp_path / constants.Dirs.WEB + web_dir.mkdir() + _patch_web_dir(monkeypatch, web_dir) + + # .web has a package.json (e.g. from the template) but no root counterpart. + web_pkg = web_dir / constants.PackageJson.PATH + web_pkg.write_text('{"name": "reflex"}') + # A stale .web lockfile with no root counterpart should be pruned. + web_lock = web_dir / constants.Bun.LOCKFILE_PATH + web_lock.write_text("stale-lock") + + with chdir(tmp_path): + frontend_skeleton.sync_root_lockfiles_to_web() + + assert web_pkg.read_text() == '{"name": "reflex"}' + assert not web_lock.exists() + + def test_install_frontend_packages_syncs_root_bun_lock( install_packages_env: InstallPackagesEnv, ): @@ -446,7 +539,7 @@ def test_install_frontend_packages_skips_unpinned_already_in_package_json( ): """An unpinned package already in package.json is not re-added.""" env = install_packages_env - env.web_package_json.write_text( + env.root_package_json.write_text( json.dumps({"dependencies": {"already-installed": "2.3.4"}}) ) calls = _record_calls(env) @@ -466,7 +559,7 @@ def test_install_frontend_packages_skips_unpinned_dev_dep_already_in_package_jso ): """An unpinned dev dep already in package.json is not re-added.""" env = install_packages_env - env.web_package_json.write_text( + env.root_package_json.write_text( json.dumps({ "devDependencies": { "already-dev": "1.2.3", @@ -498,7 +591,7 @@ def test_install_frontend_packages_unpinned_already_present_makes_no_add_call( ): """If every requested unpinned package is already present, no add call runs.""" env = install_packages_env - env.web_package_json.write_text( + env.root_package_json.write_text( json.dumps({"dependencies": {"some-pkg": "1.0.0", "@scope/pkg": "2.0.0"}}) ) calls = _record_calls(env) @@ -514,7 +607,7 @@ def test_install_frontend_packages_moves_misplaced_unpinned_dep_to_deps( ): """A regular dep currently sitting under devDependencies gets relocated.""" env = install_packages_env - env.web_package_json.write_text( + env.root_package_json.write_text( json.dumps({"devDependencies": {"some-pkg": "1.2.3"}}) ) calls = _record_calls(env) @@ -538,7 +631,9 @@ def test_install_frontend_packages_moves_misplaced_unpinned_dev_dep_to_dev( ): """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"}})) + env.root_package_json.write_text( + json.dumps({"dependencies": {"some-dev": "1.2.3"}}) + ) class FakePlugin: def get_frontend_dependencies(self): @@ -571,7 +666,7 @@ def test_install_frontend_packages_moves_misplaced_pinned_framework_dep( """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( + env.root_package_json.write_text( json.dumps({"devDependencies": {"react": "18.0.0"}}) ) calls = _record_calls(env) @@ -648,7 +743,7 @@ def test_install_frontend_packages_conflict_with_misplaced_existing_entry( ): """A conflicting name currently in devDeps is removed and re-added to deps.""" env = install_packages_env - env.web_package_json.write_text( + env.root_package_json.write_text( json.dumps({"devDependencies": {"shared-pkg": "1.0.0"}}) ) @@ -680,7 +775,7 @@ def test_install_frontend_packages_does_not_move_correctly_placed_packages( ): """Packages already in the right section trigger no remove/add.""" env = install_packages_env - env.web_package_json.write_text( + env.root_package_json.write_text( json.dumps({ "dependencies": {"regular": "1.0.0"}, "devDependencies": {"dev-only": "2.0.0"}, @@ -816,7 +911,7 @@ 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["overrides"] == {"old-override": "1.0", "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" @@ -864,11 +959,97 @@ def test_compile_package_json_preserves_user_scripts(tmp_path): assert rendered["scripts"]["export"] == constants.PackageJson.Commands.EXPORT +def test_compile_package_json_preserves_user_overrides(tmp_path, monkeypatch): + """User-added overrides survive init; framework overrides win conflicts.""" + 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({ + "overrides": { + "user-pkg": "2.0.0", + "cookie": "0.0.1", + }, + }) + ) + monkeypatch.setattr( + constants.PackageJson, + "OVERRIDES", + {"cookie": "1.1.1"}, + ) + + with chdir(tmp_path): + rendered = json.loads(frontend_skeleton._compile_package_json()) + + assert rendered["overrides"] == {"user-pkg": "2.0.0", "cookie": "1.1.1"} + + +def test_compile_package_json_preserves_additional_fields(tmp_path): + """Persisted fields beyond the framework-managed ones pass through as-is.""" + 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({ + "name": "my-app", + "type": "commonjs", + "packageManager": "bun@1.2.0", + "engines": {"node": ">=20"}, + "lint-staged": {"*.js": "eslint"}, + "scripts": {"custom": "echo hi"}, + "dependencies": {"react": "19.2.5"}, + }) + ) + + with chdir(tmp_path): + rendered = json.loads(frontend_skeleton._compile_package_json()) + + assert rendered["name"] == "my-app" + # "type" is framework-owned and always reset to "module". + assert rendered["type"] == "module" + assert rendered["packageManager"] == "bun@1.2.0" + assert rendered["engines"] == {"node": ">=20"} + assert rendered["lint-staged"] == {"*.js": "eslint"} + assert rendered["scripts"]["custom"] == "echo hi" + assert rendered["dependencies"] == {"react": "19.2.5"} + + +def test_compile_package_json_null_fields(tmp_path): + """Explicit JSON null values for managed fields fall back to empty dicts.""" + 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( + '{"scripts": null, "dependencies": null, ' + '"devDependencies": null, "overrides": null}' + ) + + with chdir(tmp_path): + rendered = json.loads(frontend_skeleton._compile_package_json()) + + assert rendered["dependencies"] == {} + assert rendered["devDependencies"] == {} + assert rendered["overrides"] == constants.PackageJson.OVERRIDES + assert rendered["scripts"]["dev"] == constants.PackageJson.Commands.DEV + + +@pytest.mark.parametrize("content", ["[]", '"not-an-object"', "42"]) +def test_compile_package_json_non_object_root(tmp_path, content): + """A persisted package.json whose root is not an object is ignored.""" + 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(content) + + with chdir(tmp_path): + rendered = json.loads(frontend_skeleton._compile_package_json()) + + assert rendered["dependencies"] == {} + assert rendered["devDependencies"] == {} + assert rendered["overrides"] == constants.PackageJson.OVERRIDES + + def test_install_frontend_packages_removes_stale_dependencies( install_packages_env: InstallPackagesEnv, ): env = install_packages_env - env.web_package_json.write_text( + env.root_package_json.write_text( json.dumps({ "dependencies": { "still-needed": "1.0.0", @@ -895,7 +1076,7 @@ def test_install_frontend_packages_no_remove_when_all_needed( install_packages_env: InstallPackagesEnv, ): env = install_packages_env - env.web_package_json.write_text( + env.root_package_json.write_text( json.dumps({"dependencies": {"keep-me": "1.0.0"}, "devDependencies": {}}) ) calls = _record_calls(env) @@ -913,7 +1094,7 @@ def test_install_frontend_packages_keeps_framework_deps_during_remove( 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( + env.root_package_json.write_text( json.dumps({ "dependencies": {"react": "19.2.5", "stale-dep": "1.0.0"}, "devDependencies": {"vite": "8.0.9"},