-
Notifications
You must be signed in to change notification settings - Fork 4.3k
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
Types: Resolve conflicts with DefinitelyTyped #21767
Comments
I've started work in DefinitelyTyped to access React types directly: |
Good writeup. Right away I'm also wondering if the following two options could be put in the pool of considerations.... 1. Don't hide React stuff behind the
|
I think this does align with the first option in the description Use React types directly from react. Everyone relies on react types and they naturally align.
It's worth discussing, but it's likely larger than we can tackle here 🙂 Some difficulty with types probably isn't sufficient to drop the facade, but it is another factor to consider. |
DefinitelyTyped/DefinitelyTyped#44086 is ready, it's the first step for Use React types directly from react. #21781 restores the types in affected packages (element, icons, primitives) and was used to generate the |
Approved the first-step PR. Should merge shortly(ish). |
DefinitelyTyped/DefinitelyTyped#44086 has been merged, which unblocks publishing |
* Restore element, icons, primitives types * Partially reverts #21613 (c2684b2) * Reverts #21784 (74fcd4c) Some `@wordpress/*` emit TypeScript type declarations and include them with the published packages. Shortly after publishing, it was discovered the `@wordpress/element` conflicted with the DefinitelyTyped 3rd party type declarations of several packages and a patch release was published removing the published types from `@wordpress/element`, `@wordpress/primitives`, and `@wordpress/icons` (#21613, #21784). The DefinitelyTyped type declarations have been updated to remove conflicts with the included `@wordpress/element` types. The types can be published again. See #21767 for more details.
#21781 has been merged. That should indicate the conclusion of this issue. |
Summary
Some
@wordpress/*
emit TypeScript type declarations and include them with the published packages. Shortly after publishing, it was discovered the@wordpress/element
conflicted with the DefinitelyTyped (DT) 3rd party type declarations of several packages and a patch release was published removing the published types from@wordpress/element
,@wordpress/primitives
, and@wordpress/icons
(#21613).This issue is for the purpose of laying out the problem and discussing paths forward.
The problem
One major purpose of
@wordpress/element
is an abstraction over React. In many ways, it is a very thin layer that re-exports React functionality.Third party types on DT expose all of the React types. The implementation in the
@wordpress/element
package is selective about what it exposes from React.This means that the DT types for
@wordpress/element
exposes all of the React types, and other@wordpress/*
DT types make use of react via@wordpress/element
. For example, theComponentType
in@wordpress/blocks
DT package comes from@wordpress/element
's DT types, but is just React'sComponentType
.When the 1st party
@wordpress/element
types were published, they superseded the DT types. They do not include the re-exports of React types. This causes a cascade of issues for other DT types depending on@wordpress/element
for React types.Considerations
In an ideal future, all packages provide 1st party type declarations which are verified by the TypeScript type system. The need for 3rd party types will go away and the types can be deprecated. There is a significant amount of work to do before we can realize this future.
In the interim, DT types provide value for packages that don't yet publish their own declarations.
Packages other than the DT types may rely on the
@wordpress/element
-provided React types.Possible solutions
Please edit to add additional solutions. It's worth considering all paths forward.
Use React types directly from react
Rather than expose React types from
@wordpress/element
,@wordpress/element
and other packages would reference React types directly.Pros: Simplicity, clear path forward, less to maintain.
Cons: Exposes React directly to consumers, breaking the facade.
Ignore conflicts with DT
We could republish the
@wordpress/element
package with types as it is now. This is known to break some DT types and will affect consumers relying on those types.Pros: Requires no effort.
Cons: Anyone relying on DT types for certain packages in negatively impacted.
We don't strictly have any obligation to maintain compatibility with DT types. I would discourage this course of action.
Expose React types from
@wordpress/element
Just like
@wordpress/element
eposes a lot of functionality from React, types could also be exposed. In practice, this has proved to be difficult.Pros: Maintain the facade in front of React.
Cons: A lot of work.
While TypeScript has clear ways of importing and exporting types, I was unable to re-export React types from
@wordpress/element
via JSDoc. At the very least, this would mean taking on a significant maintenance burden.cc: @aduth @dsifford
Related: #18942 #18838
The text was updated successfully, but these errors were encountered: