Repository navigation
Packages: Declare missing @types/react dependency
#78882
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
d2f73c0
77f9b44
94f19c9
42042bb
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Large diffs are not rendered by default.
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,7 @@ | ||
| <!-- Learn how to maintain this file at https://github.com/WordPress/gutenberg/tree/HEAD/packages#maintaining-changelogs. --> | ||
|
|
||
| ## Unreleased | ||
|
|
||
| ### Code Quality | ||
|
|
||
| - Add missing `@types/react` dependency. [#78882](https://github.com/WordPress/gutenberg/pull/78882). |
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
|
|
@@ -110,6 +110,7 @@ | |||||
| "@storybook/react-vite": "^10.2.8", | ||||||
| "@testing-library/jest-dom": "^6.9.1", | ||||||
| "@types/jest": "^29.5.14", | ||||||
| "@types/react-dom": "^19.2.3", | ||||||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Unsure if it's intentional, but we use 19.2.13 everywhere else.
Suggested change
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. That is the version for
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Ah, my mistake, I missed
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Separately, I'd be curious why we're using
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Not sure about that.
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Separately, this seems like something common enough and easy enough to test for in CI ? 🤔
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. No, only |
||||||
| "@wordpress/jest-console": "file:../jest-console", | ||||||
| "snapshot-diff": "^0.10.0", | ||||||
| "storybook": "^10.2.8", | ||||||
|
|
||||||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,7 @@ | ||
| <!-- Learn how to maintain this file at https://github.com/WordPress/gutenberg/tree/HEAD/packages#maintaining-changelogs. --> | ||
|
|
||
| ## Unreleased | ||
|
|
||
| ### Code Quality | ||
|
|
||
| - Add missing `@types/react` dependency. [#78882](https://github.com/WordPress/gutenberg/pull/78882). |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,7 @@ | ||
| <!-- Learn how to maintain this file at https://github.com/WordPress/gutenberg/tree/HEAD/packages#maintaining-changelogs. --> | ||
|
|
||
| ## Unreleased | ||
|
|
||
| ### Code Quality | ||
|
|
||
| - Add missing `@types/react` dependency. [#78882](https://github.com/WordPress/gutenberg/pull/78882). |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,7 @@ | ||
| <!-- Learn how to maintain this file at https://github.com/WordPress/gutenberg/tree/HEAD/packages#maintaining-changelogs. --> | ||
|
|
||
| ## Unreleased | ||
|
|
||
| ### Code Quality | ||
|
|
||
| - Add missing `@types/react` dependency. [#78882](https://github.com/WordPress/gutenberg/pull/78882). |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The distinction between
devDependenciesanddependenciescan get very subtle. This particular package uses React types internally (import type { ComponentType } from 'react';) but the trick is that this usage is not exported, the public.d.tsfiles don't reference it. The function that usesComponentTypeis statically passed toaddFilter.This is fragile and can easily break unexpectedly. Locally everything is fine, because
@types/reactis still innode_modules, but a third party user of the NPM package won't getdevDependenciesinstalled.What if we always put the types into
dependencies?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I’m exploring a tool creation which skips installing devDeps and then tries to resolve imports in type defs. It should hopefully catch any missing dependencies.