Tools: Lint dependency version consistency with Syncpack#77950
Open
manzoorwanijk wants to merge 1 commit into
Open
Tools: Lint dependency version consistency with Syncpack#77950manzoorwanijk wants to merge 1 commit into
manzoorwanijk wants to merge 1 commit into
Conversation
manzoorwanijk
requested review from
ajitbohra,
desrosj,
nerrad and
ntwb
as code owners
manzoorwanijk
added
the
[Type] Build Tooling
Member
Author
|
This should fail the consistency check unless we run |
|
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. |
Member
Author
|
So, you can see the lint failing: |
|
Size Change: 0 B Total Size: 7.95 MB ℹ️ View Unchanged
|
Contributor
|
Leaving here a notes from a first round of AI-assisted review Click to expandPR Review: #77950 — Tools: Lint dependency version consistency with SyncpackSummaryAdopting Syncpack to enforce cross-workspace dependency consistency is a clear win and a good replacement for the ad-hoc Two things give me pause and I'd like to discuss before merging:
The Findings Overview
1. [critical]
|
| Capability | What Syncpack does | Where it would fit in Gutenberg | Recommendation |
|---|---|---|---|
format command |
Sorts package.json keys (e.g., name/description/keywords first), sorts deps alphabetically, normalises indentation. |
Nothing else does this today. lint:pkg-json (npm-package-json-lint) only validates content rules (description-format, valid-values-author, require-publishConfig, …) — it does not sort. |
Worth a follow-up. Add syncpack format --check to lint, syncpack format to format. Reduces diff noise in package.json files. |
Banned dependencies (isBanned: true) |
Fails the lint if a banned package is declared in any package.json. |
Complementary to tools/eslint/config.mjs's no-restricted-imports, which catches imports in source. There's overlap for declared-and-imported packages (e.g., classnames, lodash, framer-motion) and a gap for transitive deps (e.g., @base-ui/react is intentionally allowed in @wordpress/ui only — that's an import-level concern, not a declaration-level one). |
Modest follow-up. Banning at the declaration level would catch things that ESLint can miss (e.g., a workspace silently re-adding classnames to its dependencies). I'd start with the unambiguous ones: classnames (we standardised on clsx), and any @wordpress/edit-post/edit-site/edit-widgets that shouldn't be cross-imported between packages. ESLint stays in charge of import-level rules. |
pinVersion |
Forces a dep to a specific version everywhere. | The root-level overrides: { jsdom: 26.1.0 } is a different thing (resolution override). I don't see a strong current use case. |
Not now. Revisit if we hit another targeted-pin scenario like the original TS 3.5→3.8 issue. |
update --check |
Reports which deps have newer versions on npm. | Could replace ad-hoc Renovate/Dependabot queries during release prep. | Optional. Mostly nice-to-have; less urgent. |
Catalogs (policy: 'catalog', etc.) |
pnpm/bun catalog support. | Gutenberg uses npm, not pnpm/bun. | Not applicable. |
Replace validate-package-lock.js (lint:lockfile) |
— | Syncpack doesn't lint lockfiles; the custom script stays. | Keep as is. |
Replace lint:pkg-json (wp-scripts lint-pkg-json) |
— | Different concern (content rules vs version consistency). | Keep both, they're complementary. |
So the natural next step — independent of this PR — would be a small follow-up that adds syncpack format and a tightly-scoped isBanned group for things we've explicitly committed to dropping (e.g., classnames).
Final note
None of the findings above require a rewrite — but (1)–(3) ask for explicit policy decisions before this lands, since merging as-is bakes in non-obvious semantics (silent major bumps via :fix, caret-vs-exact reversal, self-violating dev dep). Happy with whichever direction you pick, just want it documented in the PR.
Member
Author
|
Thanks for the thorough review! @ciampo
Yes, that is true. IMHO, we can remove
The last part is compelling enough to use caret ranges - otherwise consumers of our published packages won't receive security/patch updates for transitive deps we declare as exact. Curious to hear what @aduth thinks now that Syncpack is on the table.
Fair. The first run does carry drift, but going forward it just means "if a dependency is bumped in one area of the repo, it must be bumped everywhere" - which is the property we want. The realignment is moving to a separate stacked PR (see #77954) so each bump is reviewable, with the major one (
Yes - this PR only adds the tool. Fixes (including the
Good call - happy to drop to the latest
The case the original validator caught (cryptic TS errors from a stale install after a TS upgrade) is now well covered by
Yes - if we land the
Strong agree on |
Member
Author
|
If we agree on this one, I will address the version updates from #77954 in separate smaller, relevant chunks. |
manzoorwanijk
force-pushed
the
add/syncpack-dependency-version-linter
branch
from
b29631c to
0d2b48a
Compare
Contributor
I think there may still be value in linting the local setup — I can imagine the frustration in case I missed an |
Member
Author
Fair enough. But, my question is why only |
Contributor
|
Fair point. Maybe something to re-think more wholistically as a follow-up, to tackle it in a more foundational way (and not just restricted to TS version)? |
Member
Author
Yes, I agree. Let me revert that change as it doesn't necessarily need to be a part of this PR. |
This was referenced May 5, 2026
manzoorwanijk
force-pushed
the
add/syncpack-dependency-version-linter
branch
from
348da34 to
d1d7b63
Compare
manzoorwanijk
added a commit
that referenced
this pull request
May 8, 2026
Aligns `packages/editor` and `packages/block-editor` with `packages/sync` on `diff@^8.0.3` (needed for the Syncpack alignment work in #77950 / #77954). The bump exposes two unrelated upstream changes that would regress the post-revisions UI: 1. v6+ adds a "deletions before insertions" tie-breaker, so for inputs with multiple equal-length LCSes (whitespace-block pivots, paragraph swaps), `diffArrays` selects a different match than v4 did. The downstream `pairSimilarBlocks` step then mis-pairs blocks and shows two confusing inline diffs instead of a clean modified+unchanged pair. 2. v6+ stops treating whitespace as a token in `diffWords`, coalescing adjacent word changes into one removed/added pair and losing per-word precision in inline rich-text diffs. Fix on the consumer side so existing tests pass without touching any assertion: - Replace the imported `diffArrays` in `block-diff.js` with a local v4-compatible port of `Diff.prototype.diff` (Myers, array+strict-eq), including v4's `(added, removed)` -> `(removed, added)` swap in `buildValues` so condensed sections still render in the right order. - Switch `diffWords` -> `diffWordsWithSpace` for the inline rich-text diff, the `changedAttributes` panel diff, and the `Meta` field diff in `revision-fields-diff`. `preserve-client-ids.js` and `block-compare` (uses `diffChars`) need no changes -- neither hits the affected v6+ behaviours and their tests pass unmodified under v8. 41/41 revision-related unit tests pass; full `npm run test:unit` is green. Closes #77976
Member
I've recently been frustrated by the use of exact package versions in the project, and even sadder to learn that I'm the one who originally added it 😅 Although my logic was sound, because at that time was before we adopted mono-repo pattern for publishing packages (later in #6658). So at the time Gutenberg was primarily an application, with packages published separately through |
Member
Author
Yes, I agree. I have created a separate PR to get rid of that option - #78196 |
manzoorwanijk
force-pushed
the
add/syncpack-dependency-version-linter
branch
from
d1d7b63 to
b94c172
Compare
Member
Author
|
The only blocker for this PR is the upgrade of |
manzoorwanijk
added a commit
that referenced
this pull request
May 21, 2026
Aligns `packages/editor` and `packages/block-editor` with `packages/sync` on `diff@^8.0.3` (needed for the Syncpack alignment work in #77950 / #77954). The bump exposes two unrelated upstream changes that would regress the post-revisions UI: 1. v6+ adds a "deletions before insertions" tie-breaker, so for inputs with multiple equal-length LCSes (whitespace-block pivots, paragraph swaps), `diffArrays` selects a different match than v4 did. The downstream `pairSimilarBlocks` step then mis-pairs blocks and shows two confusing inline diffs instead of a clean modified+unchanged pair. 2. v6+ stops treating whitespace as a token in `diffWords`, coalescing adjacent word changes into one removed/added pair and losing per-word precision in inline rich-text diffs. Fix on the consumer side so existing tests pass without touching any assertion: - Replace the imported `diffArrays` in `block-diff.js` with a local v4-compatible port of `Diff.prototype.diff` (Myers, array+strict-eq), including v4's `(added, removed)` -> `(removed, added)` swap in `buildValues` so condensed sections still render in the right order. - Switch `diffWords` -> `diffWordsWithSpace` for the inline rich-text diff, the `changedAttributes` panel diff, and the `Meta` field diff in `revision-fields-diff`. `preserve-client-ids.js` and `block-compare` (uses `diffChars`) need no changes -- neither hits the affected v6+ behaviours and their tests pass unmodified under v8. 41/41 revision-related unit tests pass; full `npm run test:unit` is green. Closes #77976
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
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?
Adds Syncpack as a CI-enforced linter for declared dependency versions across the monorepo
, and removes the bespokebin/packages/validate-typescript-version.jsscript that this generalises.Follow up to #77900.
Why?
@types/nodewas unified in #77900 to fix duplicate-install / type-conflict issues caused by drift across workspaces. A scan of the repo shows that's not an isolated case — there are dozens of similar mismatches today, e.g.:react/react-dom— root pins18.3.1; most workspaces declare^18.0.0;babel-preset-defaultandelementuse^18.3.0.uuid— root11.1.1, seven packages on^14.0.0(major-version drift).clsx—^2.1.1(27),2.1.1(2),^2.1.0(1).colord—^2.7.0(7),^2.9.2(2),2.9.3(1).@babel/core— split between7.25.7,^7.25.7, and>=7.postcss—^8.4.21,^8.4.5,^8.0.0, and a hard-pinned8.4.38.In #77900 @ciampo raised the question of whether we could lint for this kind of cross-workspace alignment — even via a custom script. There was also an earlier review on #21208 (by @sirreal) where @aduth suggested generalising the bespoke
validate-typescript-versioncheck for all dependencies, similar in purpose towp-scripts check-engines. Syncpack does exactly that and is the canonical tool for the job, so there's no reason to roll our own.How?
Introduce Syncpack:
syncpack.config.mjs— threeversionGroups(ignore internal@wordpress/*workspace packages, allowpeerDependenciesto keep their wide ranges viasameRange, enforce a single version everywhere else) and onesemverGrouprequiring^ranges forprod/devdeps across the repo.package.json— addsyncpackas a devDependency, exposelint:deps(syncpack lint) andlint:deps:fix(syncpack fix), and wirelint:depsinto the existinglintconcurrent runner..github/workflows/static-checks.yml— runnpm run lint:depsin CI alongside the existinglint:pkg-jsongate.Retire the bespoke TS version validator:bin/packages/validate-typescript-version.js. Syncpack now enforces the declared-version side at a wider scope; the original cryptic-error scenario from the TS 3.5 → 3.8 transition (Build: Add TypeScript version validation #21208) is well covered today bysave-exact = true, the lockfile lint, andnpm installon CI.bin/build.mjs,bin/dev.mjs, and thebuild:profile-typesscript inpackage.json. Renumber the surrounding// Step Ncomments to keep them sequential.Testing Instructions
Testing Instructions for Keyboard
N/A — no UI changes.
Screenshots or screencast
N/A — tooling/config only.
Use of AI Tools
Drafted with assistance from Claude Code. Configuration, removals, and the PR description were reviewed and edited by hand.