Packages: Fix the published dependency surface of npm packages#79095
Conversation
|
Warning: Type of PR label mismatch To merge this PR, it requires exactly 1 label indicating the type of PR. Other labels are optional and not being checked here.
Read more about Type labels in Gutenberg. Don't worry if you don't have the required permissions to add labels; the PR reviewer should be able to help with the task. |
1 similar comment
|
Warning: Type of PR label mismatch To merge this PR, it requires exactly 1 label indicating the type of PR. Other labels are optional and not being checked here.
Read more about Type labels in Gutenberg. Don't worry if you don't have the required permissions to add labels; the PR reviewer should be able to help with the task. |
6fb2ae4 to
63a86c7
Compare
|
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. |
6e586bd to
0b27173
Compare
|
Size Change: +3 B (0%) Total Size: 8.58 MB 📦 View Changed
|
|
Flaky tests detected in fd1f7ad. 🔍 Workflow run URL: https://github.com/WordPress/gutenberg/actions/runs/27562091144
|
2165ab9 to
9df9a7f
Compare
Maybe we don't care anymore because we'd want to move away from |
1408d49 to
b0d75df
Compare
b0d75df to
f8e8f8f
Compare
Declare every module that packages' published artifacts (runtime JS and emitted .d.ts) reference but did not declare, and stop leaking rememo's types into consumers' declarations: - Move @types/react from dependencies to an optional peerDependency in the 38 packages where react itself is a peer (or types-only), so the consumer's React type version is always used and pure-JS consumers see no warning. - Add the missing react peer to packages that import react/jsx-runtime at runtime: connectors, content-types, lazy-editor, react-i18n. - Declare genuinely missing dependencies: @ariakit/react-core (components), redux (editor), @preact/signals-core (interactivity), rimraf (scripts); move storybook to dependencies in design-system-mcp. - Declare bundler build tools as optional peers of theme (postcss, esbuild, vite), matching its existing stylelint plugin pattern. - Own createSelector's types in @wordpress/data so consumers' emitted declarations reference @wordpress/data instead of rememo, which they do not declare.
f8e8f8f to
fd1f7ad
Compare
aduth
left a comment
There was a problem hiding this comment.
Couple comments, but generally LGTM 👍
| "react-refresh": "^0.14.0", | ||
| "read-pkg-up": "^7.0.1", | ||
| "resolve-bin": "^0.4.0", | ||
| "rimraf": "^5.0.10", |
There was a problem hiding this comment.
Should this be a devDependencies? I only see it used in test files.
There was a problem hiding this comment.
The reason is that test directory gets published to npm, which we should probably avoid. I will do that as a follow-up.
| "esbuild-esm-loader": "^0.3.3", | ||
| "storybook": "^10.2.8" | ||
| "storybook": "^10.2.8", | ||
| "stylelint": "^16.8.2" |
There was a problem hiding this comment.
Is the idea with making this a devDependency as well as an optional peer dependency to make sure that it's available in Gutenberg itself for type-checking?
Should that also apply to everything else we're defining as an optional peer dependency ?
There was a problem hiding this comment.
Yes, if you declare a dependency as peer and you also consume it in the package itself, then dev dependency is what works locally and peer for consumers.
What?
Follow up to #78882 (discussion).
Fixes every remaining case where a package's published artifact — its runtime JS or emitted
build-types/*.d.ts— references a module that consumers installing from npm cannot resolve, as surfaced by auditing all packages with@mawesome/dependency-audit(wired into the repo asnpm run lint:published-depsin #79094, so regressions are caught going forward).Why?
#78882 declared
@types/reactwhere packages' emitted declarations reference React types, but a full audit of the published artifacts showed more gaps, and one placement issue in #78882 itself:@types/reactas a harddependencywhilereactis a peer is the wrong placement. Whenreactis supplied by the consumer, the React type version must match the consumer's React too. A hard dependency drags in our own copy, which can collide with the consumer's@types/react(two structurally-incompatible JSX namespaces) and defeats the decoupling that madereacta peer in the first place. The type surface should mirror the runtime surface: an optionalpeerDependency(optional so pure-JS consumers see no install warning). This is the established pattern in the React ecosystem.react/jsx-runtimeat runtime without declaringreactat all.csstype#74655.@wordpress/dataleakedrememo's types into six consumers' declarations.createSelectorwas a bare re-export, so consumers' inferred selector types were emitted as... & import("rememo").EnhancedSelectorin their.d.ts, and they do not declarerememo. WithskipLibCheck: truethose exports silently degrade toanyfor npm consumers.How?
@types/reactmoves fromdependenciesto an optionalpeerDependencyin the 38 packages wherereactis a peer (or where only the types are referenced).@wordpress/elementkeeps it as a dependency since it depends onreactdirectly.react(^18.0.0) is added as a peer to the four packages that importreact/jsx-runtimeat runtime but never declared it:connectors,content-types,lazy-editor,react-i18n.@ariakit/react-core(components),redux(editor),@preact/signals-core(interactivity),rimraf(scripts);.storybookmoves from dev to prod dependencies indesign-system-mcp(its emitted types reference it)@wordpress/themedeclares its bundler plugins' host tools —postcss,esbuild,vite— as optional peers, following its existingstylelintplugin pattern.@wordpress/datanow owns thecreateSelectortypes:create-selector.tsdeclaresEnhancedSelector/GetDependantsand annotates the export explicitly (same runtime function, zero behavior change), so consumers' emitted declarations referenceimport("@wordpress/data")instead ofrememo. The two types are re-exported from the package index because TypeScript requires the type to be nameable through the package's public entry to emit a portable reference (TS2883 otherwise).No runtime changes anywhere;
npm run buildtype-checks clean and the regeneratedpackages/data/README.mdAPI docs are included.Testing Instructions
npm cinpm run build— confirm the build and type checks pass.grep -r 'import("rememo")' packages/*/build-types/— confirm zero hits (on trunk, 6 packages emit such references).npx @mawesome/dependency-audit packages/<name>.Testing Instructions for Keyboard
N/A — dependency declaration and type-only changes, no UI.
Screenshots or screencast
N/A.
Use of AI Tools
Used Claude Code (with the dependency-audit tool) to identify the affected packages, apply the dependency moves, and validate the results; the Codex CLI reviewed the
createSelectortype-ownership approach. The diff was reviewed before submission.