Skip to content

Commit

Permalink
fix: make dependency graph more robust (#909)
Browse files Browse the repository at this point in the history 
Fixes #898

Needed for easier debugging (script to clone a jsr package and all its
dependencies locally)
  • Loading branch information
@crowlKats
crowlKats authored Jan 31, 2025
1 parent 34e0ead commit 1574516
Show file tree
Hide file tree
Showing 4 changed files with 54 additions and 42 deletions.
38 changes: 25 additions & 13 deletions api/src/api/package.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ use hyper::Request;
use hyper::Response;
use hyper::StatusCode;
use indexmap::IndexMap;
use indexmap::IndexSet;
use regex::Regex;
use routerify::prelude::RequestExt;
use routerify::Router;
Expand Down Expand Up @@ -1849,6 +1850,7 @@ struct GraphDependencyCollector<'a> {
dependencies: &'a mut IndexMap<DependencyKind, DependencyInfo>,
exports: &'a IndexMap<String, IndexMap<String, String>>,
id_index: &'a mut usize,
visited: IndexSet<DependencyKind>,
}

impl<'a> GraphDependencyCollector<'a> {
Expand All @@ -1866,6 +1868,7 @@ impl<'a> GraphDependencyCollector<'a> {
dependencies,
exports,
id_index,
visited: Default::default(),
}
.build_module_info(root_module)
.unwrap();
Expand Down Expand Up @@ -1920,6 +1923,12 @@ impl<'a> GraphDependencyCollector<'a> {
}
};

if self.visited.contains(&dependency) {
return self.dependencies.get(&dependency).map(|dep| dep.id);
} else {
self.visited.insert(dependency.clone());
}

if let Some(info) = self.dependencies.get(&dependency) {
Some(info.id)
} else {
Expand All @@ -1941,24 +1950,27 @@ impl<'a> GraphDependencyCollector<'a> {
| Module::External(_) => None,
};

let mut children = vec![];
let id = *self.id_index;
*self.id_index += 1;

let mut children = IndexSet::new();
match module {
Module::Js(module) => {
if let Some(types_dep) = &module.maybe_types_dependency {
if let Some(child) = self.build_resolved_info(&types_dep.dependency)
{
children.push(child);
children.insert(child);
}
}
for dep in module.dependencies.values() {
if !dep.maybe_code.is_none() {
if let Some(child) = self.build_resolved_info(&dep.maybe_code) {
children.push(child);
children.insert(child);
}
}
if !dep.maybe_type.is_none() {
if let Some(child) = self.build_resolved_info(&dep.maybe_type) {
children.push(child);
children.insert(child);
}
}
}
Expand All @@ -1970,8 +1982,6 @@ impl<'a> GraphDependencyCollector<'a> {
| Module::External(_) => {}
}

let id = *self.id_index;

self.dependencies.insert(
dependency,
DependencyInfo {
Expand All @@ -1982,8 +1992,6 @@ impl<'a> GraphDependencyCollector<'a> {
},
);

*self.id_index += 1;

Some(id)
}
}
Expand All @@ -2004,7 +2012,7 @@ impl<'a> GraphDependencyCollector<'a> {
},
DependencyInfo {
id,
children: vec![],
children: Default::default(),
size: None,
media_type: None,
},
Expand Down Expand Up @@ -2052,7 +2060,7 @@ pub enum DependencyKind {
#[derive(Debug, Eq, PartialEq)]
pub struct DependencyInfo {
pub id: usize,
pub children: Vec<usize>,
pub children: IndexSet<usize>,
pub size: Option<u64>,
pub media_type: Option<MediaType>,
}
Expand Down Expand Up @@ -2165,6 +2173,7 @@ pub async fn get_score_handler(
mod test {
use hyper::Body;
use hyper::StatusCode;
use indexmap::IndexSet;
use serde_json::json;

use crate::api::ApiDependency;
Expand Down Expand Up @@ -3631,10 +3640,11 @@ ggHohNAjhbzDaY2iBW/m3NC5dehGUP4T2GBo/cwGhg==
assert_eq!(
deps,
vec![ApiDependencyGraphItem {
id: 0,
dependency: super::DependencyKind::Root {
path: "/mod.ts".to_string(),
},
children: vec![],
children: IndexSet::new(),
size: Some(155),
media_type: Some("TypeScript".to_string()),
}]
Expand Down Expand Up @@ -3664,21 +3674,23 @@ ggHohNAjhbzDaY2iBW/m3NC5dehGUP4T2GBo/cwGhg==
deps,
vec![
ApiDependencyGraphItem {
id: 1,
dependency: super::DependencyKind::Jsr {
scope: "scope".to_string(),
package: "foo".to_string(),
version: "1.2.3".to_string(),
entrypoint: super::JsrEntrypoint::Entrypoint(".".to_string())
},
children: vec![],
children: IndexSet::new(),
size: Some(155),
media_type: Some("TypeScript".to_string())
},
ApiDependencyGraphItem {
id: 0,
dependency: super::DependencyKind::Root {
path: "/mod.ts".to_string()
},
children: vec![0],
children: IndexSet::from([1]),
size: Some(117),
media_type: Some("TypeScript".to_string())
}
Expand Down
4 changes: 3 additions & 1 deletion api/src/api/types.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -84,8 +84,9 @@ impl From<PublishingTask> for ApiPublishingTask {
#[derive(Serialize, Deserialize, Debug, Clone, PartialEq)]
#[serde(rename_all = "camelCase")]
pub struct ApiDependencyGraphItem {
pub id: usize,
pub dependency: super::package::DependencyKind,
pub children: Vec<usize>,
pub children: indexmap::IndexSet<usize>,
pub size: Option<u64>,
pub media_type: Option<String>,
}
Expand All @@ -103,6 +104,7 @@ impl
),
) -> Self {
Self {
id: info.id,
dependency: kind,
children: info.children,
size: info.size,
Expand Down
53 changes: 25 additions & 28 deletions frontend/routes/package/(_islands)/DependencyGraph.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -48,16 +48,16 @@ interface JsrPackage {
version: string;
}

export function groupDependencies(
function groupDependencies(
items: DependencyGraphItem[],
): GroupedDependencyGraphItem[] {
const referencedBy = new Map<number, Set<number>>();
for (let i = 0; i < items.length; i++) {
for (const child of items[i].children) {
if (!referencedBy.has(child)) {
referencedBy.set(child, new Set());
for (const item of items) {
for (const childId of item.children) {
if (!referencedBy.has(childId)) {
referencedBy.set(childId, new Set());
}
referencedBy.get(child)!.add(i);
referencedBy.get(childId)!.add(item.id);
}
}

Expand All @@ -66,16 +66,15 @@ export function groupDependencies(
entrypoints: {
entrypoint: string;
isEntrypoint: boolean;
oldIndex: number;
oldId: number;
}[];
children: number[];
size: number | undefined;
mediaType: string | undefined;
oldIndices: number[];
oldIds: number[];
}>();

for (let i = 0; i < items.length; i++) {
const item = items[i];
for (const item of items) {
if (item.dependency.type === "jsr") {
const groupKey =
`${item.dependency.scope}/${item.dependency.package}@${item.dependency.version}`;
Expand All @@ -89,29 +88,28 @@ export function groupDependencies(
children: [],
size: undefined,
mediaType: undefined,
oldIndices: [],
oldIds: [],
};
group.entrypoints.push({
entrypoint: item.dependency.entrypoint.value,
isEntrypoint: item.dependency.entrypoint.type == "entrypoint",
oldIndex: i,
oldId: item.id,
});
group.children.push(...item.children);
if (item.size !== undefined) {
group.size ??= 0;
group.size += item.size;
}
group.oldIndices.push(i);
group.oldIds.push(item.id);
jsrGroups.set(groupKey, group);
}
}

const oldIndexToNewIndex = new Map<number, number>();
const idToIndex = new Map<number, number>();
const placedJsrGroups = new Set<string>();
const out: GroupedDependencyGraphItem[] = [];

for (let i = 0; i < items.length; i++) {
const item = items[i];
for (const item of items) {
if (item.dependency.type === "jsr") {
const groupKey =
`${item.dependency.scope}/${item.dependency.package}@${item.dependency.version}`;
Expand All @@ -120,12 +118,11 @@ export function groupDependencies(
if (!placedJsrGroups.has(groupKey)) {
placedJsrGroups.add(groupKey);

const groupIndicesSet = new Set(group.oldIndices);
const filteredEntrypoints = group.entrypoints.filter(({ oldIndex }) => {
const refs = referencedBy.get(oldIndex)!;

const groupIds = new Set(group.oldIds);
const filteredEntrypoints = group.entrypoints.filter(({ oldId }) => {
const refs = referencedBy.get(oldId)!;
for (const ref of refs) {
if (!groupIndicesSet.has(ref)) {
if (!groupIds.has(ref)) {
return true;
}
}
Expand Down Expand Up @@ -153,13 +150,13 @@ export function groupDependencies(
mediaType: group.mediaType,
});

for (const oldIdx of group.oldIndices) {
oldIndexToNewIndex.set(oldIdx, newIndex);
for (const oldId of group.oldIds) {
idToIndex.set(oldId, newIndex);
}
} else {
oldIndexToNewIndex.set(
i,
oldIndexToNewIndex.get(jsrGroups.get(groupKey)!.oldIndices[0])!,
idToIndex.set(
item.id,
idToIndex.get(jsrGroups.get(groupKey)!.oldIds[0])!,
);
}
} else {
Expand All @@ -169,14 +166,14 @@ export function groupDependencies(
size: item.size,
mediaType: item.mediaType,
});
oldIndexToNewIndex.set(i, out.length - 1);
idToIndex.set(item.id, out.length - 1);
}
}

for (let index = 0; index < out.length; index++) {
const newItem = out[index];
const remappedChildren = newItem.children
.map((childIdx) => oldIndexToNewIndex.get(childIdx)!)
.map((oldId) => idToIndex.get(oldId)!)
.filter((childNewIdx) => childNewIdx !== index);
newItem.children = Array.from(new Set(remappedChildren));
}
Expand Down
1 change: 1 addition & 0 deletions frontend/utils/api_types.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -302,6 +302,7 @@ export type DependencyGraphKind =
| DependencyGraphKindError;

export interface DependencyGraphItem {
id: number;
dependency: DependencyGraphKind;
children: number[];
size: number | undefined;
Expand Down

0 comments on commit 1574516

Please sign in to comment.