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

Compel users to release new versions of dependencies alongside their dependents #102

Merged
merged 13 commits into from
Oct 11, 2023
Merged
155 changes: 155 additions & 0 deletions src/release-specification.test.ts
cryptodev-2s marked this conversation as resolved.
Show resolved Hide resolved
Original file line number Diff line number Diff line change
Expand Up @@ -683,5 +683,160 @@ ${releaseSpecificationPath}
);
});
});

it("throws if there are any packages not listed in the release which have changed, and are being defined as 'dependencies' by other packages which are listed", async () => {
cryptodev-2s marked this conversation as resolved.
Show resolved Hide resolved
await withSandbox(async (sandbox) => {
const project = buildMockProject({
workspacePackages: {
a: buildMockPackage('a', {
hasChangesSinceLatestRelease: true,
unvalidatedManifest: {
dependencies: {
b: '1.0.0',
},
},
}),
b: buildMockPackage('b', {
hasChangesSinceLatestRelease: true,
}),
},
});
const releaseSpecificationPath = path.join(
sandbox.directoryPath,
'release-spec',
);
await fs.promises.writeFile(
releaseSpecificationPath,
YAML.stringify({
packages: {
a: 'minor',
},
}),
);

await expect(
validateReleaseSpecification(project, releaseSpecificationPath),
).rejects.toThrow(
`
Your release spec could not be processed due to the following issues:

* The following packages, which are dependencies of the package 'a' being released, are missing from the release spec.

- b

These packages may have changes that 'a' relies upon. Consider including them in the release spec.

If you are ABSOLUTELY SURE these packages are safe to omit, however, and want to postpone the release of a package, then list it with a directive of "intentionally-skip". For example:

packages:
b: intentionally-skip

The release spec file has been retained for you to edit again and make the necessary fixes. Once you've done this, re-run this tool.

${releaseSpecificationPath}
`.trim(),
);
});
});

it("throws if there are any packages not listed in the release which have changed, and are being defined as 'peerDependencies' by other packages which are listed", async () => {
cryptodev-2s marked this conversation as resolved.
Show resolved Hide resolved
await withSandbox(async (sandbox) => {
const project = buildMockProject({
workspacePackages: {
a: buildMockPackage('a', {
hasChangesSinceLatestRelease: true,
unvalidatedManifest: {
peerDependencies: {
b: '1.0.0',
},
},
}),
b: buildMockPackage('b', {
hasChangesSinceLatestRelease: true,
}),
},
});
const releaseSpecificationPath = path.join(
sandbox.directoryPath,
'release-spec',
);
await fs.promises.writeFile(
releaseSpecificationPath,
YAML.stringify({
packages: {
a: 'minor',
},
}),
);

await expect(
validateReleaseSpecification(project, releaseSpecificationPath),
).rejects.toThrow(
`
Your release spec could not be processed due to the following issues:

* The following packages, which are dependencies of the package 'a' being released, are missing from the release spec.

- b

These packages may have changes that 'a' relies upon. Consider including them in the release spec.

If you are ABSOLUTELY SURE these packages are safe to omit, however, and want to postpone the release of a package, then list it with a directive of "intentionally-skip". For example:

packages:
b: intentionally-skip

The release spec file has been retained for you to edit again and make the necessary fixes. Once you've done this, re-run this tool.

${releaseSpecificationPath}
`.trim(),
);
});
});

it("does not throw when a package defined as 'dependencies' by a listed package in the release has not changed", async () => {
cryptodev-2s marked this conversation as resolved.
Show resolved Hide resolved
await withSandbox(async (sandbox) => {
const project = buildMockProject({
workspacePackages: {
a: buildMockPackage('a', {
hasChangesSinceLatestRelease: true,
unvalidatedManifest: {
dependencies: {
b: '1.0.0',
c: '2.0.0',
},
},
}),
b: buildMockPackage('b', {
hasChangesSinceLatestRelease: false,
}),
},
});
const releaseSpecificationPath = path.join(
sandbox.directoryPath,
'release-spec',
);
await fs.promises.writeFile(
releaseSpecificationPath,
YAML.stringify({
packages: {
a: 'minor',
},
}),
);

const releaseSpecification = await validateReleaseSpecification(
project,
releaseSpecificationPath,
);

expect(releaseSpecification).toStrictEqual({
packages: {
a: 'minor',
},
path: releaseSpecificationPath,
});
});
});
});
});
75 changes: 68 additions & 7 deletions src/release-specification.ts
Original file line number Diff line number Diff line change
Expand Up @@ -211,9 +211,23 @@ export async function validateReleaseSpecification(
project.workspacePackages[packageName].hasChangesSinceLatestRelease,
);
const missingChangedPackageNames = changedPackageNames.filter(
cryptodev-2s marked this conversation as resolved.
Show resolved Hide resolved
(packageName) =>
!hasProperty(unvalidatedReleaseSpecification.packages, packageName) ||
unvalidatedReleaseSpecification.packages[packageName] === null,
(packageName) => {
const isADependency = Object.values(project.workspacePackages).some(
cryptodev-2s marked this conversation as resolved.
Show resolved Hide resolved
cryptodev-2s marked this conversation as resolved.
Show resolved Hide resolved
(p) => {
cryptodev-2s marked this conversation as resolved.
Show resolved Hide resolved
const { dependencies, peerDependencies } = p.unvalidatedManifest;
cryptodev-2s marked this conversation as resolved.
Show resolved Hide resolved
return (
(dependencies && hasProperty(dependencies, packageName)) ||
(peerDependencies && hasProperty(peerDependencies, packageName))
);
},
);

return (
(!hasProperty(unvalidatedReleaseSpecification.packages, packageName) ||
unvalidatedReleaseSpecification.packages[packageName] === null) &&
cryptodev-2s marked this conversation as resolved.
Show resolved Hide resolved
!isADependency
);
},
);

if (missingChangedPackageNames.length > 0) {
Expand Down Expand Up @@ -241,10 +255,8 @@ export async function validateReleaseSpecification(
});
}

Object.keys(unvalidatedReleaseSpecification.packages).forEach(
(packageName, index) => {
const versionSpecifierOrDirective =
unvalidatedReleaseSpecification.packages[packageName];
Object.entries(unvalidatedReleaseSpecification.packages).forEach(
([packageName, versionSpecifierOrDirective], index) => {
const lineNumber = indexOfFirstUsableLine + index + 2;
const pkg = project.workspacePackages[packageName];

Expand Down Expand Up @@ -301,6 +313,55 @@ export async function validateReleaseSpecification(
});
}
}

// Check to compel users to release new versions of dependencies alongside their dependents
if (
pkg &&
versionSpecifierOrDirective &&
(hasProperty(IncrementableVersionParts, versionSpecifierOrDirective) ||
isValidSemver(versionSpecifierOrDirective))
) {
const missingDependencies = Object.keys({
...(pkg.unvalidatedManifest.dependencies || {}),
...(pkg.unvalidatedManifest.peerDependencies || {}),
cryptodev-2s marked this conversation as resolved.
Show resolved Hide resolved
}).filter((dependency) => {
if (
!project.workspacePackages[dependency]?.hasChangesSinceLatestRelease
) {
return false;
}

const dependencyVersionSpecifierOrDirective =
unvalidatedReleaseSpecification.packages[dependency];

return !dependencyVersionSpecifierOrDirective;
cryptodev-2s marked this conversation as resolved.
Show resolved Hide resolved
});

if (missingDependencies.length > 0) {
errors.push({
message: [
`The following packages, which are dependencies of the package '${packageName}' being released, are missing from the release spec.`,
cryptodev-2s marked this conversation as resolved.
Show resolved Hide resolved
missingDependencies
.map((dependency) => ` - ${dependency}`)
.join('\n'),
` These packages may have changes that '${packageName}' relies upon. Consider including them in the release spec.`,
` If you are ABSOLUTELY SURE these packages are safe to omit, however, and want to postpone the release of a package, then list it with a directive of "intentionally-skip". For example:`,
YAML.stringify({
packages: missingDependencies.reduce((object, dependency) => {
return {
...object,
[dependency]: INTENTIONALLY_SKIP_PACKAGE_DIRECTIVE,
};
}, {}),
})
.trim()
.split('\n')
.map((line) => ` ${line}`)
.join('\n'),
].join('\n\n'),
});
}
}
},
);

Expand Down
9 changes: 7 additions & 2 deletions tests/unit/helpers.ts
cryptodev-2s marked this conversation as resolved.
Show resolved Hide resolved
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,10 @@ import { SemVer } from 'semver';
import { isPlainObject } from '@metamask/utils';
import type { Package } from '../../src/package';
import { PackageManifestFieldNames } from '../../src/package-manifest';
import type { ValidatedPackageManifest } from '../../src/package-manifest';
import type {
UnvalidatedPackageManifest,
ValidatedPackageManifest,
} from '../../src/package-manifest';
import type { Project } from '../../src/project';

/**
Expand Down Expand Up @@ -35,6 +38,7 @@ type MockPackageOverrides = Omit<
Partial<ValidatedPackageManifest>,
PackageManifestFieldNames.Name | PackageManifestFieldNames.Version
>;
unvalidatedManifest?: UnvalidatedPackageManifest;
};

/**
Expand Down Expand Up @@ -102,6 +106,7 @@ export function buildMockPackage(

const {
validatedManifest = {},
unvalidatedManifest = {},
directoryPath = `/path/to/packages/${name}`,
manifestPath = path.join(directoryPath, 'package.json'),
changelogPath = path.join(directoryPath, 'CHANGELOG.md'),
Expand All @@ -110,7 +115,7 @@ export function buildMockPackage(

return {
directoryPath,
unvalidatedManifest: {},
unvalidatedManifest,
validatedManifest: buildMockManifest({
...validatedManifest,
[PackageManifestFieldNames.Name]: name,
Expand Down
Loading