Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
67 changes: 60 additions & 7 deletions src/cmd/install.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -671,16 +671,16 @@ describe.sequential("install deps", () => {
// publish scoped packages to registry
{
const remoteStorage = path.join(p, "remote_storage");
await publish(path.join(remoteStorage, "@foo+bar-1.0.0"), ctx.registry.endpoint, "fake-token");
await publish(path.join(remoteStorage, "@foo+bar-2.0.0"), ctx.registry.endpoint, "fake-token");
await publish(path.join(remoteStorage, "@scope+other-1.0.0"), ctx.registry.endpoint, "fake-token");
await publish(path.join(remoteStorage, "@foo/bar-1.0.0"), ctx.registry.endpoint, "fake-token");
await publish(path.join(remoteStorage, "@foo/bar-2.0.0"), ctx.registry.endpoint, "fake-token");
await publish(path.join(remoteStorage, "@scope/other-1.0.0"), ctx.registry.endpoint, "fake-token");
}

// Copy local_storage to distDir
const distDir = await tempDir();
{
const localStorage = path.join(p, "local_storage");
await copyDir(path.join(localStorage, "@foo+bar-1.0.0"), path.join(distDir, "@foo+bar-1.0.0"));
await copyDir(path.join(localStorage, "@foo/bar-1.0.0"), path.join(distDir, "@foo/bar-1.0.0"));
}

// Copy entry to workdir
Expand Down Expand Up @@ -726,9 +726,62 @@ describe.sequential("install deps", () => {
});

expect(new Set(fileList)).toEqual(new Set([
"@foo+bar-1.0.0/package.oo.yaml",
"@foo+bar-2.0.0/package.oo.yaml",
"@scope+other-1.0.0/package.oo.yaml",
"@foo/bar-1.0.0/package.oo.yaml",
"@foo/bar-2.0.0/package.oo.yaml",
"@scope/other-1.0.0/package.oo.yaml",
]));
});

it("should install scoped package with scoped sub-dependencies via integrity check", async (ctx) => {
const p = fixture("scoped_package_with_deps");

const remoteStorage = path.join(p, "remote_storage");
await publish(path.join(remoteStorage, "@other/lib-1.0.0"), ctx.registry.endpoint, "fake-token");

const distDir = await tempDir();
{
const localStorage = path.join(p, "local_storage");
await copyDir(path.join(localStorage, "@scope/pkg-1.0.0"), path.join(distDir, "@scope/pkg-1.0.0"));
}

await copyDir(path.join(p, "entry"), ctx.workdir);

const result = await install({
all: true,
token: "fake-token",
workdir: ctx.workdir,
distDir,
registry: ctx.registry.endpoint,
searchDirs: [path.join(p, "local_storage")],
});

expect(new Set(result.primaryDepNames)).toEqual(new Set([
"@scope/pkg-1.0.0",
]));

expect(result.deps["@scope/pkg-1.0.0"]).toEqual({
name: "@scope/pkg",
version: "1.0.0",
isAlreadyExist: true,
target: expect.any(String),
});

expect(result.deps["@other/lib-1.0.0"]).toEqual({
name: "@other/lib",
version: "1.0.0",
isAlreadyExist: false,
target: expect.any(String),
});

const fileList = await globby(`**/${ooPackageName}`, {
cwd: distDir,
onlyFiles: true,
absolute: false,
});

expect(new Set(fileList)).toEqual(new Set([
"@scope/pkg-1.0.0/package.oo.yaml",
"@other/lib-1.0.0/package.oo.yaml",
]));
});

Expand Down
24 changes: 12 additions & 12 deletions src/cmd/install.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ import { execa } from "execa";
import pLimit from "p-limit";
import { ooPackageName, ooThumbnailNames } from "../const";
import { copyDir, exists, mkdir, move, remove, tempDir, walk, xTar } from "../utils/fs";
import { env, normalizePackageName } from "../utils/misc";
import { env } from "../utils/misc";
import {
findLatestVersion,
getDependencies,
Expand Down Expand Up @@ -75,7 +75,7 @@ export async function installFile(options: InstallFileOptions): Promise<InstallF
if (isTarball) {
const tempDir = await xTar(options.file);
const { name, version } = await getOOPackageBasicInfo(tempDir);
const targetDir = path.join(options.distDir, `${normalizePackageName(name)}-${version}`);
const targetDir = path.join(options.distDir, `${name}-${version}`);
const isExists = await exists(targetDir);
if (isExists) {
await remove(targetDir);
Expand All @@ -92,7 +92,7 @@ export async function installFile(options: InstallFileOptions): Promise<InstallF
}

const { name, version } = await getOOPackageBasicInfo(options.file);
const targetDir = path.join(options.distDir, `${normalizePackageName(name)}-${version}`);
const targetDir = path.join(options.distDir, `${name}-${version}`);
const isExists = await exists(targetDir);
if (isExists) {
await remove(targetDir);
Expand Down Expand Up @@ -130,7 +130,7 @@ export async function installPackage(options: InstallPackageOptions): Promise<In

primaryDepNames.push(`${dep.name}-${version}`);

const existsDisk = await exists(path.join(options.distDir, `${normalizePackageName(dep.name)}-${version}`, ooPackageName));
const existsDisk = await exists(path.join(options.distDir, `${dep.name}-${version}`, ooPackageName));
if (existsDisk && dependencies?.[dep.name] === version) {
alreadyInstalled.push({
name: dep.name,
Expand Down Expand Up @@ -158,7 +158,7 @@ export async function installPackage(options: InstallPackageOptions): Promise<In
version: dependencies![key],
};

const existsDisk = await exists(path.join(options.distDir, `${normalizePackageName(dep.name)}-${dep.version}`, ooPackageName));
const existsDisk = await exists(path.join(options.distDir, `${dep.name}-${dep.version}`, ooPackageName));
if (!existsDisk) {
needInstall.push(dep);
}
Expand Down Expand Up @@ -210,7 +210,7 @@ export async function installAll(options: InstallAllOptions): Promise<InstallAll
version: dependencies![key],
};

const existsDisk = await exists(path.join(options.distDir, `${normalizePackageName(dep.name)}-${dep.version}`, ooPackageName));
const existsDisk = await exists(path.join(options.distDir, `${dep.name}-${dep.version}`, ooPackageName));
if (existsDisk) {
alreadyInstalled.push(dep);
}
Expand Down Expand Up @@ -278,7 +278,7 @@ async function _install(options: _InstallOptions): Promise<InstallPackageResult[
const targets: InstallPackageResult["deps"] = {};

for (const dep of options.alreadyInstalled) {
const target = path.join(options.distDir, `${normalizePackageName(dep.name)}-${dep.version}`);
const target = path.join(options.distDir, `${dep.name}-${dep.version}`);

targets[`${dep.name}-${dep.version}`] = {
name: dep.name,
Expand All @@ -299,7 +299,7 @@ async function _install(options: _InstallOptions): Promise<InstallPackageResult[
});
})
.map(async (i) => {
const target = path.join(options.distDir, `${normalizePackageName(i.name)}-${i.version}`);
const target = path.join(options.distDir, `${i.name}-${i.version}`);
const isAlreadyExist = await exists(target);

targets[`${i.name}-${i.version}`] = {
Expand Down Expand Up @@ -328,7 +328,7 @@ async function _install(options: _InstallOptions): Promise<InstallPackageResult[
const searchDeps = await integrityCheck(options, tempDistDir);
const ps = searchDeps.map(async (dep) => {
const fullName = `${dep.name}-${dep.version}` as const;
const target = path.join(options.distDir, `${normalizePackageName(dep.name)}-${dep.version}`);
const target = path.join(options.distDir, `${dep.name}-${dep.version}`);
if (targets[fullName]) {
return;
}
Expand Down Expand Up @@ -397,7 +397,7 @@ async function integrityCheck(options: _InstallOptions, tempDistDir: string): Pr
addedMap.set(`${i.name}-${i.version}`, {
name: i.name,
version: i.version,
distDir: path.join(distDir, `${normalizePackageName(i.name)}-${i.version}`),
distDir: path.join(distDir, `${i.name}-${i.version}`),
});
}
});
Expand All @@ -410,7 +410,7 @@ async function integrityCheck(options: _InstallOptions, tempDistDir: string): Pr
const moveLimit = pLimit(10);
const movePs = Array.from(addedMap.values()).map((target) => {
return moveLimit(async () => {
const newTarget = path.join(tempDistDir, `${normalizePackageName(target.name)}-${target.version}`);
const newTarget = path.join(tempDistDir, `${target.name}-${target.version}`);

await move(target.distDir, newTarget);

Expand Down Expand Up @@ -456,7 +456,7 @@ async function installMissDep(deps: Deps, options: _InstallOptions): Promise<{
const collectedDeps: Deps = [];

for (const i of info) {
const target = path.join(distDir, `${normalizePackageName(i.name)}-${i.version}`);
const target = path.join(distDir, `${i.name}-${i.version}`);
await move(i.source, target);
collectedDeps.push({
name: i.name,
Expand Down
34 changes: 32 additions & 2 deletions src/cmd/list.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -120,12 +120,12 @@ describe("list", () => {
{
name: "@foo/bar",
version: "1.0.0",
distDir: path.join(localDir, "@foo+bar-1.0.0"),
distDir: path.join(localDir, "@foo/bar-1.0.0"),
},
{
name: "@scope/other",
version: "1.0.0",
distDir: path.join(localDir2, "@scope+other-1.0.0"),
distDir: path.join(localDir2, "@scope/other-1.0.0"),
},
{
name: "normal-package",
Expand All @@ -134,6 +134,36 @@ describe("list", () => {
},
].sort(sortDeps));
});

it("should list scoped packages with nested scoped dependencies", async () => {
const p = fixture("scoped_package_with_deps");

const entryDir = path.join(p, "entry");
const localDir = path.join(p, "local_storage");
const remoteDir = path.join(p, "remote_storage");

const result = await list(entryDir, [localDir, remoteDir]);

expect(result.length).toBe(3);

expect(result.sort(sortDeps)).toStrictEqual([
{
name: "@other/lib",
version: "1.0.0",
distDir: path.join(remoteDir, "@other/lib-1.0.0"),
},
{
name: "@scope/pkg",
version: "1.0.0",
distDir: path.join(localDir, "@scope/pkg-1.0.0"),
},
{
name: "normal-dep",
version: "1.0.0",
distDir: path.join(localDir, "normal-dep-1.0.0"),
},
].sort(sortDeps));
});
});

function sortDeps(a: SearchDep, b: SearchDep) {
Expand Down
76 changes: 75 additions & 1 deletion src/utils/fs.test.ts
Original file line number Diff line number Diff line change
@@ -1,9 +1,10 @@
import type { IDepMap } from "../types";
import { readdir } from "node:fs/promises";
import path from "node:path";
import { globby } from "globby";
import { beforeEach, describe, expect, it } from "vitest";
import { fixture } from "../../tests/helper/fs";
import { copyDir, remove, tempDir } from "./fs";
import { copyDir, exists, mkdir, remove, tempDir, walk } from "./fs";

beforeEach(async (ctx) => {
const temp = await tempDir();
Expand Down Expand Up @@ -62,3 +63,76 @@ describe.concurrent("copyDir", () => {
await Promise.all(ps);
});
});

describe("walk", () => {
it("should walk scoped packages with nested dependencies", async () => {
const p = fixture("scoped_package_with_deps");
const localDir = path.join(p, "local_storage");
const remoteDir = path.join(p, "remote_storage");

const map: IDepMap = new Map();
await walk("@scope/pkg", "1.0.0", [localDir, remoteDir], map);

// Should find the scoped package
expect(map.has("@scope/pkg-1.0.0")).toBe(true);
expect(map.get("@scope/pkg-1.0.0")).toEqual({
name: "@scope/pkg",
version: "1.0.0",
distDir: path.join(localDir, "@scope/pkg-1.0.0"),
});

// Should find the normal dependency
expect(map.has("normal-dep-1.0.0")).toBe(true);
expect(map.get("normal-dep-1.0.0")).toEqual({
name: "normal-dep",
version: "1.0.0",
distDir: path.join(localDir, "normal-dep-1.0.0"),
});

// Should find the nested scoped dependency
expect(map.has("@other/lib-1.0.0")).toBe(true);
expect(map.get("@other/lib-1.0.0")).toEqual({
name: "@other/lib",
version: "1.0.0",
distDir: path.join(remoteDir, "@other/lib-1.0.0"),
});
});

it("should handle missing scoped packages", async () => {
const p = fixture("scoped_package_with_deps");
const localDir = path.join(p, "local_storage");

const map: IDepMap = new Map();
await walk("@nonexistent/pkg", "1.0.0", [localDir], map);

expect(map.has("@nonexistent/pkg-1.0.0")).toBe(true);
expect(map.get("@nonexistent/pkg-1.0.0")).toEqual({
name: "@nonexistent/pkg",
version: "1.0.0",
distDir: "",
});
});
});

describe("mkdir with scoped paths", () => {
it("should create nested directories for scoped packages", async (ctx) => {
const scopedPath = path.join(ctx.workdir, "@scope/pkg-1.0.0");

await mkdir(scopedPath);

expect(await exists(scopedPath)).toBe(true);
expect(await exists(path.join(ctx.workdir, "@scope"))).toBe(true);
});
});

describe("exists with scoped paths", () => {
it("should detect scoped package directories", async (ctx) => {
const scopedPath = path.join(ctx.workdir, "@scope/pkg-1.0.0");

expect(await exists(scopedPath)).toBe(false);

await mkdir(scopedPath);

expect(await exists(scopedPath)).toBe(true);
});
});
4 changes: 2 additions & 2 deletions src/utils/fs.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@ import fsP from "node:fs/promises";
import os from "node:os";
import path from "node:path";
import * as tar from "tar";
import { normalizePackageName } from "./misc";
import { checkOOPackage, getDependencies } from "./npm";

export async function tempDir() {
Expand All @@ -12,6 +11,7 @@ export async function tempDir() {

export async function move(src: string, dest: string) {
try {
await mkdir(path.dirname(dest));
await fsP.rename(src, dest);
}
catch (err) {
Comment on lines 12 to 17
Copy link

Copilot AI Jan 7, 2026

Choose a reason for hiding this comment

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

The new parent directory creation logic in the move function should be covered by tests, especially for scoped packages where nested directories are critical. Consider adding a test case that calls move() with a destination path containing nested directories (like moving to @scope/pkg-1.0.0) to verify parent directories are properly created.

Copilot uses AI. Check for mistakes.
Expand Down Expand Up @@ -94,7 +94,7 @@ export async function xTar(tarball: string) {
export async function walk(name: string, version: string, searchDirs: string[], map: IDepMap) {
let distDir = "";
for (const searchDir of searchDirs) {
distDir = path.join(searchDir, `${normalizePackageName(name)}-${version}`);
distDir = path.join(searchDir, `${name}-${version}`);

const isCorrect = await checkOOPackage(distDir);
if (isCorrect) {
Expand Down
4 changes: 0 additions & 4 deletions src/utils/misc.ts
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,3 @@ export function nerfURL(url: string) {
const rel = new URL(".", from);
return `//${rel.host}${rel.pathname}`;
}

export function normalizePackageName(name: string): string {
return name.replace(/\//g, "+");
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
name: test-scoped-with-deps
version: 0.0.1
dependencies:
"@scope/pkg": "1.0.0"
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
name: "@scope/pkg"
version: 1.0.0
dependencies:
"normal-dep": "1.0.0"
"@other/lib": "1.0.0"
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
name: normal-dep
version: 1.0.0
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
name: "@other/lib"
version: 1.0.0