Try extracting the block library into a separate module #6351

youknowriad · 2018-04-23T09:48:30Z

related #6275

This PR extracts the block library to a separate module. If you want to understand the reasoning behind the PR, please read #6275

youknowriad · 2018-04-23T09:50:27Z

core-blocks/test/helpers/index.js

+} from '@wordpress/blocks';
+
+// Hack to avoid the wrapping HoCs.
+import { BlockEdit } from '../../../blocks/block-edit';


I still have a failing test, I suspected it's happening because of this but couldn't figure out exactly. Any idea @gziolo

ContrastChecker wasn't exported. Fixed with 6918595 :)

Thanks ❤️

aduth · 2018-04-23T12:25:39Z

Related / conflicting: #6087 cc @kanishkdudeja

youknowriad · 2018-04-23T12:32:25Z

I feel #6087 is more ambitious. Both require the same refactoring inside blocks (use imports from @wordpress/blocks). #6087 tries to build each block in a separate script/stylesheet while this one just keep them in a single stylesheet.

I think the current PR could be considered as the first iteration towards achieving #6087

gziolo · 2018-04-23T13:51:48Z

core-blocks/index.js

@@ -1,11 +1,16 @@
 /**
- * Internal dependencies
+ * External dependencies


Nit: WordPress not External

gziolo

I like this PR a lot. I would vote to have it merged as soon as possible to so we could attack other aspects proposed in the linked issue.

gziolo · 2018-04-23T13:53:02Z

core-blocks/index.js

@@ -78,3 +83,13 @@ export const registerCoreBlocks = () => {
 	setDefaultBlockName( paragraph.name );
 	setUnknownTypeHandlerName( freeform.name );
 };
+
+// Backwards compatibility
+wp.blocks.registerCoreBlocks = () => {


Interesting workaround :)

gziolo · 2018-04-23T13:53:54Z

core-blocks/paragraph/index.js

@@ -61,7 +63,7 @@ const FONT_SIZES = {
 	larger: 48,
 };

-const autocompleters = [ blockAutocompleter, ...defaultAutocompleters ];
+const autocompleters = [ blockAutocompleter, userAutocompleter ];


It should be improved with #6357.

gziolo · 2018-04-23T13:55:24Z

core-blocks/test/full-content.js

-import serialize from '../api/serializer';
-import { getBlockTypes } from '../api/registration';
+import { registerCoreBlocks } from '../';
+import { parse as grammarParse } from '../../blocks/api/post.pegjs';


Just noting that it is imported from blocks.

Yes, I know, it seems fine for a test. I didn't want to expose the grammar parsing as an official API in the blocks module.

Another question is if those tests need to use low-level functions? Maybe there is a simpler way to achieve the same level of confidence.

gziolo · 2018-04-23T13:56:57Z

docs/reference/deprecated.md

@@ -1,5 +1,9 @@
 Gutenberg's deprecation policy is intended to support backwards-compatibility for two minor releases, when possible. The current deprecations are listed below and are grouped by _the version at which they will be removed completely_. If your plugin depends on these behaviors, you must update to the recommended alternative before the noted version.

+## 3.0.0
+
+- `wp.blocks.registerCoreBlocks` function removed. Please use `wp.coreBlocks.registerCoreBlocks` instead.


Question is if we still want to keep registerCoreBlocks or rather refactor further codebase to tackle it differently :)

Yes, Could be useless with the proposed "registerBlock" action, but let's iterate in a separate PR

gziolo · 2018-04-24T08:30:44Z

blocks/api/raw-handling/test/integration/index.js

@@ -8,7 +8,7 @@ import path from 'path';
 /**
 * Internal dependencies
 */
-import { registerCoreBlocks } from '../../../../library';
+import { registerCoreBlocks } from '../../../../../core-blocks';


Should we move this test to core-blocks? It seems to be very dependent on them.

yes, maybe I'll defer to @iseulde and probably leave this to a separate task?

Also, maybe we could improve the core block tests we have now. They are all pretty equivalent relying on blockEditRender and test a snapshot. I think we should improve them one by one and just call the edit, save, transforms etc... without any mocking unless we can't do that for edit for a reason I'm missing?

gziolo

Some things to further improve

There is one test suite with integration tests which might better fit as part of core-blocks module.
There are 4 editor tests which depend on core-blocks module. In the first place, they shouldn't depend on bocks at all. We should investigate it further and try to find a way to make it work differently. This has been an issue for a while already.

Other than that everything looks solid. I'm giving 👍

* Try extracting the block library into a separate module * Add a deprecation message * Blocks: Do not expose default autocompleters * Fix php unit tests * Fix dependencies comment

youknowriad added the Framework Issues related to broader framework topics, especially as it relates to javascript label Apr 23, 2018

youknowriad self-assigned this Apr 23, 2018

youknowriad requested a review from a team April 23, 2018 09:48

youknowriad force-pushed the try/extract-block-library branch from b47f314 to 56c4b7f Compare April 23, 2018 09:49

youknowriad commented Apr 23, 2018

View reviewed changes

gziolo mentioned this pull request Apr 23, 2018

Blocks: Extend BlockEdit context with name and use it for autocompleters #6357

Merged

4 tasks

gziolo reviewed Apr 23, 2018

View reviewed changes

youknowriad force-pushed the try/extract-block-library branch from d0f0509 to 0761e09 Compare April 24, 2018 08:22

gziolo reviewed Apr 24, 2018

View reviewed changes

gziolo approved these changes Apr 24, 2018

View reviewed changes

kanishkdudeja mentioned this pull request Apr 24, 2018

[WIP] Update/build individual blocks #6087

Closed

5 tasks

youknowriad and others added 5 commits April 25, 2018 08:42

Try extracting the block library into a separate module

b928103

Add a deprecation message

227ad5e

Blocks: Do not expose default autocompleters

aee4b45

Fix php unit tests

0ded105

Fix dependencies comment

db6059d

youknowriad force-pushed the try/extract-block-library branch from 0761e09 to db6059d Compare April 25, 2018 07:42

youknowriad merged commit eabf682 into master Apr 25, 2018

youknowriad deleted the try/extract-block-library branch April 25, 2018 07:48

youknowriad mentioned this pull request May 4, 2018

Rethink the separation between editor and blocks module #6275

Closed

7 tasks

aduth mentioned this pull request May 11, 2018

Testing: Fix and update block fixture regeneration #6715

Merged

aduth mentioned this pull request May 24, 2018

Editor: Simplify block context to eliminate createInnerBlockList #6943

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Try extracting the block library into a separate module #6351

Try extracting the block library into a separate module #6351

youknowriad commented Apr 23, 2018

youknowriad Apr 23, 2018

gziolo Apr 23, 2018

youknowriad Apr 23, 2018

aduth commented Apr 23, 2018

youknowriad commented Apr 23, 2018 •

edited

Loading

gziolo Apr 23, 2018 •

edited

Loading

gziolo left a comment

gziolo Apr 23, 2018

gziolo Apr 23, 2018

gziolo Apr 23, 2018

youknowriad Apr 23, 2018

gziolo Apr 24, 2018

gziolo Apr 23, 2018

youknowriad Apr 23, 2018

gziolo Apr 24, 2018 •

edited

Loading

youknowriad Apr 24, 2018

gziolo left a comment

Try extracting the block library into a separate module #6351

Try extracting the block library into a separate module #6351

Conversation

youknowriad commented Apr 23, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

aduth commented Apr 23, 2018

youknowriad commented Apr 23, 2018 • edited Loading

gziolo Apr 23, 2018 • edited Loading

Choose a reason for hiding this comment

gziolo left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

gziolo Apr 24, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

gziolo left a comment

Choose a reason for hiding this comment

youknowriad commented Apr 23, 2018 •

edited

Loading

gziolo Apr 23, 2018 •

edited

Loading

gziolo Apr 24, 2018 •

edited

Loading