Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: cannot use file path dependencies #1779

Merged
merged 6 commits into from
Apr 20, 2022
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
7 changes: 3 additions & 4 deletions src/javascript/node-package.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,6 @@ import {
readdirSync,
readJsonSync,
} from "fs-extra";
import * as semver from "semver";
import { resolve as resolveJson } from "../_resolve";
import { Component } from "../component";
import { DependencyType } from "../dependencies";
Expand All @@ -17,7 +16,7 @@ import { Project } from "../project";
import { isAwsCodeArtifactRegistry } from "../release";
import { Task } from "../task";
import { exec, isTruthy, sorted, writeFile } from "../util";
import { extractCodeArtifactDetails } from "./util";
import { extractCodeArtifactDetails, minVersion } from "./util";

const UNLICENSED = "UNLICENSED";
const DEFAULT_NPM_REGISTRY_URL = "https://registry.npmjs.org/";
Expand Down Expand Up @@ -1010,7 +1009,7 @@ export class NodePackage extends Component {
}

if (dep.version) {
const ver = semver.minVersion(dep.version)?.version;
const ver = minVersion(dep.version);
if (!ver) {
throw new Error(
`unable to determine minimum semver for peer dependency ${dep.name}@${dep.version}`
Expand Down Expand Up @@ -1189,7 +1188,7 @@ export class NodePackage extends Component {
}

// Take version and pin as dev dependency
const ver = semver.minVersion(version)?.version;
const ver = minVersion(version);
if (!ver) {
throw new Error(
`unable to determine minimum semver for peer dependency ${name}@${version}`
Expand Down
9 changes: 9 additions & 0 deletions src/javascript/util.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import { basename, dirname, extname, join, sep } from "path";
import * as semver from "semver";

export function renderBundleName(entrypoint: string) {
const parts = join(entrypoint).split(sep);
Expand Down Expand Up @@ -32,3 +33,11 @@ export function extractCodeArtifactDetails(registryUrl: string) {
}
throw new Error("Could not get CodeArtifact details from npm Registry");
}

export function minVersion(version: string): string | undefined {
if (semver.validRange(version)) {
return semver.minVersion(version)?.version;
} else {
return version;
}
}
20 changes: 20 additions & 0 deletions test/deps.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -284,6 +284,26 @@ test("it is possible to overwrite dependency specs", () => {
]);
});

test("it is possible to have local file dependencies", () => {
// GIVEN
const p = new TestProject();

// WHEN
p.deps.addDependency("cowsay@file:./cowsay", DependencyType.RUNTIME);
p.deps.addDependency("lolcat@file:../path/to/lolcat", DependencyType.BUILD);
p.deps.addDependency(
"fortune@file:../../path/to/fortune",
DependencyType.PEER
);

// THEN
expect(p.deps.all).toStrictEqual([
{ name: "lolcat", type: "build", version: "file:../path/to/lolcat" },
{ name: "fortune", type: "peer", version: "file:../../path/to/fortune" },
{ name: "cowsay", type: "runtime", version: "file:./cowsay" },
]);
});

function depsManifest(p: Project) {
p.synth();
const filepath = join(p.outdir, Dependencies.MANIFEST_FILE);
Expand Down
44 changes: 39 additions & 5 deletions test/javascript/node-package.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import semver from "semver";
import * as YAML from "yaml";
import { Project } from "../../src";
import { NodePackage } from "../../src/javascript/node-package";
import { minVersion } from "../../src/javascript/util";
import * as util from "../../src/util";
import { mkdtemp, synthSnapshot, TestProject } from "../util";

Expand Down Expand Up @@ -46,7 +47,7 @@ function mockYarnInstall(
depRanges[depName].push("*");
depVersions[depName].push(latestPackages[depName]);
} else {
const ver = semver.minVersion(`${depVer}`)?.version;
const ver = minVersion(`${depVer}`);
if (!ver) {
throw new Error(
`unable to determine min version for ${depName}@${depVer}`
Expand All @@ -55,6 +56,7 @@ function mockYarnInstall(
// If we're given a latest package, no dependency can require higher
if (
latestPackages[depName] &&
semver.validRange(`${ver}`) &&
semver.gt(`${ver}`, latestPackages[depName])
) {
throw new Error(
Expand All @@ -69,10 +71,16 @@ function mockYarnInstall(
}
// Resolve version to install that satisfies all ranges for dep
for (const dep of Object.keys(depRanges)) {
const installVersion = semver.maxSatisfying(
depVersions[dep],
depRanges[dep].join(" || ")
);
let installVersion: string | null;
if (!semver.validRange(depVersions[dep][0])) {
// if a dependency is "file:..." or something else, just install that
installVersion = depVersions[dep][0];
} else {
installVersion = semver.maxSatisfying(
depVersions[dep],
depRanges[dep].join(" || ")
);
}
if (!installVersion) {
throw new Error(`No version given satisfies all constraints on ${dep}`);
}
Expand Down Expand Up @@ -337,3 +345,29 @@ test("devDependencies are not pinned by peerDependencies if pinnedDevDependency
expect(pkgFile.peerDependencies).toStrictEqual({ ms: "^1.4.0" });
expect(pkgFile.devDependencies).toBeUndefined();
});

test("file path dependencies are respected", () => {
// Post-synth dependency version resolution uses installed package from node_modules folder
// Mock install command to add this folder with a fixed dependency version,
// mimicking yarn installing the latest package for "*"
jest.spyOn(util, "exec").mockImplementation((command, options) => {
expect(command.startsWith("yarn install")).toBeTruthy();
mockYarnInstall(options.cwd, { ms: "file:../ms" });
});

const project = new Project({ name: "test" });
const pkg = new NodePackage(project, {
peerDependencyOptions: {
pinnedDevDependency: false,
},
});

pkg.addPeerDeps("ms@file:../ms");

project.synth();

const pkgFile = readJsonSync(join(project.outdir, "package.json"));

expect(pkgFile.peerDependencies).toStrictEqual({ ms: "file:../ms" });
expect(pkgFile.devDependencies).toBeUndefined();
});