feat: add runtime engine constraints to metadata component - #589
Conversation
Signed-off-by: sid sri <sidsri1502@gmail.com>
Up to standards ✅🟢 Issues
|
| Metric | Results |
|---|---|
| Complexity | 7 |
| Duplication | 0 |
NEW Get contextual insights on your PRs based on Codacy's metrics, along with PR and Jira context, without leaving GitHub. Enable AI reviewer
TIP This summary will be updated as you push new changes.
There was a problem hiding this comment.
I have some implementation remarks that need changing.
i think this feature should be moved upstream.
see https://github.com/CycloneDX/cyclonedx-node-yarn/pull/589/changes#r3766004355
| type ManifestFetcher = (pkg: Package) => Promise<NonNullable<any>> | ||
| type LicenseEvidenceFetcher = (pkg: Package) => AsyncGenerator<License> | ||
|
|
||
| function addEngineConstraintProperties (component: Component, engines: unknown): void { |
There was a problem hiding this comment.
Implementation detail:
I'd rather have this a pure function
either a generator ala
function * makeEngineConstraintProperties ( engines: unknown): Generator<Property> { }or a
function makeEngineConstraintProperties ( engines: unknown): Iterable<Property> { }and then, they shall be used like
makeEngineConstraintProperties(...).forEach(p => { component.properties.add(p) })
| type LicenseEvidenceFetcher = (pkg: Package) => AsyncGenerator<License> | ||
|
|
||
| function addEngineConstraintProperties (component: Component, engines: unknown): void { | ||
| if (engines === null || typeof engines !== 'object' || Array.isArray(engines)) { |
There was a problem hiding this comment.
add the docs:
| if (engines === null || typeof engines !== 'object' || Array.isArray(engines)) { | |
| // https://docs.npmjs.com/cli/v11/configuring-npm/package-json#engines | |
| if (engines === null || typeof engines !== 'object' || Array.isArray(engines)) { |
| PackageDevelopment = 'cdx:npm:package:development', | ||
| } | ||
|
|
||
| export const enum PropertyNamePrefixes { |
There was a problem hiding this comment.
please add the docs. see how it is done with PropertyNames
| type ManifestFetcher = (pkg: Package) => Promise<NonNullable<any>> | ||
| type LicenseEvidenceFetcher = (pkg: Package) => AsyncGenerator<License> | ||
|
|
||
| function addEngineConstraintProperties (component: Component, engines: unknown): void { |
There was a problem hiding this comment.
please make this a private method of BomBuilder
| const rootComponent: Component = this.makeComponentFromWorkspace(workspace, this.metaComponentType) | ||
| ?? new DummyComponent(this.metaComponentType, 'RootComponent') | ||
| rootComponent.licenses.forEach(setLicensesDeclared) | ||
| addEngineConstraintProperties(rootComponent, workspace.manifest.raw.engines) |
There was a problem hiding this comment.
this works well for the root component, but what about the other components?
better move this into makeComponent.
and while doing so, you might see that entire process can be moved to
@cyclonedx/cyclonedx-library.Contrib.FromNodePackageJsonBuilders.Builders.ComponentBuilder
-- https://github.com/CycloneDX/cyclonedx-javascript-library/blob/main/src/contrib/fromNodePackageJson/builders.ts
PS: still considdering merging this feature
There was a problem hiding this comment.
PS: created CycloneDX/cyclonedx-javascript-library#1506
|
Thank you for all your efforts. looking into the implementation details, i think the entire feature shall be moved upstream. PS: still considdering merging this feature |
2 participants
Description
Map string values from the root workspace's
package.jsonenginesobject tometadata.component.propertiesusing the CycloneDX npm property taxonomy:cdx:npm:package:constraint:engine:<name> = <version range>This preserves arbitrary runtime names, ignores malformed or non-string values, and leaves SBOM output unchanged when
enginesis absent. JSON and XML snapshots cover both a multi-runtime manifest and an existing Node-only manifest.Resolves or fixes issue: #448
Validation
yarn test:nodeon Node.js 24: 61 passingyarn test:standardyarn test:lintyarn test:dependenciesAI Tool Disclosure
OpenAI CodexGPT-5Inspect the issue and contribution requirements; implement the engines-to-CycloneDX property mapping; update fixture snapshots; run focused and full tests; review the final diff.Affirmation