refactor(dev-infra): improve type safety of NPM dist tags in release tool (#42133)

Instead of passing `string` in the release tool for NPM dist tags, we
should use a union string type that limits the tags to `latest`, `next`
and anything matching `v{number}-lts`. This avoids mistakes at
compilation-level if an invalid/unknown tag would be set by a release
action.

PR Close #42133
This commit is contained in:
Paul Gschwendtner 2021-05-17 19:00:50 +02:00 committed by atscott
parent 6d1477664b
commit 48f49bacb4
9 changed files with 38 additions and 30 deletions

View File

@ -1120,7 +1120,7 @@ function fetchLongTermSupportBranchesFromNpm(config) {
// their corresponding branches. We assume that an LTS tagged version in NPM belongs to the // their corresponding branches. We assume that an LTS tagged version in NPM belongs to the
// last-minor branch of a given major (i.e. we assume there are no outdated LTS NPM dist tags). // last-minor branch of a given major (i.e. we assume there are no outdated LTS NPM dist tags).
for (const npmDistTag in distTags) { for (const npmDistTag in distTags) {
if (ltsNpmDistTagRegex.test(npmDistTag)) { if (isLtsDistTag(npmDistTag)) {
const version = semver.parse(distTags[npmDistTag]); const version = semver.parse(distTags[npmDistTag]);
const branchName = `${version.major}.${version.minor}.x`; const branchName = `${version.major}.${version.minor}.x`;
const majorReleaseDate = new Date(time[`${version.major}.0.0`]); const majorReleaseDate = new Date(time[`${version.major}.0.0`]);
@ -1142,6 +1142,10 @@ function fetchLongTermSupportBranchesFromNpm(config) {
return { active, inactive }; return { active, inactive };
}); });
} }
/** Gets whether the specified tag corresponds to a LTS dist tag. */
function isLtsDistTag(tagName) {
return ltsNpmDistTagRegex.test(tagName);
}
/** /**
* Computes the date when long-term support ends for a major released at the * Computes the date when long-term support ends for a major released at the
* specified date. * specified date.
@ -6269,7 +6273,7 @@ class ReleaseAction {
} }
/** /**
* Builds and publishes the given version in the specified branch. * Builds and publishes the given version in the specified branch.
* @param newVersion The new version to be published. * @param releaseNotes The release notes for the version being published.
* @param publishBranch Name of the branch that contains the new version. * @param publishBranch Name of the branch that contains the new version.
* @param npmDistTag NPM dist tag where the version should be published to. * @param npmDistTag NPM dist tag where the version should be published to.
*/ */

View File

@ -15,6 +15,7 @@ import {debug, error, green, info, promptConfirm, red, warn, yellow} from '../..
import {getListCommitsInBranchUrl, getRepositoryGitUrl} from '../../utils/git/github-urls'; import {getListCommitsInBranchUrl, getRepositoryGitUrl} from '../../utils/git/github-urls';
import {GitClient} from '../../utils/git/index'; import {GitClient} from '../../utils/git/index';
import {BuiltPackage, ReleaseConfig} from '../config/index'; import {BuiltPackage, ReleaseConfig} from '../config/index';
import {NpmDistTag} from '../versioning';
import {ActiveReleaseTrains} from '../versioning/active-release-trains'; import {ActiveReleaseTrains} from '../versioning/active-release-trains';
import {runNpmPublish} from '../versioning/npm-publish'; import {runNpmPublish} from '../versioning/npm-publish';
@ -440,12 +441,12 @@ export abstract class ReleaseAction {
/** /**
* Builds and publishes the given version in the specified branch. * Builds and publishes the given version in the specified branch.
* @param newVersion The new version to be published. * @param releaseNotes The release notes for the version being published.
* @param publishBranch Name of the branch that contains the new version. * @param publishBranch Name of the branch that contains the new version.
* @param npmDistTag NPM dist tag where the version should be published to. * @param npmDistTag NPM dist tag where the version should be published to.
*/ */
protected async buildAndPublish( protected async buildAndPublish(
releaseNotes: ReleaseNotes, publishBranch: string, npmDistTag: string) { releaseNotes: ReleaseNotes, publishBranch: string, npmDistTag: NpmDistTag) {
const versionBumpCommitSha = await this._getCommitOfBranch(publishBranch); const versionBumpCommitSha = await this._getCommitOfBranch(publishBranch);
if (!await this._isCommitForVersionStaging(releaseNotes.version, versionBumpCommitSha)) { if (!await this._isCommitForVersionStaging(releaseNotes.version, versionBumpCommitSha)) {
@ -482,7 +483,7 @@ export abstract class ReleaseAction {
} }
/** Publishes the given built package to NPM with the specified NPM dist tag. */ /** Publishes the given built package to NPM with the specified NPM dist tag. */
private async _publishBuiltPackageToNpm(pkg: BuiltPackage, npmDistTag: string) { private async _publishBuiltPackageToNpm(pkg: BuiltPackage, npmDistTag: NpmDistTag) {
debug(`Starting publish of "${pkg.name}".`); debug(`Starting publish of "${pkg.name}".`);
const spinner = ora.call(undefined).start(`Publishing "${pkg.name}"`); const spinner = ora.call(undefined).start(`Publishing "${pkg.name}"`);

View File

@ -7,23 +7,12 @@
*/ */
import {ListChoiceOptions, prompt} from 'inquirer'; import {ListChoiceOptions, prompt} from 'inquirer';
import * as semver from 'semver';
import {ActiveReleaseTrains} from '../../versioning/active-release-trains'; import {ActiveReleaseTrains} from '../../versioning/active-release-trains';
import {semverInc} from '../../versioning/inc-semver'; import {semverInc} from '../../versioning/inc-semver';
import {fetchLongTermSupportBranchesFromNpm} from '../../versioning/long-term-support'; import {fetchLongTermSupportBranchesFromNpm, LtsBranch} from '../../versioning/long-term-support';
import {ReleaseAction} from '../actions'; import {ReleaseAction} from '../actions';
/** Interface describing an LTS version branch. */
interface LtsBranch {
/** Name of the branch. */
name: string;
/** Most recent version for the given LTS branch. */
version: semver.SemVer;
/** NPM dist tag for the LTS version. */
npmDistTag: string;
}
/** /**
* Release action that cuts a new patch release for an active release-train in the long-term * Release action that cuts a new patch release for an active release-train in the long-term
* support phase. The patch segment is incremented. The changelog is generated for the new * support phase. The patch segment is incremented. The changelog is generated for the new

View File

@ -12,6 +12,7 @@ import * as semver from 'semver';
import {spawnWithDebugOutput} from '../../utils/child-process'; import {spawnWithDebugOutput} from '../../utils/child-process';
import {error, green, info, red} from '../../utils/console'; import {error, green, info, red} from '../../utils/console';
import {BuiltPackage} from '../config/index'; import {BuiltPackage} from '../config/index';
import {NpmDistTag} from '../versioning';
import {FatalReleaseActionError} from './actions-error'; import {FatalReleaseActionError} from './actions-error';
@ -36,7 +37,7 @@ import {FatalReleaseActionError} from './actions-error';
* Invokes the `ng-dev release set-dist-tag` command in order to set the specified * Invokes the `ng-dev release set-dist-tag` command in order to set the specified
* NPM dist tag for all packages in the checked out branch to the given version. * NPM dist tag for all packages in the checked out branch to the given version.
*/ */
export async function invokeSetNpmDistCommand(npmDistTag: string, version: semver.SemVer) { export async function invokeSetNpmDistCommand(npmDistTag: NpmDistTag, version: semver.SemVer) {
try { try {
// Note: No progress indicator needed as that is the responsibility of the command. // Note: No progress indicator needed as that is the responsibility of the command.
await spawnWithDebugOutput( await spawnWithDebugOutput(

View File

@ -11,7 +11,7 @@ import {join} from 'path';
import * as semver from 'semver'; import * as semver from 'semver';
import {getBranchPushMatcher} from '../../../utils/testing'; import {getBranchPushMatcher} from '../../../utils/testing';
import {_npmPackageInfoCache} from '../../versioning'; import {NpmDistTag} from '../../versioning';
import {ActiveReleaseTrains} from '../../versioning/active-release-trains'; import {ActiveReleaseTrains} from '../../versioning/active-release-trains';
import * as npm from '../../versioning/npm-publish'; import * as npm from '../../versioning/npm-publish';
import {ReleaseTrain} from '../../versioning/release-trains'; import {ReleaseTrain} from '../../versioning/release-trains';
@ -44,7 +44,7 @@ describe('common release action logic', () => {
fakeNpmPackageQueryRequest(releaseConfig.npmPackages[0], {'dist-tags': {}}); fakeNpmPackageQueryRequest(releaseConfig.npmPackages[0], {'dist-tags': {}});
for (const actionCtor of actions) { for (const actionCtor of actions) {
if (await actionCtor.isActive(testReleaseTrain)) { if (await actionCtor.isActive(testReleaseTrain, releaseConfig)) {
const action = new actionCtor(testReleaseTrain, gitClient, releaseConfig, testTmpDir); const action = new actionCtor(testReleaseTrain, gitClient, releaseConfig, testTmpDir);
descriptions.push(await action.getDescription()); descriptions.push(await action.getDescription());
} }
@ -151,7 +151,7 @@ class TestAction extends ReleaseAction {
throw Error('Not implemented.'); throw Error('Not implemented.');
} }
async testBuildAndPublish(version: semver.SemVer, publishBranch: string, distTag: string) { async testBuildAndPublish(version: semver.SemVer, publishBranch: string, distTag: NpmDistTag) {
const releaseNotes = await ReleaseNotes.fromRange(version, '', ''); const releaseNotes = await ReleaseNotes.fromRange(version, '', '');
await this.buildAndPublish(releaseNotes, publishBranch, distTag); await this.buildAndPublish(releaseNotes, publishBranch, distTag);
} }

View File

@ -17,7 +17,7 @@ import {getBranchPushMatcher, installVirtualGitClientSpies, VirtualGitClient} fr
import {ReleaseConfig} from '../../config/index'; import {ReleaseConfig} from '../../config/index';
import {ActiveReleaseTrains} from '../../versioning/active-release-trains'; import {ActiveReleaseTrains} from '../../versioning/active-release-trains';
import * as npm from '../../versioning/npm-publish'; import * as npm from '../../versioning/npm-publish';
import {_npmPackageInfoCache, NpmPackageInfo} from '../../versioning/npm-registry'; import {_npmPackageInfoCache, NpmDistTag, NpmPackageInfo} from '../../versioning/npm-registry';
import {ReleaseAction, ReleaseActionConstructor} from '../actions'; import {ReleaseAction, ReleaseActionConstructor} from '../actions';
import * as constants from '../constants'; import * as constants from '../constants';
import * as externalCommands from '../external-commands'; import * as externalCommands from '../external-commands';
@ -124,8 +124,8 @@ export function parse(version: string): semver.SemVer {
export async function expectStagingAndPublishWithoutCherryPick( export async function expectStagingAndPublishWithoutCherryPick(
action: TestReleaseAction, expectedBranch: string, expectedVersion: string, action: TestReleaseAction, expectedBranch: string, expectedVersion: string,
expectedNpmDistTag: string) { expectedNpmDistTag: NpmDistTag) {
const {repo, fork, gitClient, releaseConfig} = action; const {repo, fork, gitClient} = action;
const expectedStagingForkBranch = `release-stage-${expectedVersion}`; const expectedStagingForkBranch = `release-stage-${expectedVersion}`;
const expectedTagName = expectedVersion; const expectedTagName = expectedVersion;
@ -173,7 +173,7 @@ export async function expectStagingAndPublishWithoutCherryPick(
export async function expectStagingAndPublishWithCherryPick( export async function expectStagingAndPublishWithCherryPick(
action: TestReleaseAction, expectedBranch: string, expectedVersion: string, action: TestReleaseAction, expectedBranch: string, expectedVersion: string,
expectedNpmDistTag: string) { expectedNpmDistTag: NpmDistTag) {
const {repo, fork, gitClient, releaseConfig} = action; const {repo, fork, gitClient, releaseConfig} = action;
const expectedStagingForkBranch = `release-stage-${expectedVersion}`; const expectedStagingForkBranch = `release-stage-${expectedVersion}`;
const expectedCherryPickForkBranch = `changelog-cherry-pick-${expectedVersion}`; const expectedCherryPickForkBranch = `changelog-cherry-pick-${expectedVersion}`;

View File

@ -12,6 +12,9 @@ import {ReleaseConfig} from '../config/index';
import {fetchProjectNpmPackageInfo} from './npm-registry'; import {fetchProjectNpmPackageInfo} from './npm-registry';
/** Type describing a NPM dist tag indicating long-term support. */
export type LtsNpmDistTag = `v${number}-lts`;
/** Interface describing determined LTS branches. */ /** Interface describing determined LTS branches. */
export interface LtsBranches { export interface LtsBranches {
/** List of active LTS version branches. */ /** List of active LTS version branches. */
@ -27,7 +30,7 @@ export interface LtsBranch {
/** Most recent version for the given LTS branch. */ /** Most recent version for the given LTS branch. */
version: semver.SemVer; version: semver.SemVer;
/** NPM dist tag for the LTS version. */ /** NPM dist tag for the LTS version. */
npmDistTag: string; npmDistTag: LtsNpmDistTag;
} }
/** /**
@ -57,7 +60,7 @@ export async function fetchLongTermSupportBranchesFromNpm(config: ReleaseConfig)
// their corresponding branches. We assume that an LTS tagged version in NPM belongs to the // their corresponding branches. We assume that an LTS tagged version in NPM belongs to the
// last-minor branch of a given major (i.e. we assume there are no outdated LTS NPM dist tags). // last-minor branch of a given major (i.e. we assume there are no outdated LTS NPM dist tags).
for (const npmDistTag in distTags) { for (const npmDistTag in distTags) {
if (ltsNpmDistTagRegex.test(npmDistTag)) { if (isLtsDistTag(npmDistTag)) {
const version = semver.parse(distTags[npmDistTag])!; const version = semver.parse(distTags[npmDistTag])!;
const branchName = `${version.major}.${version.minor}.x`; const branchName = `${version.major}.${version.minor}.x`;
const majorReleaseDate = new Date(time[`${version.major}.0.0`]); const majorReleaseDate = new Date(time[`${version.major}.0.0`]);
@ -80,6 +83,11 @@ export async function fetchLongTermSupportBranchesFromNpm(config: ReleaseConfig)
return {active, inactive}; return {active, inactive};
} }
/** Gets whether the specified tag corresponds to a LTS dist tag. */
export function isLtsDistTag(tagName: string): tagName is LtsNpmDistTag {
return ltsNpmDistTagRegex.test(tagName);
}
/** /**
* Computes the date when long-term support ends for a major released at the * Computes the date when long-term support ends for a major released at the
* specified date. * specified date.
@ -93,7 +101,7 @@ export function computeLtsEndDateOfMajor(majorReleaseDate: Date): Date {
} }
/** Gets the long-term support NPM dist tag for a given major version. */ /** Gets the long-term support NPM dist tag for a given major version. */
export function getLtsNpmDistTagOfMajor(major: number): string { export function getLtsNpmDistTagOfMajor(major: number): LtsNpmDistTag {
// LTS versions should be tagged in NPM in the following format: `v{major}-lts`. // LTS versions should be tagged in NPM in the following format: `v{major}-lts`.
return `v${major}-lts`; return `v${major}-lts` as const;
} }

View File

@ -8,13 +8,14 @@
import * as semver from 'semver'; import * as semver from 'semver';
import {spawnInteractiveCommand, spawnWithDebugOutput} from '../../utils/child-process'; import {spawnInteractiveCommand, spawnWithDebugOutput} from '../../utils/child-process';
import {NpmDistTag} from './npm-registry';
/** /**
* Runs NPM publish within a specified package directory. * Runs NPM publish within a specified package directory.
* @throws With the process log output if the publish failed. * @throws With the process log output if the publish failed.
*/ */
export async function runNpmPublish( export async function runNpmPublish(
packagePath: string, distTag: string, registryUrl: string|undefined) { packagePath: string, distTag: NpmDistTag, registryUrl: string|undefined) {
const args = ['publish', '--access', 'public', '--tag', distTag]; const args = ['publish', '--access', 'public', '--tag', distTag];
// If a custom registry URL has been specified, add the `--registry` flag. // If a custom registry URL has been specified, add the `--registry` flag.
if (registryUrl !== undefined) { if (registryUrl !== undefined) {

View File

@ -10,6 +10,10 @@ import fetch from 'node-fetch';
import * as semver from 'semver'; import * as semver from 'semver';
import {ReleaseConfig} from '../config/index'; import {ReleaseConfig} from '../config/index';
import {LtsNpmDistTag} from './long-term-support';
/** Type describing the possible NPM dist tags used by Angular packages. */
export type NpmDistTag = 'latest'|'next'|LtsNpmDistTag;
/** Type describing an NPM package fetched from the registry. */ /** Type describing an NPM package fetched from the registry. */
export interface NpmPackageInfo { export interface NpmPackageInfo {