Fix technique filtering logic by correctly checking for intersection #1848
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
f8b63d7 to
02871ea
Compare
…re found Signed-off-by: Yuval Michaeli <yuvalmich2@gmail.com>
02871ea to
ffd4458
Compare
prabhu
approved these changes
Jun 8, 2025
prabhu
reviewed
Jun 8, 2025
| @@ -304,7 +304,7 @@ export function filterBom(bomJson, options) { | |||
| // Set.intersection is only available in node >= 22. See Bug# 1651 | |||
| if ( | |||
| usedTechniques && | |||
| !new Set([...usedTechniques].filter((i) => allowedTechniques.has(i))) | |||
| ![...usedTechniques].some((i) => allowedTechniques.has(i)) | |||
There was a problem hiding this comment.
Have you tried adding .size? That must work too.
There was a problem hiding this comment.
Yes it also works.
However, using .some() is a bit more efficient than converting to a Set and checking .size because it stops at the first match and avoids creating intermediate data structures.
prabhu
approved these changes
Jun 8, 2025
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Bug Summary
This PR fixes a logic bug in the technique filtering block where the code attempted to check for an intersection between two sets (usedTechniques and allowedTechniques) using:
However, this condition is flawed because new Set(...) always returns a truthy object, even if the set is empty. As a result, the condition never evaluated to true, and the intended filtering logic was never triggered.
Fix
The condition has been updated to use
.some()without set usage:This correctly checks whether any technique in usedTechniques is present in allowedTechniques, and filters out the component if there is no match.
Linked Issue
#1849