fix: handle licenses property used in old packages
#11
Conversation
| ], | ||
| }) | ||
| ) | ||
| ).toBe("(MIT OR ISC)"); |
There was a problem hiding this comment.
Could you add a test using an actual package.json file to the basic-cases directory under tests/integrations? Since license information is retrieved via npm query, we need to prepare a test that uses it.
| if (Array.isArray(dep.licenses)) { | ||
| if (dep.licenses.length > 1) { | ||
| const joined = dep.licenses.map((l) => l.type).join(" OR "); | ||
| return `(${joined})`; | ||
| } | ||
| return dep.licenses.at(0)?.type ?? ""; | ||
| } |
There was a problem hiding this comment.
If there are elements without a type, could you treat the entire retrieval as a failure and return ""?
The licenses specification is vague, and there's no guarantee that type will always be present.
https://docs.npmjs.com/cli/v10/configuring-npm/package-json#license
If none of the elements have a type, a string like " OR OR OR " might be generated. However, if elements without type are excluded, there's a risk that a string like "MIR OR ISC" could be generated when "MIT OR ISC OR Apache" is expected.
It might be safer to make the function work only when all elements in the array have a type.
Why
Some old packages used invalid styles for license notation. (ref)
licensepropertylicensespropertyThe license-manager has already supported license objects in
licensebut not array inlicenses.Some popular packages depend on such packages with invalid license notation, so I want to handle them correctly.
e.g. npm-run-all depends on memorystream or undici depends on busboy
I also checked the pnpm behavior, and it handles the
licensesproperty. (ref)What
This PR adds a handler for the
licensesproperty, as pnpm does.licensespropertytypeof the first element iflicensescontains only an elementlicensescontains multiple elements