Skip to content

refactor(pnp): support resolve from global cached pkg#167

Open
stormslowly wants to merge 8 commits intomainfrom
fix/pnp_api_locate
Open

refactor(pnp): support resolve from global cached pkg#167
stormslowly wants to merge 8 commits intomainfrom
fix/pnp_api_locate

Conversation

@stormslowly
Copy link
Collaborator

@stormslowly stormslowly commented Mar 4, 2026

Problem

web-infra-dev/rspress#1246

File: ../../../.yarn/berry/cache/react-lazy-with-preload-npm-2.2.1-34daf03779-10c0.zip/node_modules/react-lazy-with-preload/lib/index.js:4:23-30
  × Module not found: Can't resolve 'react' in '/Users/bytedance/.yarn/berry/cache/react-lazy-with-preload-npm-2.2.1-34daf03779-10c0.zip/node_modules/react-lazy-with-preload/lib'
 
File: ../../../.yarn/berry/cache/react-lazy-with-preload-npm-2.2.1-34daf03779-10c0.zip/node_modules/react-lazy-with-preload/lib/index.js:4:23-30
  × Module not found: Can't resolve 'react' in '/Users/bytedance/.yarn/berry/cache/react-lazy-with-preload-npm-2.2.1-34daf03779-10c0.zip/node_modules/react-lazy-with-preload/lib'
   ╭─[4:14]
 2 │ Object.defineProperty(exports, "__esModule", { value: true });
 3 │ exports.lazyWithPreload = void 0;
 4 │ var react_1 = require("react");
   ·               ────────────────
 5 │ function lazyWithPreload(factory) {
 6 │     var ReactLazyComponent = (0, react_1.lazy)(factory);
   ╰────

In short, the Rspack resolver will suffer a resolve failure when resolving from a global cache module. In this example ~/.yarn/berry/cache/pkg-name-hash-hash/node_modules/pkg-name/...

We already have a solution by specifying the PnP Manifest. It has two drawbacks

  1. User has another config
  2. Resolver sticks to a PnP manifest, which will prevent the resolver's ability to resolve in a different PnP project.

Compared to yarn PnP, there is no config to specify a .pnp.cjs. So should rspack-resolver.

What changed

Before this PR, Rspack Resolver does only phase 2's job. If a resolving request's module path goes to the global cache path, no PnPAPI will be found. So resolving failed.

In this PR, the Rspack resolver implements phase 1 in find_pnp_manifest to find PnPAPI for the global cache path.

  1. Find all the candidate manifests that match the module path of the request.
  2. Pick a best candidate for later resolving.

How yarn Plug'n'Play works from the resolver's view

PnP resolve Steps:

  1. Find the right PnPAPI for the current resolving request
  2. call the PnPAPIs (resolveToUnqualified...) to finish the request.
    Let's focus on "find the right PnPAPI".

Find the PnPAPI

Where does PnPapi come from? The .pnp.cjs file exports the API.

> require('./.pnp.cjs')
{
  // ....
  resolveToUnqualified: [Function (anonymous)],
  resolveUnqualified: [Function (anonymous)],
  resolveRequest: [Function (anonymous)],
  //...
}

How to find the PnPAPI? When you need to find something in a company, the easiest way is to ask the manager. So as the PnpAPI.
There is a manager in every .pnp.cjs. If there are multiple .pnp.cjs files involved in the project, which manager rules?

The one comes as module.parent.id === "internal/preload"
All the modules required via --require module_file.js will have "internal/preload" parent.

In a Yarn PnP project, scripts will be run this way:

node --require './current/pkg/.pnp.cjs' script.js
if (module.parent && module.parent.id === `internal/preload`) {
  defaultApi.setup();
  // ...
}

In the setup API, one important thing is to intercept the Node Module API _resolveFilename, which does the job of resolving
In PnP intercepted _resolveFilename, the manager finds the right PnPAPI by resolve request's parameters (specifier and module path).

PnPAPI manager does the PnPAPI (.pnp.cjs) finding in two phases.

phase 1

Try all the registered PnPAPIs to check if the module path matches the PnPAPI's locations. This phase is important when resolving from a global cache ZIP FS.

for example:

resolve(
  "react-router-dom",
  "/Users/bytedance/.yarn/berry/cache/@rspress-core-npm-2.0.3-xxxx-yyyy.zip/node_modules/@rspress/core/dist/runtime/index.js"
)
const locator = apiEntry.instance.findPackageLocator( "/Users/bytedance/.yarn/berry/cache/@rspress-core-npm-2.0.3-xxxx-yyyy.zip/node_modules/@rspress/core/dist/runtime/index.js");
/*
   It checks if modulePath's ancestor path exists in packageLocatorsByLocations
   const entry = packageLocatorsByLocations.get(relativeLocation);
*/      

If the module path matches one PnPAPI, this PnPAPI is picked. And it will be called in this resolve request.

If no PnPAPIs match, we will go to phase 2.

phase 2

Try to find .pnp.cjs from the module path; if there is none, we walk up the directory.
It works as pnp::find_closest_pnp_manifest_path.

After finding the PnPAPIPath, PnP will fetch the stored PnPapi instance or load from the found path (instance: loadApiInstance(pnpApiPath)) and register it.

In the Rspack resolver's view, we have the pnp-rs crate, which implements PnPAPI for resolving. But we have to implement the manager's logic in the Rspack resolver by ourselves. This PR does some parts of the manager's job.

@gemini-code-assist
Copy link

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 refactors the Yarn Plug'n'Play (PnP) resolution logic to better support resolving modules from global cached packages. It streamlines the internal caching of PnP manifests and enhances the discovery mechanism, removing the need for a manual pnp_manifest configuration option. These changes aim to improve the robustness and flexibility of PnP integration within the resolver.

Highlights

  • PnP Manifest Caching Refactor: The internal caching mechanism for Yarn Plug'n'Play (PnP) manifests has been significantly refactored. The pnp_manifest_content_cache and pnp_manifest_path_cache fields in ResolverGeneric were renamed to pnp_manifests and pnp_manifest_path_for_path respectively, and their types were updated to use Arc<pnp::Manifest> for better shared ownership.
  • Enhanced find_pnp_manifest Logic: The find_pnp_manifest function now employs a two-phase lookup strategy. It first checks already-loaded manifests via a location trie to handle global cache paths, then falls back to a filesystem walk if necessary. This improves support for resolving from globally cached packages.
  • Removed pnp_manifest Option: The pnp_manifest option, which allowed specifying a direct path to a PnP manifest file, has been removed from ResolveOptions. The resolver now dynamically discovers PnP manifests based on the enhanced find_pnp_manifest logic.

🧠 New Feature in Public Preview: You can now enable Memory to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console.

Changelog
  • src/lib.rs
    • Removed mapref::one::Ref from dashmap import.
    • Renamed pnp_manifest_content_cache to pnp_manifests and updated its type to Arc<pnp::Manifest>.
    • Renamed pnp_manifest_path_cache to pnp_manifest_path_for_path and updated its key type to CachedPath.
    • Added descriptive comments for the new PnP related fields.
    • Updated initialization and clearing logic for the renamed PnP caches.
    • Refactored find_pnp_manifest to implement a two-phase lookup for PnP manifests, including handling global cache paths.
    • Changed the return type of find_pnp_manifest to Option<Arc<pnp::Manifest>>.
    • Simplified resolve_pnp_unqualified to directly use the Arc<pnp::Manifest> returned by find_pnp_manifest.
  • src/options.rs
    • Removed the pnp_manifest: Option<PathBuf> field from the ResolveOptions struct.
    • Updated the Default implementation for ResolveOptions to remove the pnp_manifest field initialization.
  • src/tests/pnp.rs
    • Removed the pnp_manifest field from ResolveOptions initialization in a test case, aligning with its removal from the struct.
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.

@stormslowly stormslowly changed the title refactor(pnp): align pnpm findApiPathFor to support resolve from global ca… refactor(pnp): support resolve from global cached pkg Mar 4, 2026
@stormslowly
Copy link
Collaborator Author

@codex

@codspeed-hq
Copy link

codspeed-hq bot commented Mar 4, 2026

Merging this PR will not alter performance

✅ 12 untouched benchmarks


Comparing fix/pnp_api_locate (45223e6) with main (b83c182)

Open in CodSpeed

Copy link

@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 refactors the pnpm manifest discovery logic to better align with pnpm's behavior, particularly for resolving packages from a global cache. The changes include renaming cache-related fields for clarity, removing the pnp_manifest option in favor of automatic discovery, and rewriting find_pnp_manifest with a more robust two-phase lookup. However, it introduces a high-severity security regression in find_pnp_manifest. The new 'Phase 1' search allows any manifest to claim ownership of any path, which can be exploited for cross-project module resolution hijacking, potentially leading to Remote Code Execution (RCE) in shared resolver environments. Additionally, there's a suggestion to improve error handling for manifest loading to make the resolver more robust against configuration errors.

@stormslowly stormslowly force-pushed the fix/pnp_api_locate branch 2 times, most recently from d9f0294 to 6d013ac Compare March 5, 2026 07:42
@stormslowly stormslowly marked this pull request as ready for review March 5, 2026 18:22
@stormslowly stormslowly requested review from Copilot and hardfist March 5, 2026 18:22
Copy link

@chatgpt-codex-connector chatgpt-codex-connector bot left a comment

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 85fec66b37

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Copilot encountered an error and was unable to review this pull request. You can try again by re-requesting a review.

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.

2 participants