fix: replace auto-detected root component with user-provided one #1421

max619 · 2025-08-15T11:20:18Z

Previously, disabling root component auto-detection while specifying a root component left the auto-detected root in the BOM, with dependencies still attached to it (issue #1418).

This change ensures that all instances of the auto-detected root are replaced by the user-provided component using a componentSubstitutionMap during component generation. Also introduced a new RichComponentBuilder to centralize handling of component creation, BOM reference initialization, and PURL assignment, moving relevant logic out of Extractor.

Regression tests were added for that particular case

jkowalleck · 2025-08-15T12:27:47Z

so much refactoring for a simple fix? sorry, i don't intent to merge this out-of-scope modification.

please revert your changes and only fix the "bug" in the scope of #1418.

Thank you in advance.

PS: if you want to do design/architecture changes, then please propose them in a dedicated ticket where we can discuss the impact and expected outcome. An implementation of these changes may happen later :-)

max619 · 2025-08-15T13:34:01Z

@jkowalleck Simplified a bit

max619 · 2025-08-25T15:28:09Z

@jkowalleck looking forward for your review :)

jkowalleck · 2025-09-22T09:26:51Z

could you fix those merge conflicts?

Previously, disabling root component auto-detection while specifying a root component left the auto-detected root in the BOM, with dependencies still attached to it (issue CycloneDX#1418). This change ensures that all instances of the auto-detected root are replaced by the user-provided component using a `componentSubstitutionMap` during component generation. Regression tests were added for that particular case Signed-off-by: Maxim Bagryantsev <[email protected]>

jkowalleck · 2025-10-17T11:02:46Z

src/extractor.ts

   * @throws {@link Error} when no component could be fetched
   */
-  makeComponent (pkg: PackageDescription, collectEvidence: boolean, logger?: WebpackLogger): CDX.Models.Component {
+  #makeComponent (pkg: PackageDescription, collectEvidence: boolean, rootComponents: RootComponentCreationResult | undefined, logger?: WebpackLogger): CDX.Models.Component {


why was this function made private? this is an unexpected change.

jkowalleck · 2025-10-17T11:04:35Z

src/extractor.ts

    }

+    if (rootComponents?.detectedRootComponent !== undefined && doComponentsMatch(component, rootComponents.detectedRootComponent)) {
+      component = rootComponents.rootComponent


unintended design change.

a "makeSomething" function makes something - no matter what.
it does not cut steps

jkowalleck · 2025-10-17T11:08:42Z

src/_helpers.ts

  packageJson: NonNullable<any>
 }

+export interface RootComponentCreationResult {


unintended design.

the function that returns this RootComponentCreationResult is private. so the type definition for this very function should happen where this private function is defined.

jkowalleck · 2025-10-17T11:17:00Z

src/plugin.ts

    builder: CDX.Builders.FromNodePackageJson.ComponentBuilder,
    logger: WebpackLogger
-  ): CDX.Models.Component | undefined {
+  ): RootComponentCreationResult | undefined {


could you please explain what the idea behind the change of the algorithm of this function makeRootComponent is?

i mean, before, it had a name that pretty much described what it did.
now it does ... what?

Copilot

Pull Request Overview

This PR adds a regression test for issue #1418 to verify that rootComponentName correctly overrides the root component and all autodetected instances of it. The implementation tracks both the configured root component and the detected root component, then replaces any dependency components that match the detected root with the configured root component.

Reviewed Changes

Copilot reviewed 13 out of 14 changed files in this pull request and generated 3 comments.

Show a summary per file

File	Description
`tests/integration/setup.js`	Adds 'regression-issue1418' to the test setups list
`tests/integration/regression-issue1418/*`	New regression test setup with webpack config, source files, and package manifests
`tests/integration/index.test.js`	Adds test configuration for the new regression test
`tests/integration/__snapshots__/index.test.js.snap`	Updates snapshots with expected output and fixes for feature-issue1344-no-detect
`src/plugin.ts`	Updates to create and track both root and detected root components
`src/extractor.ts`	Updates component generation to replace detected root with configured root
`src/_helpers.ts`	Adds `RootComponentCreationResult` interface and `doComponentsMatch` helper function

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2025-10-29T14:46:30Z

src/plugin.ts

+    }
+
+    const rootComponent = builder.makeComponent(rootPackageJson)
+    if(rootComponent === undefined) { return undefined }


Missing space after 'if' keyword. Should be if (rootComponent === undefined) to maintain consistency with the project's coding style.

Suggested change

if(rootComponent === undefined) { return undefined }

if (rootComponent === undefined) { return undefined }

Copilot · 2025-10-29T14:46:31Z

src/plugin.ts

+    const rootComponents = this.#makeRootComponent(compilation.compiler.context, cdxComponentBuilder, logger.getChildLogger('RootComponentBuilder'))
+    bom.metadata.component = rootComponents?.rootComponent


[nitpick] The variable name rootComponents (plural) is misleading as it contains a single root component along with metadata about detection. Consider renaming to rootComponentResult or rootComponentInfo to better reflect that it's a result object rather than a collection.

Suggested change

const rootComponents = this.#makeRootComponent(compilation.compiler.context, cdxComponentBuilder, logger.getChildLogger('RootComponentBuilder'))

bom.metadata.component = rootComponents?.rootComponent

const rootComponentResult = this.#makeRootComponent(compilation.compiler.context, cdxComponentBuilder, logger.getChildLogger('RootComponentBuilder'))

bom.metadata.component = rootComponentResult?.rootComponent

Copilot · 2025-10-29T14:46:31Z

src/plugin.ts

+    if (detectedRootPackageJson !== rootPackageJson) {
+      normalizePackageManifest(
+        detectedRootPackageJson,
+        w => { logger.debug('normalizePackageJson from PkgPath', path, 'caused:', w) }
+      )
+    }


Missing null/undefined check for detectedRootPackageJson before calling normalizePackageManifest. If rootComponentAutodetect is false and detectedRootPackageJson is undefined, this will cause a runtime error when the condition is true.

max619 requested a review from a team as a code owner August 15, 2025 11:20

jkowalleck linked an issue Aug 15, 2025 that may be closed by this pull request

[BUG] rootComponentName does not override root component but adds a new one #1418

Open

2 tasks

max619 force-pushed the fix/1418 branch from 933a024 to 00ff7a2 Compare August 15, 2025 13:33

max619 force-pushed the fix/1418 branch from 00ff7a2 to 6a345e9 Compare October 2, 2025 10:16

Merge branch 'master' into fix/1418

beaec31

jkowalleck requested changes Oct 17, 2025

View reviewed changes

jkowalleck requested a review from Copilot October 29, 2025 14:44

Copilot AI reviewed Oct 29, 2025

View reviewed changes

	if(rootComponent === undefined) { return undefined }
	if (rootComponent === undefined) { return undefined }

		const rootComponents = this.#makeRootComponent(compilation.compiler.context, cdxComponentBuilder, logger.getChildLogger('RootComponentBuilder'))
		bom.metadata.component = rootComponents?.rootComponent

Uh oh!

Uh oh!

fix: replace auto-detected root component with user-provided one #1421

Are you sure you want to change the base?

fix: replace auto-detected root component with user-provided one #1421

Conversation

max619 commented Aug 15, 2025

Uh oh!

jkowalleck commented Aug 15, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

max619 commented Aug 15, 2025

Uh oh!

max619 commented Aug 25, 2025

Uh oh!

jkowalleck commented Sep 22, 2025

Uh oh!

jkowalleck Oct 17, 2025

Choose a reason for hiding this comment

Uh oh!

jkowalleck Oct 17, 2025

Choose a reason for hiding this comment

Uh oh!

jkowalleck Oct 17, 2025

Choose a reason for hiding this comment

Uh oh!

jkowalleck Oct 17, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull Request Overview

Reviewed Changes

Uh oh!

Copilot AI Oct 29, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Oct 29, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Oct 29, 2025

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

jkowalleck commented Aug 15, 2025 •

edited

Loading