Skip to content

refactor(pm): read_json#2656

Draft
elrrrrrrr wants to merge 3 commits intonextfrom
pkg-json-read
Draft

refactor(pm): read_json#2656
elrrrrrrr wants to merge 3 commits intonextfrom
pkg-json-read

Conversation

@elrrrrrrr
Copy link
Contributor

Summary

Test Plan

@elrrrrrrr elrrrrrrr changed the title refactor: read_json refactor(pm): read_json Mar 3, 2026
@gemini-code-assist
Copy link
Contributor

Summary of Changes

Hello, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!

This pull request significantly refactors how package.json files are handled across the pm crate, focusing on performance and maintainability. It introduces a robust caching layer for package manifests, reducing redundant disk reads. The internal representation of package information has been modernized to align with a more comprehensive PackageJson type, and the distinction between npm lifecycle scripts and arbitrary user-defined scripts is now clearer. Additionally, the changes enhance the predictability of path-related operations by making the current working directory an explicit parameter in relevant functions.

Highlights

  • Package.json Caching: Introduced a caching mechanism for PackageJson objects (get_or_load_package_json) to optimize file I/O, particularly for frequently accessed project and workspace package.json files.
  • Refactored PackageInfo Model: Updated the PackageInfo model to directly use the richer PackageJson structure from the ruborist crate, separating npm lifecycle scripts into a dedicated LifecycleScripts struct from general scripts.
  • Explicit Current Working Directory: Modified link commands and other functions to explicitly accept a Path argument for the current working directory (cwd) instead of relying on std::env::current_dir(), improving testability and clarity.
  • Enhanced PackageJson Structure: The PackageJson struct in the ruborist crate has been extended with additional fields such as description, private, files, main, types, typings, and publish_config for more comprehensive package metadata representation.
  • Unified JSON Reading: Consolidated package.json and package-lock.json reading into type-specific functions (load_package_json_from_path, load_package_lock_json_from_path) that return deserialized structs directly.
Changelog
  • agents/code-guard.md
    • Added a new checklist item regarding use import declarations.
  • crates/pm/src/cmd/link.rs
    • Modified link_current_to_global and link_global_to_local functions to accept a cwd: &Path argument.
    • Updated PackageInfo::from_path to PackageInfo::load to utilize the new caching mechanism.
    • Removed std::env::set_current_dir calls from tests.
  • crates/pm/src/cmd/publish.rs
    • Replaced load_package_json_from_path with get_or_load_package_json for cached access.
    • Updated PublishMeta::from_json to PublishMeta::from_package_json to work with the PackageJson type.
    • Updated PackageInfo::from_json to PackageInfo::from_package_json.
  • crates/pm/src/cmd/run.rs
    • Replaced load_package_json_from_path with get_or_load_package_json.
    • Removed Scripts and Value imports, streamlining dependencies.
    • Updated script retrieval logic to use pkg.name and pkg.scripts directly from the PackageJson struct.
    • Added lifecycle_scripts to PackageInfo initialization and simplified script existence checks.
  • crates/pm/src/helper/fuzzy_select.rs
    • Replaced load_package_json_from_path with get_or_load_package_json for consistent caching.
  • crates/pm/src/helper/lock.rs
    • Imported read_json_file and set_package_json for JSON operations and cache management.
    • Replaced crate::fs::metadata with fs::metadata for file system checks.
    • Updated ensure_package_lock to use load_package_lock_json_from_path.
    • Refactored update_package_json to accept a new UpdatePackageJsonOptions struct for better parameter organization.
    • Replaced crate::fs::read_to_string and crate::fs::write with fs::read_to_string and fs::write.
    • Added write-through logic to update the PACKAGE_JSON_CACHE after modifying package.json files.
    • Modified is_pkg_lock_outdated to explicitly read package.json from disk to ensure consistency checks are based on the latest file state.
    • Updated load_package_json_from_path and load_package_lock_json_from_path to return deserialized structs directly.
    • Updated test calls to update_package_json to use the new UpdatePackageJsonOptions struct.
  • crates/pm/src/helper/tree_builder.rs
    • Removed direct PackageJson import from utoo_ruborist::manifest.
    • Replaced load_package_json_from_path with get_or_load_package_json for cached loading in init_tree.
  • crates/pm/src/main.rs
    • Introduced a cwd variable in the link command handler to pass to link_current_to_global and link_global_to_local.
  • crates/pm/src/model/package.rs
    • Added HashMap import for script management.
    • Replaced Scripts struct with LifecycleScripts to specifically handle npm lifecycle hooks.
    • Updated LifecycleScripts::from_json to LifecycleScripts::from_scripts to extract lifecycle hooks from a HashMap.
    • Removed the internal PublishConfig struct, now using the one from utoo_ruborist::manifest.
    • Updated PublishMeta::from_json to PublishMeta::from_package_json to align with the PackageJson type.
    • Modified PackageInfo to include a HashMap<String, String> for general scripts and a LifecycleScripts struct for lifecycle hooks.
    • Introduced PackageInfo::load for cached PackageJson loading and updated PackageInfo::from_path for non-cached disk reads.
    • Updated PackageInfo::from_json to PackageInfo::from_package_json to work with the new PackageJson type.
    • Adjusted tests to reflect the changes in PackageInfo and LifecycleScripts.
  • crates/pm/src/model/publish_payload.rs
    • Imported utoo_ruborist::manifest::PackageJson.
    • Changed PublishPayloadInput::package_json type from &serde_json::Value to &PackageJson.
    • Updated version_metadata creation and description retrieval to use fields directly from the PackageJson struct.
    • Modified tests to construct PackageJson instances directly.
  • crates/pm/src/service/binary.rs
    • Replaced load_package_json_from_path with read_json_file for reading JSON files.
    • Replaced crate::fs::try_exists with fs::try_exists for file existence checks.
    • Updated pkg mutation logic to use read_json_file for raw Value parsing.
    • Replaced crate::fs calls with fs in tests for consistency.
  • crates/pm/src/service/install.rs
    • Imported build_deps, fs, UpdatePackageJsonOptions, update_cwd_to_root, and load_package_lock_json_from_path.
    • Updated update_package_json call to use the new UpdatePackageJsonOptions struct.
    • Replaced crate::cmd::deps::build_deps with build_deps.
    • Replaced crate::fs::try_exists and crate::util::json::read_json_file with fs::try_exists and load_package_lock_json_from_path.
  • crates/pm/src/service/package.rs
    • Replaced Scripts import with LifecycleScripts.
    • Updated process_project_hooks to use PackageInfo::load for cached package data.
    • Changed script access from package_info.scripts.get_script to package_info.lifecycle_scripts.get_script.
    • Renamed read_package_scripts to read_lifecycle_scripts and updated its implementation to use PackageJson and LifecycleScripts::from_scripts.
    • Modified PackageInfo creation to include lifecycle_scripts and set general scripts to Default::default().
    • Updated script queueing and execution logic to use package.lifecycle_scripts.
    • Adjusted tests to reflect changes in PackageInfo and LifecycleScripts.
  • crates/pm/src/service/pm_pack.rs
    • Replaced load_package_json_from_path with get_or_load_package_json.
    • Updated pack function to use PackageJson and PackageInfo::from_package_json.
    • Updated version extraction and the collected closure to use pkg.version and pkg.to_value().
  • crates/pm/src/service/publish.rs
    • Replaced load_package_json_from_path with get_or_load_package_json.
    • Updated publish function to use pkg (PackageJson) for PublishPayloadInput.
  • crates/pm/src/service/script.rs
    • Changed script access from package.scripts.get_script to package.lifecycle_scripts.get_script.
    • Replaced Scripts import with LifecycleScripts in tests and updated test cases accordingly.
  • crates/pm/src/util/cloner.rs
    • Updated validate_name_version to directly compare pkg.name and pkg.version from the PackageJson type.
  • crates/pm/src/util/json.rs
    • Removed serde_json::Value import, favoring direct deserialization.
    • Updated load_package_json_from_path to return utoo_ruborist::manifest::PackageJson directly.
    • Updated load_package_lock_json_from_path to return utoo_ruborist::lock::PackageLock directly.
    • Adjusted tests to assert on pkg.name and pkg.version directly from the deserialized PackageJson.
  • crates/pm/src/util/linker.rs
    • Removed std::env import and env::set_current_dir calls from tests, relying on explicit path arguments.
  • crates/pm/src/util/user_config.rs
    • Added imports for Result, DashMap, PackageJson, and load_package_json_from_path.
    • Introduced PACKAGE_JSON_CACHE as a LazyLock<DashMap<PathBuf, PackageJson>> for caching.
    • Added get_or_load_package_json function for cached PackageJson loading.
    • Added set_package_json function for write-through caching to maintain cache consistency.
  • crates/ruborist/src/lib.rs
    • Exported PublishConfig from crate::model::package_json within the manifest module.
  • crates/ruborist/src/model/package_json.rs
    • Added description, private, files, main, types, typings, and publish_config fields to the PackageJson struct.
    • Defined a new PublishConfig struct for publish-related configuration.
    • Included tests for parsing the newly added publish-related fields and verifying round-trip serialization.
Activity
  • The pull request extensively refactors the handling of package.json files across the pm and ruborist crates.
  • It introduces a new caching layer for PackageJson objects to improve performance by reducing file I/O.
  • The PackageInfo model has been updated to use a more structured PackageJson type and to differentiate between lifecycle and general scripts.
  • Function signatures have been modified to explicitly pass cwd paths, enhancing testability and reducing reliance on global state.
  • Numerous file system operations (crate::fs::*) have been updated to use direct fs::* calls for consistency.
  • Tests have been adjusted throughout the codebase to accommodate the new data structures and function parameters.
Using Gemini Code Assist

The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.

Invoking Gemini

You can request assistance from Gemini at any point by creating a comment using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands on the current page.

Feature Command Description
Code Review /gemini review Performs a code review for the current pull request in its current state.
Pull Request Summary /gemini summary Provides a summary of the current pull request in its current state.
Comment @gemini-code-assist Responds in comments when explicitly tagged, both in pull request comments and review comments.
Help /gemini help Displays a list of available commands.

Customization

To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/ folder in the base of the repository. Detailed instructions can be found here.

Limitations & Feedback

Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here.

You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension.

Footnotes

  1. Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution.

Copy link
Contributor

@gemini-code-assist gemini-code-assist bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request is a significant and beneficial refactoring that replaces the use of serde_json::Value with a strongly-typed PackageJson struct for handling package.json files. This improves type safety and maintainability across the codebase. The introduction of a caching layer for package.json files is also a good performance enhancement. The changes are systematic and well-executed. I have one suggestion to make the refactoring even more consistent by updating one function that still relies on serde_json::Value.

Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant