Skip to content

Commit

Permalink
fix(node-packages): pin peer dependencies with undefined version (pro…
Browse files Browse the repository at this point in the history 
…jen#1609)

fixes projen#1611 

Peer dependencies added without a fixed version are being added to devDependencies in package.json as `name@^version` even with `pinnedDevDependency=true`, which is throwing a JSII warning. This change adds another pass through the peer dependencies in the post-synth step to pin those peer dependencies who's version is not known during presynth. While this causes a degree of code duplication, I decided against replacing the pre-synth peerDependency pass as it minimises the impact of this change on existing workflows.

This change is tested both in mocked unit tests and through yarn link to a local project, where it displayed the expected behaviour, including correctly respecting any existing versioning in `package.json`.

---
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
  • Loading branch information
@cgatt
cgatt authored Feb 24, 2022
1 parent 38127d7 commit 1811df3
Show file tree
Hide file tree
Showing 2 changed files with 249 additions and 38 deletions.
45 changes: 35 additions & 10 deletions src/javascript/node-package.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -403,7 +403,10 @@ export class NodePackage extends Component {
super(project);

this.packageName = options.packageName ?? project.name;
this.peerDependencyOptions = options.peerDependencyOptions ?? {};
this.peerDependencyOptions = {
pinnedDevDependency: true,
...options.peerDependencyOptions,
};
this.allowLibraryDependencies = options.allowLibraryDependencies ?? true;
this.packageManager = options.packageManager ?? NodePackageManager.YARN;
this.entrypoint = options.entrypoint ?? "lib/index.js";
Expand Down Expand Up @@ -887,8 +890,7 @@ export class NodePackage extends Component {

// synthetic dependencies: add a pinned build dependency to ensure we are
// testing against the minimum requirement of the peer.
const pinned = this.peerDependencyOptions.pinnedDevDependency ?? true;
if (pinned) {
if (this.peerDependencyOptions.pinnedDevDependency) {
for (const dep of this.project.deps.all.filter(
(d) => d.type === DependencyType.PEER
)) {
Expand Down Expand Up @@ -1056,23 +1058,46 @@ export class NodePackage extends Component {
}
}

return sorted(result);
return result;
};

const rendered = this._renderedDeps;
if (!rendered) {
throw new Error("assertion failed");
}
pkg.dependencies = resolveDeps(pkg.dependencies, rendered.dependencies);
pkg.devDependencies = resolveDeps(
pkg.devDependencies,
rendered.devDependencies
);
pkg.peerDependencies = resolveDeps(

const deps = resolveDeps(pkg.dependencies, rendered.dependencies);
const devDeps = resolveDeps(pkg.devDependencies, rendered.devDependencies);
const peerDeps = resolveDeps(
pkg.peerDependencies,
rendered.peerDependencies
);

if (this.peerDependencyOptions.pinnedDevDependency) {
for (const [name, version] of Object.entries(peerDeps)) {
// Skip if we already have a runtime dependency on this peer
// or if devDependency version is already set.
// Relies on the "*" devDependency added in the presynth step
if (deps[name] || rendered.devDependencies[name] !== "*") {
continue;
}

// Take version and pin as dev dependency
const ver = semver.minVersion(version)?.version;
if (!ver) {
throw new Error(
`unable to determine minimum semver for peer dependency ${name}@${version}`
);
}

devDeps[name] = ver;
}
}

pkg.dependencies = sorted(deps);
pkg.devDependencies = sorted(devDeps);
pkg.peerDependencies = sorted(peerDeps);

const updated = JSON.stringify(pkg, undefined, 2);

if (original === updated) {
Expand Down
242 changes: 214 additions & 28 deletions test/javascript/node-package.test.ts
View file
Original file line number Diff line number Diff line change
@@ -1,11 +1,101 @@
import { readFileSync, writeFileSync, mkdirSync } from "fs";
import { join } from "path";
import { readJsonSync } from "fs-extra";
import semver from "semver";
import * as YAML from "yaml";
import { Project } from "../../src";
import { NodePackage } from "../../src/javascript/node-package";
import * as util from "../../src/util";
import { mkdtemp, synthSnapshot, TestProject } from "../util";

/**
* Mocks a yarn install, writing to yarn.lock
* and creating package.json files in node_modules for all dependencies
* Will "install" the max version that complies with all dependency ranges
* NOT A PERFECT MODEL OF YARN. JUST CLOSE ENOUGH.
* @param outdir Test project's outdir, where package.json and node_modules live
* @param latestPackages Package name and version to "install" for "*" deps
*/
function mockYarnInstall(
outdir: string,
latestPackages: Record<string, string>
) {
const pkgJson = readJsonSync(join(outdir, "package.json"));
const yarnLock: Record<string, string> = {};
const depRanges: Record<string, string[]> = {};
const depVersions: Record<string, string[]> = {};
const depTypes = [
"dependencies",
"devDependencies",
"peerDependencies",
"bundledDependencies",
"optionalDependencies",
];
for (const depType of depTypes) {
// Look up the ranges for each dependency
for (const [depName, depVer] of Object.entries(pkgJson[depType] ?? {})) {
// init arrays if undefined
depRanges[depName] = depRanges[depName] ?? [];
depVersions[depName] = depVersions[depName] ?? [];
if (depVer === "*") {
if (!latestPackages[depName]) {
throw new Error(
`No latest package defined for dependency ${depName}`
);
}
depRanges[depName].push("*");
depVersions[depName].push(latestPackages[depName]);
} else {
const ver = semver.minVersion(`${depVer}`)?.version;
if (!ver) {
throw new Error(
`unable to determine min version for ${depName}@${depVer}`
);
}
// If we're given a latest package, no dependency can require higher
if (
latestPackages[depName] &&
semver.gt(`${ver}`, latestPackages[depName])
) {
throw new Error(
`${depType} requirement ${depName}@${depVer} exceeds provided "latest" version ${latestPackages[depName]}`
);
}
depRanges[depName].push(`${depVer}`);
// Also take the min version as a valid install version
depVersions[depName].push(ver);
}
}
}
// 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(" || ")
);
if (!installVersion) {
throw new Error(`No version given satisfies all constraints on ${dep}`);
}
mkdirSync(join(outdir, `node_modules/${dep}`), { recursive: true });
writeFileSync(
join(outdir, `node_modules/${dep}/package.json`),
JSON.stringify({
name: `${dep}`,
version: `${installVersion}`,
})
);
// Not accurate to yaml.lock v1 format, but close enough.
yarnLock[
`${depRanges[dep].map((range) => `${dep}@${range}`).join(", ")}`
] = `version "${installVersion}"`;
}
// Use double quoted keys just to make output more predictable
writeFileSync(
join(outdir, "yarn.lock"),
YAML.stringify(yarnLock, { defaultKeyType: "QUOTE_DOUBLE" })
);
}

afterEach(() => {
jest.resetAllMocks();
jest.restoreAllMocks();
Expand Down Expand Up @@ -56,33 +146,7 @@ test('lockfile updated (install twice) after "*"s are resolved', () => {
.spyOn(util, "exec")
.mockImplementation((command, options) => {
expect(command.startsWith("yarn install")).toBeTruthy();

const pkgJson = readJsonSync(join(options.cwd, "package.json"));
const ver = pkgJson.dependencies.ms;

// if the version in package.json is "*", simulate "yarn install" by
// creating a node_modules entry and a yarn.lock file with "*"
if (ver === "*") {
mkdirSync(join(options.cwd, "node_modules/ms"), { recursive: true });
writeFileSync(
join(options.cwd, "node_modules/ms/package.json"),
JSON.stringify({
name: "ms",
version: "2.1.3",
})
);

writeFileSync(join(options.cwd, "yarn.lock"), "ms: *");
return;
}

// if there is a specific version, just write it to yarn.lock
if (ver) {
writeFileSync(join(options.cwd, "yarn.lock"), `ms: ${ver}`);
return;
}

throw new Error(`unexpected version: ${ver}`);
mockYarnInstall(options.cwd, { ms: "2.1.3" });
});

const project = new Project({ name: "test" });
Expand All @@ -95,7 +159,7 @@ test('lockfile updated (install twice) after "*"s are resolved', () => {
const yarnLockPath = join(project.outdir, "yarn.lock");
const yarnLock: string | undefined = readFileSync(yarnLockPath, "utf8");

expect(yarnLock).toStrictEqual("ms: ^2.1.3");
expect(yarnLock).toStrictEqual('"ms@^2.1.3": version "2.1.3"\n');
expect(execMock).toBeCalledTimes(2);
});

Expand Down Expand Up @@ -151,3 +215,125 @@ test("no install if package.json did not change at all", () => {
project.synth();
expect(execMock).not.toBeCalled();
});

test('"*" peer dependencies are pinned in devDependencies', () => {
// 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: "1.2.3" });
});

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

pkg.addPeerDeps("ms");

project.synth();

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

expect(pkgFile.peerDependencies).toStrictEqual({ ms: "^1.2.3" });
expect(pkgFile.devDependencies).toStrictEqual({ ms: "1.2.3" });
});

test("manually set devDependencies are not changed when a peerDependency is added", () => {
// 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: "1.3.4" });
});

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

const orig = {
name: "test",
devDependencies: {
ms: "^1.3.0",
},
main: "lib/index.js",
license: "Apache-2.0",
version: "0.0.0",
"//": '~~ Generated by projen. To modify, edit .projenrc.js and run "npx projen".',
};

writeFileSync(
join(project.outdir, "package.json"),
JSON.stringify(orig, undefined, 2)
);

pkg.addPeerDeps("ms");

project.synth();

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

expect(pkgFile.peerDependencies).toStrictEqual({ ms: "^1.3.4" });
expect(pkgFile.devDependencies).toStrictEqual({ ms: "^1.3.0" });
});

test("devDependencies are not pinned by peerDependencies if a regular (runtime) dependency also exists", () => {
// 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: "1.3.8" });
});

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

pkg.addDeps("ms");
pkg.addPeerDeps("ms");

project.synth();

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

expect(pkgFile.peerDependencies).toStrictEqual({ ms: "^1.3.8" });
expect(pkgFile.dependencies).toStrictEqual({ ms: "^1.3.8" });
expect(pkgFile.devDependencies).toBeUndefined();
});

test("devDependencies are not pinned by peerDependencies if pinnedDevDependency is false", () => {
// 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: "1.4.0" });
});

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

pkg.addPeerDeps("ms");

project.synth();

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

expect(pkgFile.peerDependencies).toStrictEqual({ ms: "^1.4.0" });
expect(pkgFile.devDependencies).toBeUndefined();
});

0 comments on commit 1811df3

Please sign in to comment.