Skip to content

Validation: Add a published-dependency audit script#79094

Merged
manzoorwanijk merged 3 commits into
trunkfrom
add/dependency-audit-check
Jun 19, 2026
Merged

Validation: Add a published-dependency audit script#79094
manzoorwanijk merged 3 commits into
trunkfrom
add/dependency-audit-check

Conversation

@manzoorwanijk

@manzoorwanijk manzoorwanijk commented Jun 10, 2026

Copy link
Copy Markdown
Member

What?

Follow up to #78882 (comment), where integrating a check against undeclared published dependencies was suggested.

Adds @mawesome/dependency-audit to tools/validation and exposes it as npm run lint:published-deps. The audit verifies that every bare import reachable in each package's published artifact — runtime JS and emitted .d.ts — is declared and resolvable from that package's own dependency ranges, never via root hoisting, a devDependency, or a workspace link.

This PR adds a script and its ignore config so the check can be run locally and evaluated; it also adds a CI gate. A companion PR (#79095) fixes everything the audit currently reports — until it lands, this PR doubles as a live demonstration of what the tool catches.

Why?

Missing published dependencies have been a recurring, hard-to-catch bug class (#74310, #74655, #78882): a module resolves at build time in the monorepo, but consumers installing the package from npm cannot resolve it. For type-surface gaps the failure is silent — with skipLibCheck: true the affected exports degrade to any. We had no protection against regressions, and as #78882 showed, new gaps keep being introduced.

On current trunk, npm run lint:published-deps reports 21 real findings across 12 packages (all fixed by #79095), for example:

@wordpress/components@34.0.0
  ✗ types      [undeclared]     @ariakit/react-core/utils/types  (build-types/card/card-divider/hook.d.ts)
@wordpress/react-i18n@4.47.0
  ✗ runtime    [undeclared]     react/jsx-runtime  (build/index.cjs)
@wordpress/editor@14.47.0
  ✗ types      [undeclared]     redux  (build-types/store/index.d.ts)
  ✗ types      [undeclared]     rememo  (build-types/store/selectors.d.ts)
@wordpress/interactivity@6.47.0
  ✗ types      [undeclared]     @preact/signals-core  (build-types/scopes.d.ts)

With #79095 merged (and this branch rebased), the audit runs clean and deterministically: 126 packages, 0 findings, 1 collapsed, 12 ignored, 8 notices (exit 0).

How?

  • @mawesome/dependency-audit is added as a devDependency of the private @wordpress/validation-tools workspace (keeping it out of the root, per workspace conventions), with a dependency-audit script that audits packages/*.
  • A root lint:published-deps script delegates to it, mirroring lint:lockfile/lint:tsconfig.
  • tools/validation/dependency-audit.config.json documents every intentional suppression with the reason and the follow-up needed. The remaining ignores are: test-fixture imports in e2e-tests (scoped to that package's plugins/** path), the optional puppeteer-core/install / puppeteer-firefox imports in scripts, two libraries that ship no types and have no @types/* package (equivalent-key-map, moment-timezone's deep import), two ESM/CJS packaging mismatches to fix separately (edit-site-initboot, interactivity-routerinteractivity), and one temporary rule for @tannin/sprintf pending a tool release that resolves ESM-only types.
  • --collapse-root-cause makes findings whose root cause is another audited package's packaging notice (currently: editor cannot resolve @wordpress/block-editor's types because block-editor does not expose them) report against the producer instead of failing every consumer.

Exit codes: 0 clean, 1 findings, 2 a package could not be audited — so the script is CI-ready once we decide to gate (intended follow-up, possibly after evaluating it on a few real PRs).

Testing Instructions

  1. npm ci
  2. npm run build (the audit reads the built artifacts)
  3. npm run lint:published-deps — on this branch (current trunk) expect exit 1 with 21 findings, each carrying a remediation suggestion; after Packages: Fix the published dependency surface of npm packages #79095 merges and this branch rebases, expect exit 0 with 126 packages, 0 findings, 1 collapsed, 12 ignored, 8 notices, 3 skipped.
  4. To see it catch a fresh regression once green: remove e.g. "redux" from packages/editor/package.json dependencies, run npm install && npm run lint:published-deps, and observe the undeclared finding reappear.

Testing Instructions for Keyboard

N/A — tooling only, no UI.

Screenshots or screencast

N/A.

Use of AI Tools

Used Claude Code to wire up the tool, run the audits, and author the ignore config. The diff was reviewed before submission.

@manzoorwanijk

manzoorwanijk commented Jun 10, 2026

Copy link
Copy Markdown
Member Author

@aduth this is blocked until we have Node 24 via #72973. Meanwhile, I have done the package fixes in #79095

@aduth

aduth commented Jun 10, 2026

Copy link
Copy Markdown
Member

@manzoorwanijk If it were distributed as a GitHub Action, would that maybe be an option to isolate it enough to avoid that dependency on Node 24 ?

Or I guess even within this project, if we'd have an appetite to have some CI checks run on Node 24+ only. That means people may not be able to reliably run it locally, but this feels like a pretty rare and clear-enough output to signal from CI alone.

@manzoorwanijk

Copy link
Copy Markdown
Member Author

I am instead widening the floor to add Node v20 support in the tool - manzoorwanijk/mawesome#48

I will remove the Node v20 support once Gutenberg migrates.

@manzoorwanijk manzoorwanijk force-pushed the add/dependency-audit-check branch 2 times, most recently from 9c85307 to a6d2bff Compare June 10, 2026 17:22
@github-actions

github-actions Bot commented Jun 10, 2026

Copy link
Copy Markdown

Size Change: 0 B

Total Size: 8.05 MB

compressed-size-action

@manzoorwanijk

Copy link
Copy Markdown
Member Author

Here it is - working

https://github.com/WordPress/gutenberg/actions/runs/27293481108/job/80620011399?pr=79094

@github-actions

github-actions Bot commented Jun 10, 2026

Copy link
Copy Markdown

Flaky tests detected in f58fee3.
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/27819792931
📝 Reported issues:

@manzoorwanijk manzoorwanijk force-pushed the add/dependency-audit-check branch from a6d2bff to 14f482e Compare June 15, 2026 17:36
@github-actions github-actions Bot added the [Package] Editor /packages/editor label Jun 15, 2026
@manzoorwanijk manzoorwanijk force-pushed the add/dependency-audit-check branch 3 times, most recently from 09d9195 to 92d5691 Compare June 15, 2026 19:29
@manzoorwanijk manzoorwanijk requested a review from aduth June 15, 2026 19:42
@manzoorwanijk manzoorwanijk force-pushed the add/dependency-audit-check branch from 92d5691 to 4aa2bca Compare June 15, 2026 19:42
@manzoorwanijk manzoorwanijk added the [Type] Code Quality Issues or PRs that relate to code quality label Jun 15, 2026
@manzoorwanijk manzoorwanijk self-assigned this Jun 15, 2026
@manzoorwanijk manzoorwanijk marked this pull request as ready for review June 15, 2026 19:42
@manzoorwanijk manzoorwanijk requested a review from desrosj as a code owner June 15, 2026 19:42
@github-actions

Copy link
Copy Markdown

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: manzoorwanijk <manzoorwanijk@git.wordpress.org>
Co-authored-by: aduth <aduth@git.wordpress.org>

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

@manzoorwanijk manzoorwanijk force-pushed the add/dependency-audit-check branch from 4aa2bca to 3bb460e Compare June 15, 2026 20:09

@aduth aduth left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think Windows compatibility should be confirmed, but otherwise this looks like it's in good shape 👍

Comment on lines +3 to +6
{
"comment": "equivalent-key-map ships no type declarations and has no @types/* package. Referenced only in @wordpress/data's emitted .d.ts.",
"package": "equivalent-key-map"
},

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I suppose I should get on that 😅

Comment thread packages/editor/src/components/post-text-editor/utils.js
Comment thread tools/validation/package.json Outdated
@manzoorwanijk

manzoorwanijk commented Jun 17, 2026

Copy link
Copy Markdown
Member Author

@aduth while working on this PR, I noticed that if I do npm_config_min_release_age=0 npm install and commit the lockfile, then npm doesn't check for minimum release age for packages already in lockfile 🤷, and we have no check on CI to validate the lockfile. Though we have validate-package-lock.cjs, but it probably needs some improvement. The best way to do is git checkout trunk package-lock.json && npm install, which should fail due to minimum release age and we can also do a diff of the lockfile to ensure it's not in an inconsistent state.

There is also check-local-changes.cjs, which does some of it but not all.

@aduth

aduth commented Jun 17, 2026

Copy link
Copy Markdown
Member

@aduth while working on this PR, I noticed that if I do npm_config_min_release_age=0 npm install and commit the lockfile, then npm doesn't check for minimum release age for packages already in lockfile 🤷

Oh wow, so npm doesn't validate release age if the package update is already in the lockfile? That's rather concerning, since I've been trusting the release age behavior when running npm install in other peoples' branches 😬 I agree this would be good to try to preemptively detect in CI.

@manzoorwanijk

Copy link
Copy Markdown
Member Author

Probably, the npm theory is that a poisoned dependency (inside the release age) shouldn't end up in lockfile because npm install <dep> should block it, but in our case, CI and may be many devs still use npm v10, which doesn't enforce it. So, we are all vulnerable to the attacks 😄

@manzoorwanijk manzoorwanijk force-pushed the add/dependency-audit-check branch from 4a7da8a to 6185da5 Compare June 18, 2026 11:37
manzoorwanijk and others added 3 commits June 19, 2026 15:46
Wire @mawesome/dependency-audit into tools/validation and expose it as
`npm run lint:published-deps`. The audit verifies that every bare import
reachable in each package's published artifact (runtime JS and emitted
.d.ts) is declared and resolvable from the package's own dependencies,
catching modules that resolve in the monorepo only via hoisting, a
devDependency, or a workspace link.

The ignore config documents the known-intentional findings (test
fixtures, optional imports) and the remaining deferrals, each with the
reason and the follow-up needed. No CI gate yet; this only adds the
script so it can be run locally and evaluated. On current trunk the
audit reports the 21 real findings fixed by the companion PR.
Co-authored-by: Andrew Duthie <1779930+aduth@users.noreply.github.com>

@aduth aduth left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM 👍 It's kinda slow (1m35s), and in reviewing the internal implementation I can see why with how it's TypeScript-analyzing the packed package in an isolated context, though I can also understand why it has to be that way vs. something more naive like just looking at import sources in the published files for things like the @types/react case you're trying to explicitly catch. So I think it's a worthwhile tradeoff.

"scripts": {
"check-installed-deps": "node ./check-installed-deps.mjs",
"check-licenses": "node ./check-licenses.mjs",
"dependency-audit": "dependency-audit --collapse-root-cause \"../../packages/*\"",

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You'd know better than I if this will work as intended, but flagging that you document --collapse-root-cause as applying for a "a multi-target run", and it's not clear to me if the changes from glob expansion to string glob (which in effect reduces arguments from multiple to one) will produce the result we expect.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The global here results in a multi target run - i.e. multiple packages audited together.

Comment thread package-lock.json
"resolve.exports": "^2.0.3",
"tar": "^7.5.16",
"tinyglobby": "^0.2.17",
"typescript": "^6.0.3"

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should this have been a peer dependency? Especially with TypeScript 7.0 being imminent, I wouldn't want us to have to be pulling multiple major versions of TypeScript if we can avoid it, and this is a pretty heavy dependency to put on downstream consumers (no dependencies, but 24MB installed).

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No, it’s needed as a runtime dependency because the package uses the programmatic API from TypeScript which tsgo doesn’t ship yet. It will be available from TS 7.1.

Comment on lines +97 to +100
- name: Audit published dependencies
if: ${{ matrix.annotations }}
run: npm run lint:published-deps -- --fail-unused-ignores

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Your PR message says "This PR only adds the script and its ignore config so the check can be run locally and evaluated; it does not add a CI gate yet", but this is a CI gate, isn't it? Is that comment outdated?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, it’s stale.

@manzoorwanijk manzoorwanijk merged commit 5ccc16d into trunk Jun 19, 2026
45 of 49 checks passed
@manzoorwanijk manzoorwanijk deleted the add/dependency-audit-check branch June 19, 2026 18:18
@github-actions github-actions Bot added this to the Gutenberg 23.5 milestone Jun 19, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

[Package] Editor /packages/editor [Type] Code Quality Issues or PRs that relate to code quality

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants