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

feat(core): convert commands to use nx project graph instead of package graph #3667

Merged
merged 54 commits into from
May 8, 2023

Conversation

fahslaj
Copy link
Contributor

@fahslaj fahslaj commented Apr 28, 2023

No description provided.

fahslaj and others added 30 commits March 31, 2023 12:37
chore: move legacy package graph functions


chore: extract project operations into individual files


chore(core): move legacy features to @lerna/legacy-core


chore: bump nx version to 15.8.8


chore: add lodash types


chore: use resetDaemonClient argument when creating project graph

it("lists packages under explicitly configured node_modules directories", async () => {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This test and the one below represent functionality that is no longer supported, which is the explicit configuration of including node_modules within the package config globs.

Nx does not support projects within the node_modules directory.

@@ -220,28 +221,6 @@ Map {
]);
});

it("produces a topological ordering that _excludes_ devDependencies when value is 'dependencies' (DEPRECATED)", async () => {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is removed functionality.

The publish command will always function as if this option is set to "--all", as that is what topological traversing of the Nx project graph will get us by default.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This file has the test cases from legacy-core/src/lib/collect-updates/index.spec.ts ported to work with the new projects-based api.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This function combines the functionality of the old getFilteredPackages and filterPackages helpers into one function, adapting it to the new projects-based api.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Test and function were moved up a directory.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This replaces listable-format.ts

@@ -252,6 +258,30 @@ export class Package {
return writePkg(this.manifestLocation, this[PKG] as any).then(() => this);
}

getLocalDependency(
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Lerna does not recognize peerDependencies

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This, along with toposort-projects, replace the QueryGraph & PackageGraph structure for the purposes of topological operations.

There were not unit tests for this functionality beforehand. The test cases added are based on observed behavior of the previous version of Lerna, with the exception of the overlapping cycles case. In this case, the output is slightly different than the previous Lerna, but it is still a valid topological ordering.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Test cases created by observing Lerna's existing behavior.

// reset lerna.json and package-lock.json after publish of a local version
if (options.local) {
execSync("git checkout -- lerna.json package-lock.json");
}
} else {
// JH: remove unneeded await
version(versionOptions);
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This --dryRun case will still need to be reviewed. I suspect that it will still create a git commit for the version operation.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This and get-unpublished-packages below are no longer exported from Lerna. If absolutely needed, we can add back and export the old version of the function that uses the package graph.

@@ -146,28 +145,6 @@ lerna publish from-package --git-head ${CODEBUILD_RESOLVED_SOURCE_VERSION}

Under all other circumstances, this value is derived from a local `git` command.

### `--graph-type <all|dependencies>`
Copy link
Contributor Author

@fahslaj fahslaj May 3, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

--graph-type is no longer supported. This was called out as deprecated in the code, giving a warning when used that it would be removed in Lerna v6.

The Nx project graph doesn't distinguish between dependencies or dev dependencies, so it naturally doesn't implement this flag's behavior. I figured since it was deprecated, it should be removed instead of implemented over the Nx project graph.

@fahslaj fahslaj marked this pull request as ready for review May 3, 2023 20:17
@fahslaj fahslaj requested a review from JamesHenry May 3, 2023 20:17
@JamesHenry JamesHenry merged commit 8e813c4 into lerna:v7 May 8, 2023
11 checks passed
@JamesHenry JamesHenry deleted the feature/nx-project-graph branch May 8, 2023 15:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants