Editor: Implement `EntityProvider` and use it to refactor custom sources with a backwards compatibility hook for meta sources. by epiqueras · Pull Request #17153 · WordPress/gutenberg

epiqueras · 2019-08-22T21:32:15Z

Description

This PR implements the notorious EntityProvider and a useEntityProp hook, and uses them to refactor custom sources, with a backwards compatibility hook for meta sources.

It's the first step in getting the features of #16565 and:

Editor: Implement the EntityHandlers component and basic template blocks. #17020
Block Library: Implement template blocks using nested block editors. #17118
Block Library: Add EntityProvider, controlled InnerBlocks and basic template blocks. #17135

shipped 🚀

How has this been tested?

All the current test suites that test meta attribute sources were verified to still be passing.

Types of Changes

New Feature: Add a new EntityProvider component and a useEntityProp hook that allows blocks to edit and sync with different entities, removing the need for custom sources in the process.

Checklist:

My code is tested.
My code follows the WordPress code style.
My code follows the accessibility standards.
My code has proper inline documentation.
I've included developer documentation if appropriate.

youknowriad · 2019-08-23T07:32:07Z

 		"@wordpress/blocks": "file:../blocks",
 		"@wordpress/components": "file:../components",
 		"@wordpress/compose": "file:../compose",
+		"@wordpress/core-data": "file:../core-data",


This is not something we should be doing. Adding core-data as a dependency to the generic BlockEditor module (WP agnostic).

I think we can probably use the BlockListBlock filter to add this in the Editor package instead.

Yeah, good idea.

I paused when doing this initially, but then went with it, because block-library does depend on core-data now, but that is not WP agnostic as block-editor is.

youknowriad · 2019-08-23T07:34:47Z

-				<ReusableBlocksButtons />
-				<ConvertToGroupButtons />
-			</BlockEditorProvider>
+			<EntityProvider kind="postType" type={ post.type } id={ post.id }>


Right, makes me think later once we have the "full site editing" modes, this entity provider wouldn't be added automatically but only when the "Post" block is used (for the other modes, not the regular one).

youknowriad · 2019-08-23T07:35:39Z

+	}
+
+	return [ attributes, setAttributes ];
+}


Can we somehow bail early if we detect that the blockType doesn't have any "meta" attribute (for performance reasons).

Yeah, it does that:

if ( Object.values( attributeTypes ).some( ( type ) => type.source === 'meta' ) ) {

youknowriad

I like this, I wonder if this improves the editor perfs a bit?

epiqueras · 2019-08-23T17:38:57Z

Master:

Average time to load: 4560ms
Average time to DOM content load: 4252ms
Average time to type character: 53.8ms
Slowest time to type character: 129ms
Fastest time to type character: 48ms

This Branch:

Average time to load: 4583ms
Average time to DOM content load: 4297ms
Average time to type character: 47.085ms
Slowest time to type character: 102ms
Fastest time to type character: 41ms

epiqueras · 2019-08-23T17:49:36Z

Change:

Average time to load: 0.50%
Average time to DOM content load: 1.06%
Average time to type character: -12.48%
Slowest time to type character: -20.93%
Fastest time to type character: -14.58%

🚀

…ces with a backwards compatibility hook for meta sources.

youknowriad · 2019-09-20T08:52:06Z

+		return acc;
+	}, {} ),
+	...kinds.reduce( ( acc, kind ) => {
+		acc[ kind.name ] = new Proxy(


This doesn't work in IE11 (Proxy and Reflect), any ideas how we can solve it?

I've been thinking here, it feels like there's unwanted complexity here (the whole entities object), it feels like what we need is just a global variable with two keys and where we generate a new context each time the value is inexistant (a map of map for instance)

We still want it to not work if there is no entity config to avoid unnecessary confusion in digging for errors.

Here: c04c3f8

youknowriad

This is looking good, let's just simplify the context per kind/entityName creation and ship.

youknowriad · 2019-09-23T13:29:09Z

+ */
+import { defaultEntities, kinds } from './entities';
+
+const _entities = {


Can we avoid introducing new coding patterns for consistency (we don't really use _something in our codebase, I assume this is a way to tell it's a private thinig)

Sure, changed.

youknowriad · 2019-09-23T13:31:01Z

+const getEntity = ( kind, type ) => {
+	if ( ! _entities[ kind ] ) {
+		throw new Error( `Missing entity config for kind: ${ kind }.` );
+	}


If we remove this check, and just create an empty object, the first time a "kind" is requested, we can simplify furthere (remove the _entities initialization entirely. I'm not certain it adds much value to try to validate that it's an existing kind because we can also just pass a random entity "type" and it will pass the test (we don't validate entity names).

Because type can be dynamic. kind cannot, you need an entity config for any of the API to work.

I don't see why "kind" can't be dynamic in the future as well tbh. I still think it's . just useless code but I'm fine shipping that way.

razwan · 2019-10-04T09:59:53Z

Hey @epiqueras

This seems to break the editor for me when I add blocks that use post meta as source for one of their attributes since the 6.6.0 update was released.

I've created a small plugin to test this and got the same result:

Cannot read property 'my_block_meta' of undefined

The error comes from the withMetaAttributeSource filter here

Please let me know if I'm doing anything wrong or if there is any way to avoid this until next update if it's really a bug.

Thanks

youknowriad · 2019-10-04T10:02:57Z

@razwan would you mind creating an issue to track this?

razwan · 2019-10-04T11:22:29Z

@youknowriad done. #17767

aduth · 2019-12-05T20:38:32Z

Is the "prop" in useEntityProp intended merely as an abbreviation of "property" (as in a property of the entity object), or does it have any association to a React prop?

epiqueras · 2019-12-05T20:57:12Z

Merely as an abbreviation.

…

On Thu, Dec 5, 2019 at 12:38 PM Andrew Duthie ***@***.***> wrote: Is the "prop" in useEntityProp intended merely as an abbreviation of "property" (as in a property of the entity object), or does it have any association to a React prop? — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#17153?email_source=notifications&email_token=AESFA2GAGMWM3AJQD2SFHEDQXFRETA5CNFSM4IOZOY62YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEGCCECY#issuecomment-562307595>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AESFA2E4GLFGUGMTY4T64WLQXFRETANCNFSM4IOZOY6Q> .

youknowriad · 2019-12-11T17:13:29Z

Added a dev note to document the fact that the meta keys are not available in the getBlockAttributes selector anymore. Devs should rely on getEditedPostAttribute instead. See #19067

epiqueras · 2019-12-11T17:42:22Z

Thanks!

epiqueras added [Package] Core data /packages/core-data [Package] Editor /packages/editor [Package] Block editor /packages/block-editor labels Aug 22, 2019

epiqueras added this to the Future milestone Aug 22, 2019

epiqueras requested review from ellatrix, nerrad, talldan and youknowriad as code owners August 22, 2019 21:32

epiqueras self-assigned this Aug 22, 2019

youknowriad reviewed Aug 23, 2019

View reviewed changes

epiqueras mentioned this pull request Sep 7, 2019

Overview: Custom Entity Sources #17368

Closed

epiqueras added 3 commits September 16, 2019 14:33

Editor: Implement EntityProvider and use it to refactor custom sour…

971b984

…ces with a backwards compatibility hook for meta sources.

Core Data: Handle dynamic entity contexts using a proxy.

9b19acb

Block Editor: Replace dependency on Core Data with an Editor filter.

c484874

epiqueras force-pushed the add/entity-provider-and-refactor-custom-sources branch from 8f3ede8 to c484874 Compare September 16, 2019 18:33

Core Data: Fix linting error from rebase.

7e1b750

youknowriad reviewed Sep 20, 2019

View reviewed changes

epiqueras force-pushed the add/entity-provider-and-refactor-custom-sources branch from eedc6e9 to 7e1b750 Compare September 20, 2019 15:53

Core Data: Simplify proxy usage into a context specific proxy.

c04c3f8

youknowriad reviewed Sep 23, 2019

View reviewed changes

Core Data: Clean up variable names.

3a46a47

youknowriad approved these changes Sep 23, 2019

View reviewed changes

epiqueras merged commit 03e034f into master Sep 23, 2019

youknowriad deleted the add/entity-provider-and-refactor-custom-sources branch September 23, 2019 17:13

youknowriad modified the milestones: Future, Gutenberg 6.6 Sep 30, 2019

youknowriad added the Full Site Editing label Oct 9, 2019

This was referenced Dec 6, 2019

Editor: Shim meta attribute source on registered edit component #18960

Merged

Meta Attribute returning empty after upgrading gutenberg #19067

Closed

youknowriad added the Needs Dev Note Requires a developer note for a major WordPress release cycle label Dec 11, 2019

aduth mentioned this pull request Dec 11, 2019

Editor: Defer block validation until after source application #16569

Closed

jorgefilipecosta mentioned this pull request Feb 12, 2020

Block Editor Dev Notes 5.4 #20185

Closed

23 tasks

aduth mentioned this pull request Feb 28, 2020

Editor: Shim meta attributes for early block registrations #20544

Merged

youknowriad removed the Needs Dev Note Requires a developer note for a major WordPress release cycle label Apr 8, 2020

youknowriad mentioned this pull request Apr 10, 2020

Validation fails if attribute is persisted to post content & meta. #4989

Closed

2 tasks

jsnajdr mentioned this pull request Oct 13, 2020

editor: remove two unused registry controls #26048

Merged

SantosGuillamot mentioned this pull request Aug 4, 2023

Connecting block attributes and custom fields & Block Bindings API for WP 6.5 #53300

Closed

42 tasks

Conversation

epiqueras commented Aug 22, 2019

Description

How has this been tested?

Types of Changes

Checklist:

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

youknowriad left a comment

Choose a reason for hiding this comment

Uh oh!

epiqueras commented Aug 23, 2019

Uh oh!

epiqueras commented Aug 23, 2019

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

youknowriad left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

razwan commented Oct 4, 2019

Uh oh!

youknowriad commented Oct 4, 2019

Uh oh!

razwan commented Oct 4, 2019

Uh oh!

aduth commented Dec 5, 2019

Uh oh!

epiqueras commented Dec 5, 2019 via email

Uh oh!

youknowriad commented Dec 11, 2019

Uh oh!

epiqueras commented Dec 11, 2019

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants