-
Notifications
You must be signed in to change notification settings - Fork 4.6k
@wordpress/data: migrate index.js to index.ts
#73597
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
Conversation
c7f5248 to
800dca2
Compare
|
Size Change: +15 B (0%) Total Size: 2.54 MB
ℹ️ View Unchanged
|
index.js to index.ts
|
This looks fine to me. Is it ready for review? |
|
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. |
manzoorwanijk
left a comment
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.
This looks good to me. I have only one concern about exporting the whole types.ts file.
| export { dispatch } from './dispatch'; | ||
| export { select } from './select'; | ||
|
|
||
| export * from './types'; |
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.
This is the only part that needs a second opinion. I am not sure if we want to expose all the types here.
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 don't know but I'm also not very concerned either, because we could change our decision later. It's not a public API in the sense of a WordPress public API. It's only a public API to npm consumers (where breaking changes are allowed)
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.
Yeah, it makes sense! Are you fine with these changes? Can I merge the PR?
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.
yes
What?
Closes
This PR exports
@wordpress/data’s TypeScript types from its root entrypoint and tighten the public API typings so that downstream packages (like@wordpress/core-dataand routes) get stable, correct types without relying on internalbuild-types/*paths.Why?
Previously, the core generics for
@wordpress/data(such asStoreDescriptorandReduxStoreConfig) were defined insrc/types.tsand emitted tobuild-types/types.d.ts. When@wordpress/core-data’s declarations were generated, thestoreexport ended up typed viaimport("@wordpress/data/build-types/types")…. In the monorepo and in external projects, TypeScript often couldn’t resolve that internal subpath, leading to:This forced consumers to add ad‑hoc
declare module '@wordpress/data'shims undertypings/just to get proper type information. The goal of this PR is to make the published root module,@wordpress/data, the single canonical place to import both runtime functions and types, so that downstream.d.tsfiles no longer need to reference private internal paths.How?
This PR converts the .js + jsdoc approach to define types to a
.tsfie. This allows us to exportStoreDescriptor andReduxStoreConfig` in the index.ts.Testing Instructions
trunk, runnpm run buildpackages/core-data/build-types/index.d.tsexport const store: import("@wordpress/data/build-types/types").StoreDescriptor<import("@wordpress/data/build-types/types").ReduxStoreConfig<any, {typings/wordpress__data.d.ts:https://github.com/WordPress/gutenberg/blob/9d9dce0d9a3e9582eaaa054a50a600bee20b8cc9/typings/wordpress__data.d.ts#L1-L4
npm run build.packages/core-data/build-types/index.d.tsexport const store: import("@wordpress/data").StoreDescriptor< import("@wordpress/data").ReduxStoreConfig<...>