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

fix: traversal limit ignoring default value #66

Closed
wants to merge 1 commit into from

Conversation

xenobytezero
Copy link
Contributor

@xenobytezero xenobytezero commented Apr 25, 2024

Tasks

  • I've read the CONTRIBUTING guide
  • Necessary tests have been added

The moduleParsed hook calls out to getCorrespondingPackageFromModuleId for each imported module, which tries to find a package.json to extract information from. If it can't find a package.json it should traverse up the directory tree a max of 10 times before failing.

The current implementation passes getCorrespondingPackageFromModuleId directly to the nodeModuleImportedIds.map() call. This means that the traverseLimit param will not be undefined and use the default of 10, and instead will be the current index in the array (the second param of .map()). We found this meant a number of packages were getting missed in our output SBOM.

This pull request uses an arrow function to call getCorrespondingPackageFromModuleId, which will correctly use the default value of 10.

@xenobytezero xenobytezero requested a review from janbiasi as a code owner April 25, 2024 12:34
@janbiasi
Copy link
Owner

janbiasi commented May 1, 2024

@xenobytezero you've also fixed this in your other PR #68 – Am I allowed to close this PR in favor of your newer one?

@xenobytezero
Copy link
Contributor Author

@xenobytezero you've also fixed this in your other PR #68 – Am I allowed to close this PR in favor of your newer one?

If #68 is going ahead, absolutely.

@xenobytezero xenobytezero deleted the fix-traversal-limit branch December 13, 2024 13:01
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.

2 participants