Merged
Conversation
8cb32c0 to
9ab0989
Compare
9ab0989 to
a9d9b7d
Compare
dmsnell
approved these changes
May 23, 2022
Member
dmsnell
left a comment
There was a problem hiding this comment.
I don't want to hold this up, but I'd like to see some further exploration after this merges to move the individual configs into their entity type files rather than having them all here (as we did with entity-types).
Thanks for working so hard on this.
This was referenced May 23, 2022
westonruter
added a commit
that referenced
this pull request
May 23, 2022
…p-tests-config * 'trunk' of github.com:WordPress/gutenberg: (88 commits) Components: refactor `AlignmentMatrixControl` to pass `exhaustive-deps` (#41167) [RNMobile] Add 'Insert from URL' option to Image block (#40334) [RNMobile] Improvements to Getting Started Guides (#40964) Post Author Name: Add to and from Post Author transformations (#41151) CheckboxControl: Add unit tests (#41165) Improve inline documentation (#41209) Mobile Release v1.76.1 (#41196) Use explicit type definitions for entity configuration (#40995) Scripts: Convert file extension to js in `block.json` during build (#41068) Reflects revert in 6446878 (#41221) get_style_nodes should be compatible with parent method. (#41217) Gallery: Opt-in to axial (column/row) block spacing controls (#41175) Table of Contents block: convert line breaks to spaces in headings. (#41206) Add support for button elements to theme.json (#40260) Global Styles: Load block CSS conditionally (#41160) Update URL (#41188) Improve autocompleter performance (#41197) Site Editor: Set min-width for styles preview (#41198) Remove Navigation Editor screen from experiments page (#40878) Fix broken Page title for pages created inline within in Nav block (#41063) ...
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What?
This PR adds Entity configuration types to
core-data/src/entities.ts.It is an alternative to #40024. See also #39025, a mega branch that proposes TypeScript signatures for all
@wordpress/core-dataselectors.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. That's exactly what this PR enables by introducing the Entity types:
That
PluginEntitytype can then be reused to build thegetEntityRecordsignature.Why write the kind and name twice?
This is a trade-off. I've spent hours exploring the available options with @dmsnell and we concluded that it's only possibl to either:
const config =and miss out on autocompletion, config type validation, and require usingas const. Reuse the JS entities configuration in the TypeScript type system #40024 explored thatconst config =and have all of the above, but at the cost of using super complex type plumbing. This TS playground explores thatThis PR implements the latter approach.
Why have a new config type?
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
PluginEntitymakes reasoning about all these data sources much easier down the road, e.g. it enables the following succinct formulation of the Kind type: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