Skip to content

Commit

Permalink
fix(upgrade-dependencies): some dependencies are still attempted to b…
Browse files Browse the repository at this point in the history
…e upgraded by ncu, then reverted by projen (#3463)

Fixes #3462

---
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
  • Loading branch information
dkershner6 committed Apr 5, 2024
1 parent cdc2ea0 commit ab74dab
Show file tree
Hide file tree
Showing 8 changed files with 71 additions and 17 deletions.
4 changes: 2 additions & 2 deletions .projen/tasks.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

22 changes: 20 additions & 2 deletions src/javascript/upgrade-dependencies.ts
Original file line number Diff line number Diff line change
Expand Up @@ -372,8 +372,11 @@ export class UpgradeDependencies extends Component {
const dependencies = [];

const deps = this.project.deps.all
// remove those that have a pinned version
.filter((d) => includeConstraint || !(d.version && d.version[0] === "^"))
// remove those that have a constraint version (unless includeConstraint is true)
.filter(
(d) =>
includeConstraint || this.packageCanBeUpgradedInPackageJson(d.version)
)
// remove override dependencies
.filter((d) => d.type !== DependencyType.OVERRIDE);

Expand All @@ -388,6 +391,21 @@ export class UpgradeDependencies extends Component {
return dependencies.map((d) => d.name);
}

/**
* Projen can alter a package's version in package.json when either the version is omitted, or set to "*".
* Otherwise, the exact version selected is placed in the package.json file and upgrading is handled through the package manager
* rather than npm-check-updates.
*
* @param version semver from DependencyCoordinates.version, may be undefined
* @returns whether the version is the default versioning behavior
*/
private packageCanBeUpgradedInPackageJson(
version: string | undefined
): boolean {
// No version means "latest"
return !version || version === "*";
}

private createWorkflow(
task: Task,
github: GitHub,
Expand Down
4 changes: 2 additions & 2 deletions test/__snapshots__/integ.test.ts.snap

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion test/cdk/__snapshots__/jsii.test.ts.snap

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion test/cdk8s/cdk8s-app-project-ts.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -186,7 +186,7 @@ test("upgrade task ignores pinned versions", () => {
expect(tasks.upgrade.steps).toMatchInlineSnapshot(`
[
{
"exec": "npx npm-check-updates@16 --upgrade --target=minor --peer --dep=dev,peer,prod,optional --filter=@types/jest,cdk8s-cli,eslint-import-resolver-typescript,eslint-plugin-import,jest,projen,ts-jest,typescript,cdk8s-plus-22,cdk8s,constructs",
"exec": "npx npm-check-updates@16 --upgrade --target=minor --peer --dep=dev,peer,prod,optional --filter=@types/jest,eslint-import-resolver-typescript,eslint-plugin-import,jest,projen,ts-jest,typescript",
},
{
"exec": "yarn install --check-files",
Expand Down
48 changes: 42 additions & 6 deletions test/javascript/upgrade-dependencies.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -96,9 +96,9 @@ test("upgrades command includes all dependencies", () => {
`);
});

test("ncu upgrade command does not include dependencies with version constraints, but package manager upgrade does", () => {
test("ncu upgrade command does not include dependencies with any version constraint, but package manager upgrade does", () => {
const project = createProject({
deps: ["some-dep@^10"],
deps: ["some-dep@^10", "other-dep@10.0.0"],
});

const tasks = synthSnapshot(project)[TaskRuntime.MANIFEST_FILE].tasks;
Expand All @@ -113,6 +113,36 @@ test("ncu upgrade command does not include dependencies with version constraints
{
"exec": "yarn install --check-files",
},
{
"exec": "yarn upgrade constructs jest jest-junit projen standard-version other-dep some-dep",
},
{
"exec": "npx projen",
},
{
"spawn": "post-upgrade",
},
]
`);
});

test("ncu upgrade command should include dependencies with * versions, along with package manager upgrade", () => {
const project = createProject({
deps: ["some-dep@*"],
});

const tasks = synthSnapshot(project)[TaskRuntime.MANIFEST_FILE].tasks;

expect(tasks.upgrade.steps[0].exec).toContain("some-dep");
expect(tasks.upgrade.steps[2].exec).toContain("some-dep");
expect(tasks.upgrade.steps).toMatchInlineSnapshot(`
[
{
"exec": "npx npm-check-updates@16 --upgrade --target=minor --peer --dep=dev,peer,prod,optional --filter=jest,projen,some-dep",
},
{
"exec": "yarn install --check-files",
},
{
"exec": "yarn upgrade constructs jest jest-junit projen standard-version some-dep",
},
Expand All @@ -128,9 +158,15 @@ test("ncu upgrade command does not include dependencies with version constraints

test("ncu upgrade command is not added if no ncu upgrades are needed", () => {
const project = createProject({
deps: ["some-dep@^10"],
deps: ["some-dep@^10", "other-dep@10.0.0"],
depsUpgradeOptions: {
exclude: ["jest", "projen"],
exclude: [
"constructs",
"jest",
"jest-junit",
"projen",
"standard-version",
],
},
});

Expand All @@ -143,7 +179,7 @@ test("ncu upgrade command is not added if no ncu upgrades are needed", () => {
"exec": "yarn install --check-files",
},
{
"exec": "yarn upgrade constructs jest-junit standard-version some-dep",
"exec": "yarn upgrade other-dep some-dep",
},
{
"exec": "npx projen",
Expand Down Expand Up @@ -448,7 +484,7 @@ test("upgrade task created without projen defined versions at NodeProject", () =
expect(tasks.upgrade.steps).toMatchInlineSnapshot(`
[
{
"exec": "npx npm-check-updates@16 --upgrade --target=minor --peer --dep=dev,peer,prod,optional --filter=jest,projen,axios,markdownlint",
"exec": "npx npm-check-updates@16 --upgrade --target=minor --peer --dep=dev,peer,prod,optional --filter=jest,projen",
},
{
"exec": "yarn install --check-files",
Expand Down
4 changes: 2 additions & 2 deletions test/jsii/__snapshots__/jsii.test.ts.snap

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion test/typescript/typescript.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -259,7 +259,7 @@ test("upgrade task ignores pinned versions", () => {
expect(tasks.upgrade.steps).toMatchInlineSnapshot(`
[
{
"exec": "npx npm-check-updates@16 --upgrade --target=minor --peer --dep=dev,peer,prod,optional --filter=@types/jest,eslint-import-resolver-typescript,eslint-plugin-import,jest,projen,ts-jest,typescript",
"exec": "npx npm-check-updates@16 --upgrade --target=minor --peer --dep=dev,peer,prod,optional --filter=@types/jest,eslint-import-resolver-typescript,eslint-plugin-import,jest,projen,ts-jest",
},
{
"exec": "yarn install --check-files",
Expand Down

0 comments on commit ab74dab

Please sign in to comment.