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 props-bot label.

If you're merging code through a pull request on GitHub, copy and paste the following into the bottom of the merge commit message.

Co-authored-by: aduth <[email protected]>
Co-authored-by: t-hamano <[email protected]>
Co-authored-by: sirreal <[email protected]>
Co-authored-by: jsnajdr <[email protected]>

To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook.

Size Change: 0 B

Total Size: 2.42 MB

_{compressed-size-action}

I think I might add back the original logic as a fallback if the license isn't available from package-lock.json fields.

After recalling that I've seen pull requests and diffs erratically adding or removing the license field, I did some more digging and it seems like this has been buggy in the past, so it could be good to keep the current logic if for some reason the lockfile doesn't include the license.

• [BUG] "license" key in lockfile is never populated npm/cli#5532
• [BUG] License is missing for all dependencies in package-lock.json npm/cli#7384

I think I might add back the original logic as a fallback if the license isn't available from package-lock.json fields.

Added in 53413fd.

Those changes seem to reintroduce the issue, based on the failing build 😭

My guess to what's happening:

• --package-lock-only is causing optional dependencies like fsevents to be included when it wouldn't have been included previously, since it exists in package-lock.json and not in the file system
• The package-lock.json entry for fsevents in-fact does not include a license detail (source)

The combination of these two means that it will try to read the package.json from node_modules, but it doesn't exist.

I'm not feeling that --package-lock-only is particularly reliable.

This is tricky to solve, because when not using --package-lock-only, the dependency information doesn't seem to include the optional field. I managed to work around this in aed9cc4 by recursively building up a set of known optional dependencies based on optionalDependencies discovered through the dependency tree.

So what I've done is reverted back to something like I described in my original comment as an "earlier iteration" (f0dbfab), skipping optional dependencies if they're not supported for the current platform.

Hm, I'm actually not sure about this solution:

• Just because a package is defined as an optional dependency by one package doesn't mean that it should be considered optional for the entire tree
• I'm similarly confused why we even get to the point where we're checking licenses on optional dependencies to begin with, because the logic only recurses into dependencies, not optionalDependencies.

Flaky tests detected in ce24828.
Some tests passed with failed attempts. The failures may not be related to this commit but are still reported for visibility. See the documentation for more information.

🔍 Workflow run URL: https://github.com/WordPress/gutenberg/actions/runs/19269742226
📝 Reported issues:

• [Flaky Test] should select the wrapper div for a group #71211 in /test/e2e/specs/editor/various/block-hierarchy-navigation.spec.js

@jsnajdr and @sirreal might know of a good approach to this problem 👀

Are you familiar with npm query and have you tried it?

• https://docs.npmjs.com/cli/v11/using-npm/dependency-selectors#dependency-type-selectors
• https://docs.npmjs.com/cli/v11/commands/npm-query

Something like this might list the desired packages:

npm query '.workspace :not(.optional, .dev)'

Edit:

Check licenses uses it here:

The docs have also lead me to @npmcli/arborist which I believe powers the query command and is likely a powerful npm tree querying tool.

If we want to be future-proof, it's good to known that pnpm's pnpm-lock.yml file doesn't contain the license information at all, and pnpm list cannot list it either. We shouldn't rely on NPM features that will prevent a seamless switch in the future.

I had explored using npm query, even trying to consider using it to filter based on licenses directly. The challenges there were that we have a lot of fallback logic for pulling license information from package files. The other consideration is that there's a lot of variability with the details provided through different interfaces (npm ls, npm ls --package-lock-only, npm query). I can't recall why specifically I abandoned that approach, and it may have been to try to keep the changes surgical, but at this point it's worth revisiting given everything else has failed 😬

I gave npm query another try in 8b9bb6b, and I think a solution using this approach could work, with a couple caveats:

• I feel like it muddies the logic a bit where we're mostly filtering out packages in the binary script scripts/check-licenses.js, with utils/license.js existing only to sort out more the more complicated packages (e.g. multi-license, other edge cases)
• With that in mind, it might not actually solve the root problem if one of those edge case packages is installed in the way that produces the errors we've seen. We'd be relying on those packages to have simple licenses that get filtered out in the first pass

I think these changes work, but I'll plan to do some more testing on Monday. I'd also welcome feedback on the approach in the meantime.

Marking this ready for review. I think this approach using npm query could work and be more efficient at leveraging npm to do the work of filtering out packages with permissible licenses as a first pass.

The code changes look good, but I'm having trouble testing and getting it to fail. Is the procedure still to modify package-lock.json and run the script?

The code changes look good, but I'm having trouble testing and getting it to fail. Is the procedure still to modify package-lock.json and run the script?

Good call on the instructions. I'll take a closer look and update those, but my guess is that npm query is doing a "first-pass" at flagging any bad license in package-lock.json, but the fallback behavior in checkDeps will start looking at other license indicators in e.g. node_modules/@nx/nx/package.json that will have a valid license ("MIT"), so the testing procedure likely involves manual edits to a few different places (package-lock.json, package-specific package.json, and in my experience even README.md and LICENSE.md files because it looks for content there as well).

I updated the testing instructions in the original comment. sass-embedded-darwin-arm64 is a pretty good one to try because it only requires updating node_modules/sass-embedded-darwin-arm64/package.json in addition to package-lock.json (it doesn't include license details anywhere else in the package).

Verifying the code changes in bin/check-licenses.mjs is similar, but picking a (production) dependency of one of the packages that's shipped as a script.

I picked deepmerge (dependency of @wordpress/block-editor)

• Modify license value for node_modules/deepmerge in package-lock.json
• Modify license value in node_modules/deepmerge/package.json
• rm node_modules/deepmerge/license.txt node_modules/deepmerge/readme.md

node ./bin/check-licenses.mjs
 ERROR  Module deepmerge has an incompatible license 'MITbad'