fix(npm): improve peer dependency resolution #17835
Conversation
| &id.name, | ||
| &id.version, | ||
| &id.nv.name, | ||
| &id.nv.version, |
There was a problem hiding this comment.
Some changes that could be done (like only providing &id.nv here), will be in the next deno_graph refactor PR.
|
|
||
| use super::NpmPackageId; | ||
| use crate::npm::registry::NpmPackageInfo; | ||
| use crate::npm::registry::NpmPackageVersionInfo; |
There was a problem hiding this comment.
This file doesn't make much sense now, but it will be reused elsewhere in the deno_graph refactor.
| @@ -1629,40 +1753,55 @@ mod test { | |||
| packages, | |||
| vec![ | |||
| NpmResolutionPackage { | |||
| id: NpmPackageNodeId::from_serialized("[email protected]").unwrap(), | |||
| pkg_id: NpmPackageId::from_serialized( | |||
| "[email protected][email protected]" | |||
There was a problem hiding this comment.
These package identifiers always include the descendant peer dep resolutions now. This means lockfiles will have more information in them, but that's necessary in order to differentiate resolutions.
| self.graph.borrow_node(parent_id).parents.clone() | ||
| { | ||
| let path = path.with_specifier(specifier); | ||
| for grand_parent in grand_parents { |
There was a problem hiding this comment.
This was previously so wrong it's embarrassing, but it seems like it's worked out ok. The issue here is that we can't look at all of a node's parents... we have to just look at the current graph path's ancestors (now fixed).
| /// A pointer to a specific node in a graph path. The underlying node id | ||
| /// may change as peer dependencies are created. | ||
| #[derive(Clone, Debug)] | ||
| struct NodeIdRef(Arc<Mutex<NodeId>>); |
There was a problem hiding this comment.
Btw, I tried to use an Rc<RefCell<NodeId>> here, but the lsp code keeps complaining about the future not being send even when I wrap the code that calls this in a tokio::task::spawn_local(async move { ... }).await.unwrap(). I don't understand why it doesn't work.
|
Code is much simpler now by nodes not storing parents. |
This PR fixes peer dependency resolution to only resolve peers based on the current graph traversal path. Previously, it would resolve a peers by looking at a graph node's ancestors, which is not correct because graph nodes are shared by different resolutions.
It also stores more information about peer dependency resolution in the lockfile.