feat(core): refactor lock file parsing and pruning #13864

meeroslav · 2022-12-16T10:20:24Z

Progress tracking:

Current Behavior

State of the existing implementation:

It’s heavily guided by final stringification, so the intermediate structure carries a lot of metadata from lock files
The structure has flaws that are not verified on the construction
The resulting lock file is sometimes inaccurate (leading to failed installation)
We have no way to test the validity of the parsed/pruned structure
The code/logic is very complex, hacky and hard to read

Expected Behavior

The goal of the rewriting:

Simplify the graph representation
Have a way to check the validity
Prune graph instead of metadata
Separate graph logic from package manager specifics

Implementation clarification

New logic uses a builder similar to ProjectGraphBuilder to construct a graph from packages and dependencies, which allows us to:
- Check the consistency (all nodes should have incoming nodes, each edge must have defined from and to node, etc.)
- Easily tree-shake and traverse graph when pruning
The graph contains minimal information from lock files:
- Dependency name
- Package name (if different from dependency name)
- Version (or URL for tarball packages)
Stringification is decoupled from the actual pruning process
- We prune the LockFileGraph and then use this as a base for lock file stringification
- Stringification uses content from the original lock file. This is trivial for Yarn and Pnpm but requires some gymnastics for Npm (find the package hierarchy and handle V1 and V3 mapping separately).
Unit tests use actual generated lock files as input params
- Unit tests are grouped around logic problems
  - Alias and tarball packages
  - Same package with same version installed twice in different locations
  - Real-world NextJS examples
  - Pruning single and multiple dependencies
  - Optional children
  - Workspaces
- Additional manually crafted will be added to test a few corner cases which are not that easy to generate
The resulting code has a lot of validations all over the place to ensure we detect broken things

Note, due to complexity and differences between PRs, the code is larger than 1000 loc (without warnings, etc.)
We can compress files more, but that would come at the expense of readability.

Related Issue(s)

Fixes #13863
Fixes #14096
Fixes #14275
Fixes #14721
Related to #9761
Related to #14106

vercel · 2022-12-16T10:20:29Z

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name	Status	Preview	Comments	Updated
nx-dev	✅ Ready (Inspect)	Visit Preview	💬 Add your feedback	Feb 6, 2023 at 0:36AM (UTC)

nx-cloud · 2022-12-16T10:22:52Z

☁️ Nx Cloud Report

CI is running/has finished running commands for commit cbc7b01. As they complete they will appear below. Click to see the status, the terminal output, and the build insights.

📂 See all runs for this branch

✅ Successfully ran 8 targets

Sent with 💌 from NxCloud.

…lock-files

FrozenPandaz · 2023-02-03T20:02:19Z

e2e/node/src/node.test.ts

+    const { combinedOutput: nestCombinedOutput } = await runCLIAsync(
+      `build ${nestapp} --generatePackageJson`
+    );
+    expect(nestCombinedOutput).not.toMatch(/Graph is not consistent/);


remove this

FrozenPandaz · 2023-02-03T20:06:35Z

packages/nx/package.json

@@ -33,6 +33,7 @@
  "homepage": "https://nx.dev",
  "dependencies": {
    "@parcel/watcher": "2.0.4",
+    "@pnpm/lockfile-types": "^4.3.6",


FrozenPandaz · 2023-02-06T15:31:07Z

packages/nx/src/lock-file/npm-parser.ts

+      }
+    });
+  } else {
+    Object.entries(data.dependencies).forEach(([packageName, snapshot]) => {


Consider moving this to a separate file.

FrozenPandaz · 2023-02-06T15:47:45Z

packages/nx/src/lock-file/npm-parser.ts

+
+  const output: NpmLockFile = {
+    name: packageJson.name || rootLockFile.name,
+    version: packageJson.version || '0.0.1',


TODO: If there is no version in packageJson, npm doesn't produce a version so we should probably do the same.

FrozenPandaz · 2023-02-06T15:51:37Z

packages/nx/src/lock-file/npm-parser.ts

+    const child = keyMap.get(searchPath);
+    if (
+      child.data.version === versionRange ||
+      satisfies(child.data.version, versionRange)


What is this for? It shouldn't matter if the version matches.

If the node is there, then it will be depended upon; Correct version or not.

github-actions · 2023-03-03T21:56:46Z

This pull request has already been merged/closed. If you experience issues related to these changes, please open a new issue referencing this pull request.

meeroslav self-assigned this Dec 16, 2022

meeroslav marked this pull request as draft December 16, 2022 10:20

meeroslav added type: bug scope: core core nx functionality labels Dec 16, 2022

vercel bot had a problem deploying to Preview December 16, 2022 10:21 Failure

meeroslav force-pushed the fix-lock-files branch from daa3cac to b36d029 Compare December 16, 2022 12:59

vercel bot had a problem deploying to Preview December 16, 2022 13:00 Failure

meeroslav force-pushed the fix-lock-files branch from b36d029 to b1ecc3a Compare December 16, 2022 15:20

vercel bot had a problem deploying to Preview December 16, 2022 15:26 Failure

meeroslav force-pushed the fix-lock-files branch 2 times, most recently from 1e8abab to 6b68cd7 Compare December 16, 2022 15:40

vercel bot deployed to Preview December 16, 2022 15:51 View deployment

meeroslav force-pushed the fix-lock-files branch from 6b68cd7 to e4a1427 Compare December 21, 2022 21:48

vercel bot deployed to Preview December 21, 2022 21:54 View deployment

meeroslav force-pushed the fix-lock-files branch from e4a1427 to a198fab Compare December 22, 2022 22:52

vercel bot deployed to Preview December 22, 2022 22:59 View deployment

vercel bot deployed to Preview January 3, 2023 22:17 View deployment

meeroslav force-pushed the fix-lock-files branch from 949dbcd to 92ce7ae Compare January 3, 2023 22:22

vercel bot deployed to Preview January 3, 2023 22:31 View deployment

meeroslav force-pushed the fix-lock-files branch 2 times, most recently from c842cea to 94db30c Compare January 3, 2023 22:38

vercel bot deployed to Preview January 3, 2023 22:45 View deployment

vercel bot deployed to Preview January 4, 2023 08:21 View deployment

vercel bot deployed to Preview January 4, 2023 13:09 View deployment

meeroslav force-pushed the fix-lock-files branch from b727299 to a069798 Compare January 4, 2023 13:14

vercel bot deployed to Preview January 4, 2023 13:21 View deployment

vercel bot deployed to Preview January 4, 2023 16:11 View deployment

vercel bot deployed to Preview January 5, 2023 12:03 View deployment

meeroslav added 2 commits February 1, 2023 14:39

chore(repo): Merge remote-tracking branch 'upstream/master' into fix-…

09e5103

…lock-files

fix(core): fix and tests for yarn registry parsing. fixes nrwl#14721

2cae54b

vercel bot deployed to Preview February 1, 2023 14:33 View deployment

meeroslav force-pushed the fix-lock-files branch 2 times, most recently from cf6d132 to 4a6026a Compare February 1, 2023 19:27

fix(core): fix missing hoisted dep pnpm

5533d83

meeroslav force-pushed the fix-lock-files branch from 4a6026a to 5533d83 Compare February 1, 2023 20:16

vercel bot deployed to Preview February 1, 2023 20:24 View deployment

chore(core): cleanup code

9e33f32

vercel bot deployed to Preview February 2, 2023 13:47 View deployment

fix(core): do not break if hoisted package cannot be found on yarn

218a5c6

meeroslav changed the title ~~fix(core): refactor lock file parsing and pruning~~ feat(core): refactor lock file parsing and pruning Feb 2, 2023

vercel bot deployed to Preview February 2, 2023 20:41 View deployment

FrozenPandaz and others added 2 commits February 3, 2023 15:06

chore(core): fixes

1caa4bb

fix(core): move types to specific package manager parsers

38c484a

vercel bot deployed to Preview February 3, 2023 21:39 View deployment

meeroslav added 2 commits February 3, 2023 22:46

cleanup(core): move logic from utils to pm parsers

4a18482

cleanup(core): simplify createLockFile

f3d31ae

vercel bot deployed to Preview February 3, 2023 21:57 View deployment

chore(repo): Merge remote-tracking branch 'upstream/master' into fix-…

cbc7b01

…lock-files

vercel bot deployed to Preview February 6, 2023 12:36 View deployment

FrozenPandaz approved these changes Feb 6, 2023

View reviewed changes

FrozenPandaz merged commit b802861 into nrwl:master Feb 6, 2023

This was referenced Feb 7, 2023

pruneDependencies fails for pnpm after upgrading Nx #14599

Closed

nx workspace-lint Broken on >15 #14591

Closed

Error on generated package.json with npm package at private registries #14825

Closed

veimox mentioned this pull request Feb 15, 2023

Segfault generating package.json #15019

Closed

github-actions bot locked as resolved and limited conversation to collaborators Mar 3, 2023

meeroslav deleted the fix-lock-files branch April 27, 2023 10:00

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(core): refactor lock file parsing and pruning #13864

feat(core): refactor lock file parsing and pruning #13864

meeroslav commented Dec 16, 2022 •

edited

Loading

vercel bot commented Dec 16, 2022 •

edited

Loading

nx-cloud bot commented Dec 16, 2022 •

edited

Loading

FrozenPandaz Feb 3, 2023

FrozenPandaz Feb 3, 2023

FrozenPandaz Feb 6, 2023

FrozenPandaz Feb 6, 2023

FrozenPandaz Feb 6, 2023

github-actions bot commented Mar 3, 2023

feat(core): refactor lock file parsing and pruning #13864

feat(core): refactor lock file parsing and pruning #13864

Conversation

meeroslav commented Dec 16, 2022 • edited Loading

Current Behavior

Expected Behavior

Implementation clarification

Related Issue(s)

vercel bot commented Dec 16, 2022 • edited Loading

nx-cloud bot commented Dec 16, 2022 • edited Loading

☁️ Nx Cloud Report

FrozenPandaz Feb 3, 2023

Choose a reason for hiding this comment

FrozenPandaz Feb 3, 2023

Choose a reason for hiding this comment

FrozenPandaz Feb 6, 2023

Choose a reason for hiding this comment

FrozenPandaz Feb 6, 2023

Choose a reason for hiding this comment

FrozenPandaz Feb 6, 2023

Choose a reason for hiding this comment

github-actions bot commented Mar 3, 2023

meeroslav commented Dec 16, 2022 •

edited

Loading

vercel bot commented Dec 16, 2022 •

edited

Loading

nx-cloud bot commented Dec 16, 2022 •

edited

Loading