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

can't use internals for public API #1192

Open
jkowalleck opened this issue Dec 19, 2024 · 1 comment
Open

can't use internals for public API #1192

jkowalleck opened this issue Dec 19, 2024 · 1 comment
Labels
enhancement New feature or request help wanted Extra attention is needed

Comments

@jkowalleck
Copy link
Member

jkowalleck commented Dec 19, 2024

some properties and constructor parameters use internal types, like SortableStringables.

this is an issue, as these internals are not exported. they can not be used downstream.
It prevents usage like follows:

component.evidence = new CDX.Models.ComponentEvidence({
        licenses: new CDX.Models.LicenseRepository(this.getLicenseEvidence(dirname(pkg.path), logger)),
        copyright: new SortableStringables(copyrights)
      })

context

some symbols are mark as internals, they are not exported, and they are not public API.
see https://cyclonedx-javascript-library.readthedocs.io/en/latest/typedoc/node/modules/_internal_.html

but yet, some internals are used in public API, as properties or parameters.

requirements / goals

  • no internals are used in public API, as properties or parameters.
    They miught be used for inheritance, still.

solution

have dedicated classes, that are properly exported and public API.
for example

// pseudo-code 
export CDX.Models.CopyrightRepository extends SortableStringables{}

just like we did before:

export class AnalysisResponseRepository extends SortableStringables<AnalysisResponse> {}

considerations

since this would change types of public API, we would introduce it in a two-staged manner:

  1. non-breaking:
    • allo the new class -- since the new classes inherit from the old one, all is good
      ComponentEvidence(..., copyright: CopyrightRepository|SortableStringables)
    • dont change the typeof the property -- since the new classes inherit from the old one, all is good
      copyright: SortableStringables
  2. breaking: remove any usage of non-public classes in public API.
    prepare a breaking change, but dont mere it yet; have it added to the next major milestone.
    see must not use internals for public API #1193
@jkowalleck
Copy link
Member Author

jkowalleck commented Dec 19, 2024

@Frozen-byte this issue is caused by https://github.com/CycloneDX/cyclonedx-webpack-plugin/pull/1338/files#r1867685873

If you like, you can work on a non-breaking solution

@jkowalleck jkowalleck changed the title don't use internals for public API can't use internals for public API Dec 19, 2024
@jkowalleck jkowalleck added the help wanted Extra attention is needed label Dec 19, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request help wanted Extra attention is needed
Projects
None yet
Development

No branches or pull requests

1 participant