Scripts: Update license checker to ignore invalid package entries #73528
+5
−1
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
aduth
requested review from
ajitbohra,
gziolo,
nerrad,
ntwb and
ryanwelcher
as code owners
|
The following accounts have interacted with this PR and/or linked issues. I will continue to update these lists as activity occurs. You can also manually ask me to refresh this list by adding the If you're merging code through a pull request on GitHub, copy and paste the following into the bottom of the merge commit message. To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook. |
aduth
added
[Tool] WP Scripts
|
Size Change: 0 B Total Size: 2.54 MB ℹ️ View Unchanged
|
t-hamano
approved these changes
Nov 25, 2025
Contributor
t-hamano
left a comment
t-hamano
left a comment
There was a problem hiding this comment.
Thanks for the PR! I think this approach works correctly.
I may not be understanding it correctly, but I wonder if a further improvement would be possible, where if a library does not have a package.json, the license is referenced from the root package-lock.json. For example, we can see that @unrs/resolver-binding-wasm32-wasi, which is causing the problem this time, is under the MIT license.
Member
Author
Yeah, I think that could be a nice improvement. It's a can of worms honestly, since the current behavior surprises me, as I was hoping I might do a timeboxed exploration of whether we could do a "fallback pass" using |
Contributor
|
For now, I think it's okay to merge this PR and try to improve it in a follow-up PR 👍 |
Member
Author
|
That sounds good @t-hamano 👍 For posterity, I did a trial run of a "two-pass" approach using the lockfile information as a fallback. And while it works, I think it ends up being a bit clunky, and I don't love the idea of continuing to build out the CLI frontend at the expense of the |
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.
What?
Relates to #73026
Updates license checker script to avoid checking packages with incomplete installations.
Why?
This was intended to have been fixed with #73026, but the issue continues to persist infrequently on
trunk(see recent example).How?
Corrupt packages will be reported in the results of
npm query, but most of their values are absent. A valid package should include its name in the reported result. The changes here update the logic to expect a valid name before checking the package.Example of a "corrupt" package:
Compare to a valid package:
(The above were identified by adding a
console.logdebugging statement to printpackages)I also considered whether some other query pseudo selectors could help here (e.g. "invalid" or "extraneous"), but those may result in more false negatives than the approach considered here, as invalid or extraneous packages can have legitimate issues.
Another option could be to filter the
devproperty, as this particular check is considering "dev"-only packages, and the corrupt package results indev: false. However, I suspect this defaults tofalse, and that this problem could potentially surface for non-development dependencies as well.Testing Instructions
I was able to reproduce the issue on
trunkby creating an empty folder for one of our optional dependencies not installed on my machine:Running
npm exec wp-scripts check-licenses -- --devpreviously resulted in an error.On this branch, the check should succeed.