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.

Sign up for GitHub

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

fix(release): properly handle deeply-transitive updates #30325

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

fix(release): properly handle deeply-transitive updates #30325

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
248 changes: 247 additions & 1 deletion packages/js/src/generators/release-version/release-version.spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1221,6 +1221,22 @@ Valid values are: "auto", "", "~", "^", "="`,
},
],
},
'project-with-double-transitive-dependency-on-my-lib': {
projectRoot:
'libs/project-with-double-transitive-dependency-on-my-lib',
packageName: 'project-with-double-transitive-dependency-on-my-lib',
version: '0.0.1',
packageJsonPath:
'libs/project-with-double-transitive-dependency-on-my-lib/package.json',
localDependencies: [
{
// Depends on my-lib via project-with-transitive-dependency-on-my-lib
projectName: 'project-with-transitive-dependency-on-my-lib',
dependencyCollection: 'devDependencies',
version: '^0.0.1',
},
],
},
});
});
afterEach(() => {
Expand Down Expand Up @@ -1256,6 +1272,20 @@ Valid values are: "auto", "", "~", "^", "="`,
"version": "0.0.1",
}
`);
expect(
readJson(
tree,
'libs/project-with-double-transitive-dependency-on-my-lib/package.json'
)
).toMatchInlineSnapshot(`
{
"devDependencies": {
"project-with-transitive-dependency-on-my-lib": "^0.0.1",
},
"name": "project-with-double-transitive-dependency-on-my-lib",
"version": "0.0.1",
}
`);

// It should not include transitive dependents in the versionData because we are filtering to only my-lib and updateDependents is set to "never"
expect(
Expand Down Expand Up @@ -1316,6 +1346,22 @@ Valid values are: "auto", "", "~", "^", "="`,
"version": "0.0.1",
}
`);

// The version of project-with-double-transitive-dependency-on-my-lib is untouched because it is not in the same batch as my-lib and updateDependents is set to "never"
expect(
readJson(
tree,
'libs/project-with-double-transitive-dependency-on-my-lib/package.json'
)
).toMatchInlineSnapshot(`
{
"devDependencies": {
"project-with-transitive-dependency-on-my-lib": "^0.0.1",
},
"name": "project-with-double-transitive-dependency-on-my-lib",
"version": "0.0.1",
}
`);
});

it('should always update transitive dependents when updateDependents is set to "auto"', async () => {
Expand Down Expand Up @@ -1347,6 +1393,20 @@ Valid values are: "auto", "", "~", "^", "="`,
"version": "0.0.1",
}
`);
expect(
readJson(
tree,
'libs/project-with-double-transitive-dependency-on-my-lib/package.json'
)
).toMatchInlineSnapshot(`
{
"devDependencies": {
"project-with-transitive-dependency-on-my-lib": "^0.0.1",
},
"name": "project-with-double-transitive-dependency-on-my-lib",
"version": "0.0.1",
}
`);

// It should include the appropriate versionData for transitive dependents
expect(
Expand Down Expand Up @@ -1389,11 +1449,24 @@ Valid values are: "auto", "", "~", "^", "="`,
],
"newVersion": "0.0.2",
},
"project-with-transitive-dependency-on-my-lib": {
"project-with-double-transitive-dependency-on-my-lib": {
"currentVersion": "0.0.1",
"dependentProjects": [],
"newVersion": "0.0.2",
},
"project-with-transitive-dependency-on-my-lib": {
"currentVersion": "0.0.1",
"dependentProjects": [
{
"dependencyCollection": "devDependencies",
"rawVersionSpec": "^0.0.1",
"source": "project-with-double-transitive-dependency-on-my-lib",
"target": "project-with-transitive-dependency-on-my-lib",
"type": "static",
},
],
"newVersion": "0.0.2",
},
},
}
`);
Expand Down Expand Up @@ -1433,6 +1506,179 @@ Valid values are: "auto", "", "~", "^", "="`,
"version": "0.0.2",
}
`);
expect(
readJson(
tree,
'libs/project-with-double-transitive-dependency-on-my-lib/package.json'
)
).toMatchInlineSnapshot(`
{
"devDependencies": {
"project-with-transitive-dependency-on-my-lib": "^0.0.2",
},
"name": "project-with-double-transitive-dependency-on-my-lib",
"version": "0.0.2",
}
`);
});

it('should not double-version transitive dependents when updateDependents is set to "auto"', async () => {
expect(readJson(tree, 'libs/my-lib/package.json').version).toEqual(
'0.0.1'
);
expect(
readJson(tree, 'libs/project-with-dependency-on-my-lib/package.json')
).toMatchInlineSnapshot(`
{
"dependencies": {
"my-lib": "~0.0.1",
},
"name": "project-with-dependency-on-my-lib",
"version": "0.0.1",
}
`);
expect(
readJson(
tree,
'libs/project-with-transitive-dependency-on-my-lib/package.json'
)
).toMatchInlineSnapshot(`
{
"devDependencies": {
"project-with-dependency-on-my-lib": "^0.0.1",
},
"name": "project-with-transitive-dependency-on-my-lib",
"version": "0.0.1",
}
`);
expect(
readJson(
tree,
'libs/project-with-double-transitive-dependency-on-my-lib/package.json'
)
).toMatchInlineSnapshot(`
{
"devDependencies": {
"project-with-transitive-dependency-on-my-lib": "^0.0.1",
},
"name": "project-with-double-transitive-dependency-on-my-lib",
"version": "0.0.1",
}
`);

// It should include the appropriate versionData for transitive dependents
expect(
await releaseVersionGenerator(tree, {
// version all projects
projects: Object.values(projectGraph.nodes),
projectGraph,
specifier: 'patch',
currentVersionResolver: 'disk',
specifierSource: 'prompt',
releaseGroup: createReleaseGroup('independent'),
updateDependents: 'auto',
})
).toMatchInlineSnapshot(`
{
"callback": [Function],
"data": {
"my-lib": {
"currentVersion": "0.0.1",
"dependentProjects": [
{
"dependencyCollection": "dependencies",
"rawVersionSpec": "~0.0.1",
"source": "project-with-dependency-on-my-lib",
"target": "my-lib",
"type": "static",
},
],
"newVersion": "0.0.2",
},
"project-with-dependency-on-my-lib": {
"currentVersion": "0.0.1",
"dependentProjects": [
{
"dependencyCollection": "devDependencies",
"rawVersionSpec": "^0.0.1",
"source": "project-with-transitive-dependency-on-my-lib",
"target": "project-with-dependency-on-my-lib",
"type": "static",
},
],
"newVersion": "0.0.2",
},
"project-with-double-transitive-dependency-on-my-lib": {
"currentVersion": "0.0.1",
"dependentProjects": [],
"newVersion": "0.0.2",
},
"project-with-transitive-dependency-on-my-lib": {
"currentVersion": "0.0.1",
"dependentProjects": [
{
"dependencyCollection": "devDependencies",
"rawVersionSpec": "^0.0.1",
"source": "project-with-double-transitive-dependency-on-my-lib",
"target": "project-with-transitive-dependency-on-my-lib",
"type": "static",
},
],
"newVersion": "0.0.2",
},
},
}
`);

expect(readJson(tree, 'libs/my-lib/package.json')).toMatchInlineSnapshot(`
{
"name": "my-lib",
"version": "0.0.2",
}
`);

// The version of project-with-dependency-on-my-lib gets bumped by a patch number and the dependencies reference is updated to the new version of my-lib
expect(
readJson(tree, 'libs/project-with-dependency-on-my-lib/package.json')
).toMatchInlineSnapshot(`
{
"dependencies": {
"my-lib": "~0.0.2",
},
"name": "project-with-dependency-on-my-lib",
"version": "0.0.2",
}
`);

// The version of project-with-transitive-dependency-on-my-lib gets bumped by a patch number and the devDependencies reference is updated to the new version of project-with-dependency-on-my-lib because of the transitive dependency on my-lib
expect(
readJson(
tree,
'libs/project-with-transitive-dependency-on-my-lib/package.json'
)
).toMatchInlineSnapshot(`
{
"devDependencies": {
"project-with-dependency-on-my-lib": "^0.0.2",
},
"name": "project-with-transitive-dependency-on-my-lib",
"version": "0.0.2",
}
`);
expect(
readJson(
tree,
'libs/project-with-double-transitive-dependency-on-my-lib/package.json'
)
).toMatchInlineSnapshot(`
{
"devDependencies": {
"project-with-transitive-dependency-on-my-lib": "^0.0.2",
},
"name": "project-with-double-transitive-dependency-on-my-lib",
"version": "0.0.2",
}
`);
});
});

Expand Down
64 changes: 23 additions & 41 deletions packages/js/src/generators/release-version/release-version.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -627,13 +627,19 @@ To fix this you will either need to add a package.json file at that location, or
options.releaseGroup.projectsRelationship === 'independent';
const transitiveLocalPackageDependents: LocalPackageDependency[] = [];
if (includeTransitiveDependents) {
for (const directDependent of allDependentProjects) {
// Look through localPackageDependencies to find any which have a target on the current dependent
const possibleDependents = [...allDependentProjects];
while (possibleDependents.length > 0) {
const dependent = possibleDependents.pop();

for (const localPackageDependency of Object.values(
localPackageDependencies
).flat()) {
if (localPackageDependency.target === directDependent.source) {
if (
localPackageDependency.target === dependent.source &&
!transitiveLocalPackageDependents.includes(localPackageDependency)
) {
transitiveLocalPackageDependents.push(localPackageDependency);
possibleDependents.push(localPackageDependency);
}
}
}
Expand Down Expand Up @@ -926,40 +932,13 @@ To fix this you will either need to add a package.json file at that location, or
const isAlreadyDirectDependent = allDependentProjects.some(
(dep) => dep.source === transitiveDependentProject.source
);
if (isAlreadyDirectDependent) {
// Don't continue directly in this scenario - we still need to update the dependency version
// but we don't want to bump the project's own version as it will end up being double patched
const dependencyProjectName = transitiveDependentProject.target;
const dependencyPackageRoot = projectNameToPackageRootMap.get(
dependencyProjectName
);
if (!dependencyPackageRoot) {
throw new Error(
`The project "${dependencyProjectName}" does not have a packageRoot available. Please report this issue on https://github.com/nrwl/nx`
);
}
const dependencyPackageJsonPath = joinPathFragments(
dependencyPackageRoot,
'package.json'
);
const dependencyPackageJson = readJson(
tree,
dependencyPackageJsonPath
);

updateDependentProjectAndAddToVersionData({
dependentProject: transitiveDependentProject,
dependencyPackageName: dependencyPackageJson.name,
newDependencyVersion: dependencyPackageJson.version,
forceVersionBump: false, // Never bump version for direct dependents
});
continue;
}

// Check if the transitive dependent originates from a circular dependency
const isFromCircularDependency = circularDependencies.has(
`${transitiveDependentProject.source}:${transitiveDependentProject.target}`
);
const isInCurrentBatch = options.projects.some(
(project) => project.name === transitiveDependentProject.source
);
const dependencyProjectName = transitiveDependentProject.target;
const dependencyPackageRoot = projectNameToPackageRootMap.get(
dependencyProjectName
Expand All @@ -979,14 +958,17 @@ To fix this you will either need to add a package.json file at that location, or
dependentProject: transitiveDependentProject,
dependencyPackageName: dependencyPackageJson.name,
newDependencyVersion: dependencyPackageJson.version,
/**
* For these additional dependents, we need to update their package.json version as well because we know they will not come later in the topologically sorted projects loop.
* The one exception being if the dependent is part of a circular dependency, in which case we don't want to force a version bump as this would come in addition to the one
* already applied.
*/
forceVersionBump: isFromCircularDependency
? false
: updateDependentsBump,
// For these additional transitive dependents we will need to force a version bump unless
// we know they will be updated in the current batch, either because they are a direct
// dependent, part of the current batch of projects, or are part of a circular
// dependency. In either of those cases, we don't want to force a version bump as it
// would double bump the version.
forceVersionBump:
isAlreadyDirectDependent ||
isInCurrentBatch ||
isFromCircularDependency
? false
: updateDependentsBump,
});
}

Expand Down