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

Yarn - Fix incorrect impact graph #410

Merged
merged 7 commits into from
Sep 27, 2023
Merged

Conversation

Or-Geva
Copy link
Contributor

@Or-Geva Or-Geva commented Sep 21, 2023

  • All tests passed. If this feature is not already covered by the tests, I added new tests.
  • I used npm run format for formatting the code before submitting the pull request.

Prior to this PR, there was an issue with the way Yarn calculated the impact graph. The problem intruduced from using the 'yarn list' output. The 'yarn list' output did not accurately represent the project's dependencies as it removed duplicate leaves and flattened the tree structure.

In order to resolve this issue, this PR changes the following:

  1. Instead of constructing the entire dependency tree, we now compute only a partial tree. This partial tree specifically represents the paths from the project's root to the vulnerable dependencies only. This modification eliminates the need to calculate the entire tree, which not only reduces memory consumption but also aligns with our goal of displaying only paths to vulnerable dependencies.

  2. Rather than relying on yarn list, we have switched to using yarn why which provides the correct path to a specific dependency. This involves gathering all vulnerable dependencies and utilizing 'yarn why' to map out the path of each dependency from the root.

@Or-Geva Or-Geva requested a review from yahavi September 21, 2023 12:26
@Or-Geva Or-Geva added the safe to test Approve running integration tests on a pull request label Sep 21, 2023
@github-actions github-actions bot removed the safe to test Approve running integration tests on a pull request label Sep 21, 2023
@Or-Geva Or-Geva added bug Something isn't working safe to test Approve running integration tests on a pull request labels Sep 21, 2023
@github-actions github-actions bot removed the safe to test Approve running integration tests on a pull request label Sep 21, 2023
@Or-Geva Or-Geva added the safe to test Approve running integration tests on a pull request label Sep 21, 2023
@github-actions github-actions bot removed the safe to test Approve running integration tests on a pull request label Sep 21, 2023
src/main/treeDataProviders/utils/yarnImpactGraph.ts Outdated Show resolved Hide resolved
src/main/treeDataProviders/utils/yarnImpactGraph.ts Outdated Show resolved Hide resolved
src/main/treeDataProviders/utils/yarnImpactGraph.ts Outdated Show resolved Hide resolved
src/main/treeDataProviders/utils/yarnImpactGraph.ts Outdated Show resolved Hide resolved
src/test/tests/yarnImpactGraph.test.ts Outdated Show resolved Hide resolved
@Or-Geva Or-Geva added the safe to test Approve running integration tests on a pull request label Sep 27, 2023
@github-actions github-actions bot removed the safe to test Approve running integration tests on a pull request label Sep 27, 2023
@Or-Geva Or-Geva added the safe to test Approve running integration tests on a pull request label Sep 27, 2023
@github-actions github-actions bot removed the safe to test Approve running integration tests on a pull request label Sep 27, 2023
@Or-Geva Or-Geva added the safe to test Approve running integration tests on a pull request label Sep 27, 2023
@github-actions github-actions bot removed the safe to test Approve running integration tests on a pull request label Sep 27, 2023
@Or-Geva Or-Geva merged commit eabb0f1 into jfrog:master Sep 27, 2023
1 check failed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants