-
Notifications
You must be signed in to change notification settings - Fork 4.6k
Reuse the JS entities configuration in the TypeScript type system #40024
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
|
@dmsnell This PR is the critical fork in the road that will determine how will the entity record selectors be typed. Resolving merge conflicts is tedious here so I'll hold off on that until we converge on the approach. |
|
@adamziel do you have any branches or commits lying around where we tried using something like an if not I'll see if I can build from here. looks like everything is prime finally for playing with this |
f287783 to
9959b2a
Compare
|
@dmsnell I'm not entirely sure what do you mean, but if you refer to the explorations you've done in #39481 then I don't think we have any branches doubling down on that approach anymore. I think the mega-branch done that at one point, but rebasing got out of hand with the number of intermediary commits so I squashed it and lost that work :( I went ahead and recreated some of that in the TS Playground. May not be what you're after, though. |
|
that playground link works! thanks. what I'm after is this: I think we started with the intuition that we would create I want to make sure we don't introduce all the |
Add Entity configuration types to core-data/src/entities.ts for the selector types to lean upon in the upcoming PR. Writing the kind and name twice is a trade-off. I've spent hours exploring the available options with @dmsnell and we concluded that it's only possible to either: * Infer it from the config and miss out on autocompletion, config type validation, and require using as const. Reuse the JS entities configuration in the TypeScript type system #40024 explored that * Infer it from the config and have all of the above, but at the cost of using super complex type plumbing. This TS playground explores that * Type it explicitly, have autocompletion and type validation without complex types, but duplicate a few lines of code. This commit implements the latter approach.
|
Closing in favor of the alternative: #40995 |
What?
This is a subset of #39025 – a mega branch that proposes TypeScript signatures for all
@wordpress/core-dataselectors.This PR adds Entity configuration types to
core-data/src/entities.ts.Why?
Consider the
getEntityRecordselector:gutenberg/packages/core-data/src/selectors.js
Lines 157 to 171 in 395aea4
Different entity kinds, like
root,postType,taxonomy, are associated with different entity names. For example,kind: root, name: pluginis a valid combination, butkind: postType, name: pluginis not. Other valid combinations are configured in theentities.tsfile via a JavaScript object.An ideal
getEntityRecordfunction signature would only accept valid combinations, then require thekeyto be eithernumberor astring, and return the list of corresponding entity records.Again, ideally, there would only be a single source of truth for all the information. I'd rather avoid rewriting them in TypeScript as in my experience this adds maintenance burden, leads to difficult bugs, and gets out of sync sooner or later.
This PR, then, takes all the JavaScript configuration details, and brings them into the TypeScript type system using the
as constassertion:Then, it transforms these const types into an Entity Config type like this:
That
PluginConfigtype can then be reused to create agetEntityRecordsignature as seen in #39025.Why have a new config type when the consts could be used directly?
Good question! It's needed because entity configuration comes from three different sources:
entities.tskind=postTypeiseditas seen inloadPostTypeEntitiesA somewhat eccentric metaphor would be picturing it as a MySQL query that joins three tables:
A common format like the
PluginConfigmakes reasoning about all these data sources much easier down the road, e.g. it enables the following succinct formulation of the Kind type:The downside of
as constis that it provides no autocompletion or type constraints to the developer writing new typesThat's true! It is the price to pay for having a single source of truth. I think it's a worthy trade-off, but there may be a way that enables both using a clever trick I found on StackOverflow:
The downside of that approach is that it requires a few complex types that muddy the big picture. See the related TypeScript playground.
Test plan
Confirm the checks are green and that no entity configuration got changed when I split the large array into atomic declarations. The changes here should only affect the type system so there is nothing to test in the browser.
cc @dmsnell @jsnajdr @youknowriad @sarayourfriend @getdave @draganescu @scruffian