Uh oh!
There was an error while loading. Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork 11.8k
build: add pre-release checks for dependency on framework#24772
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
Uh oh!
There was an error while loading. Please reload this page.
Changes from all commits
File filter
Filter by extension
Conversations
Uh oh!
There was an error while loading. Please reload this page.
Jump to
Uh oh!
There was an error while loading. Please reload this page.
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,54 @@ | ||
| /** | ||
| * @license | ||
| * Copyright Google LLC All Rights Reserved. | ||
| * | ||
| * Use of this source code is governed by an MIT-style license that can be | ||
| * found in the LICENSE file at https://angular.io/license | ||
| */ | ||
| import semver from 'semver'; | ||
| import { Log, bold, ReleasePrecheckError } from '@angular/ng-dev'; | ||
| import { checkPeerDependencies } from './peer-deps-check.mjs'; | ||
| import { checkSchematicsAngularLatestVersion } from './latest-versions-check.mjs'; | ||
| import { PackageMap } from '../../../lib/packages.js'; | ||
| /** Environment variable that can be used to skip this pre-check. */ | ||
| const skipEnvVar = 'SKIP_DEPENDENCY_RANGE_PRECHECK'; | ||
| /** | ||
| * Ensures that dependency ranges are properly updated before publishing | ||
| * a new version. This check includes: | ||
| * | ||
| * - checking of `latest-versions.ts` of `@schematics/angular`. | ||
| * - checking of peer dependencies in `@angular-devkit/build-angular`. | ||
| * | ||
| * @throws {ReleasePrecheckError} If validation fails. | ||
| */ | ||
| export async function assertValidDependencyRanges( | ||
| newVersion: semver.SemVer, | ||
| allPackages: PackageMap, | ||
| ) { | ||
| if (process.env[skipEnvVar] === '1') { | ||
| return; | ||
| } | ||
| const failures: string[] = [ | ||
| ...(await checkPeerDependencies(newVersion, allPackages)), | ||
| ...(await checkSchematicsAngularLatestVersion(newVersion)), | ||
| ]; | ||
| if (failures.length !== 0) { | ||
| Log.error('Discovered errors when validating dependency ranges.'); | ||
| for (const f of failures) { | ||
| Log.error(` - ${bold(f)}`); | ||
| } | ||
| Log.warn(); | ||
| Log.warn('Please fix these failures before publishing a new release.'); | ||
| Log.warn(`These checks can be forcibly ignored by setting: ${skipEnvVar}=1`); | ||
| Log.warn(); | ||
| throw new ReleasePrecheckError(); | ||
| } | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,37 @@ | ||
| /** | ||
| * @license | ||
| * Copyright Google LLC All Rights Reserved. | ||
| * | ||
| * Use of this source code is governed by an MIT-style license that can be | ||
| * found in the LICENSE file at https://angular.io/license | ||
| */ | ||
| import semver from 'semver'; | ||
| export async function checkSchematicsAngularLatestVersion( | ||
| newVersion: semver.SemVer, | ||
| ): Promise<string[]> { | ||
| const { | ||
| default: { latestVersions }, | ||
| } = await import('../../../packages/schematics/angular/utility/latest-versions.js'); | ||
| const keysToCheck = ['ng-packagr', 'Angular']; | ||
| const { major, minor } = newVersion; | ||
| const isPrerelease = !!newVersion.prerelease[0]; | ||
| const failures: string[] = []; | ||
| let expectedFwDep = `^${major}.${minor}.0`; | ||
| if (isPrerelease) { | ||
| expectedFwDep = `^${major}.${minor}.0-next.0`; | ||
| } | ||
| for (const key of keysToCheck) { | ||
| if (latestVersions[key] !== expectedFwDep) { | ||
| failures.push( | ||
| `latest-versions: Invalid dependency range for "${key}". Expected: ${expectedFwDep}`, | ||
| ); | ||
| } | ||
| } | ||
devversion marked this conversation as resolved.
Uh oh!There was an error while loading. Please reload this page. | ||
| return failures; | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,74 @@ | ||
| /** | ||
| * @license | ||
| * Copyright Google LLC All Rights Reserved. | ||
| * | ||
| * Use of this source code is governed by an MIT-style license that can be | ||
| * found in the LICENSE file at https://angular.io/license | ||
| */ | ||
| import path from 'path'; | ||
| import url from 'url'; | ||
| import semver from 'semver'; | ||
| import { PackageMap } from '../../../lib/packages.js'; | ||
| /** Path to the current directory. */ | ||
| const currentDir = path.dirname(url.fileURLToPath(import.meta.url)); | ||
| /** Path to the project directory. */ | ||
| const projectDir = path.join(currentDir, '../../../'); | ||
devversion marked this conversation as resolved.
Uh oh!There was an error while loading. Please reload this page. | ||
| /** Describes a parsed `package.json` file. */ | ||
| interface PackageJson { | ||
| name?: string; | ||
| peerDependencies?: Record<string, string>; | ||
| } | ||
| export async function checkPeerDependencies( | ||
| newVersion: semver.SemVer, | ||
| allPackages: PackageMap, | ||
| ): Promise<string[]> { | ||
| const { major, minor } = newVersion; | ||
| const isPrerelease = !!newVersion.prerelease[0]; | ||
| const isMajor = minor === 0; | ||
| let expectedFwPeerDep = `^${major}.0.0`; | ||
| if (isMajor && isPrerelease) { | ||
| expectedFwPeerDep = `^${major}.0.0-next.0`; | ||
| } else if (isPrerelease) { | ||
| expectedFwPeerDep = `^${major}.0.0 || ^${major}.${minor}.0-next.0`; | ||
| } | ||
| const failures: string[] = []; | ||
| for (const pkgInfo of Object.values(allPackages)) { | ||
| failures.push(...checkPackage(pkgInfo.packageJson, expectedFwPeerDep)); | ||
| } | ||
Collaborator There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Consider: constfailures=Object.values(allPackages).flatMap((pkgInfo)=>checkPackage(pkgInfo.packageJson,expectedFwPeerDep));MemberAuthor There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Fair enough. I think it comes down to personal preference. Please feel free to make this change if you feel like it | ||
| return failures; | ||
| } | ||
| /** Checks the given package and collects errors for the peer dependency ranges. */ | ||
| function checkPackage(pkgJson: PackageJson, expectedFwPeerDep: string): string[] { | ||
| if (pkgJson.peerDependencies === undefined) { | ||
| return []; | ||
| } | ||
| const failures: string[] = []; | ||
devversion marked this conversation as resolved.
Uh oh!There was an error while loading. Please reload this page. | ||
| for (const [depName, range] of Object.entries(pkgJson.peerDependencies)) { | ||
| // Even though `ng-packagr` might not strictly follow the same release schedules | ||
| // like official Angular packages, we generally expect it to match. It's better | ||
| // flagging it than silently passing pre-checks. The caretaker can always forcibly | ||
| // ignore this check. | ||
| if (!depName.startsWith('@angular/') && depName !== 'ng-packagr') { | ||
| continue; | ||
| } | ||
| if (range !== expectedFwPeerDep) { | ||
| failures.push( | ||
| `${pkgJson.name}: Unexpected peer dependency range for "${depName}". Expected: ${expectedFwPeerDep}`, | ||
| ); | ||
| } | ||
| } | ||
| return failures; | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,11 @@ | ||
| { | ||
| "extends": "../../tsconfig.json", | ||
| "compilerOptions": { | ||
| "module": "Node16", | ||
| "moduleResolution": "Node16", | ||
| "noEmit": true, | ||
| "types": [] | ||
| }, | ||
| "include": ["**/*.mts"], | ||
| "exclude": [] | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nit: I could see someone trying
SKIP_DEPENDENCY_RANGE_PRECHECK="true"and being confused when that doesn't work. I wonder if we should skip if the variable is truthy or at least throw an error that the only valid value is1or unset?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Honestly, I don't know it's worth doing that. the error is pretty clear about what needs to be used, and it's also only the experienced caretakers running the release process.