Skip to content
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

Block Directory: Update endpoints to remove __experimental namespace #23528

Merged
merged 8 commits into from
Jul 1, 2020

Conversation

ryelle
Copy link
Contributor

@ryelle ryelle commented Jun 26, 2020

Description

This is in tandem with WordPress/wordpress-develop#359, which adds the plugins & block directory endpoints into core under the wp/v2 namespace. I also had to change how the assets are detected (JS vs CSS), since the regex wasn't matching files with version parameters (see #23499).

I also changed the endpoints in gutenberg to use wp/v2, to preserve compatibility with sites not on trunk.

How has this been tested?

I added a few blocks to a post, they added correctly. I removed some, and saved the post, the unused plugins were uninstalled correctly.

@ryelle ryelle self-assigned this Jun 26, 2020
@ryelle ryelle added the [Feature] Block Directory Related to the Block Directory, a repository of block plugins label Jun 26, 2020
@github-actions
Copy link

github-actions bot commented Jun 26, 2020

Size Change: +29 B (0%)

Total Size: 1.13 MB

Filename Size Change
build/block-directory/index.js 7.42 kB +29 B (0%)
ℹ️ View Unchanged
Filename Size Change
build/a11y/index.js 1.14 kB 0 B
build/annotations/index.js 3.62 kB 0 B
build/api-fetch/index.js 3.4 kB 0 B
build/autop/index.js 2.82 kB 0 B
build/blob/index.js 620 B 0 B
build/block-directory/style-rtl.css 941 B 0 B
build/block-directory/style.css 942 B 0 B
build/block-editor/index.js 109 kB 0 B
build/block-editor/style-rtl.css 10.7 kB 0 B
build/block-editor/style.css 10.7 kB 0 B
build/block-library/editor-rtl.css 7.44 kB 0 B
build/block-library/editor.css 7.44 kB 0 B
build/block-library/index.js 129 kB 0 B
build/block-library/style-rtl.css 8.04 kB 0 B
build/block-library/style.css 8.05 kB 0 B
build/block-library/theme-rtl.css 730 B 0 B
build/block-library/theme.css 732 B 0 B
build/block-serialization-default-parser/index.js 1.88 kB 0 B
build/block-serialization-spec-parser/index.js 3.1 kB 0 B
build/blocks/index.js 48.2 kB 0 B
build/components/index.js 198 kB 0 B
build/components/style-rtl.css 15.9 kB 0 B
build/components/style.css 15.9 kB 0 B
build/compose/index.js 9.65 kB 0 B
build/core-data/index.js 11.4 kB 0 B
build/data-controls/index.js 1.29 kB 0 B
build/data/index.js 8.44 kB 0 B
build/date/index.js 5.47 kB 0 B
build/deprecated/index.js 772 B 0 B
build/dom-ready/index.js 569 B 0 B
build/dom/index.js 3.19 kB 0 B
build/edit-navigation/index.js 9.88 kB 0 B
build/edit-navigation/style-rtl.css 1.02 kB 0 B
build/edit-navigation/style.css 1.02 kB 0 B
build/edit-post/index.js 304 kB 0 B
build/edit-post/style-rtl.css 5.51 kB 0 B
build/edit-post/style.css 5.51 kB 0 B
build/edit-site/index.js 16.6 kB 0 B
build/edit-site/style-rtl.css 3 kB 0 B
build/edit-site/style.css 3 kB 0 B
build/edit-widgets/index.js 9.32 kB 0 B
build/edit-widgets/style-rtl.css 2.42 kB 0 B
build/edit-widgets/style.css 2.42 kB 0 B
build/editor/editor-styles-rtl.css 537 B 0 B
build/editor/editor-styles.css 539 B 0 B
build/editor/index.js 44.9 kB 0 B
build/editor/style-rtl.css 3.85 kB 0 B
build/editor/style.css 3.86 kB 0 B
build/element/index.js 4.65 kB 0 B
build/escape-html/index.js 733 B 0 B
build/format-library/index.js 7.72 kB 0 B
build/format-library/style-rtl.css 547 B 0 B
build/format-library/style.css 548 B 0 B
build/hooks/index.js 2.13 kB 0 B
build/html-entities/index.js 622 B 0 B
build/i18n/index.js 3.56 kB 0 B
build/is-shallow-equal/index.js 710 B 0 B
build/keyboard-shortcuts/index.js 2.52 kB 0 B
build/keycodes/index.js 1.94 kB 0 B
build/list-reusable-blocks/index.js 3.12 kB 0 B
build/list-reusable-blocks/style-rtl.css 450 B 0 B
build/list-reusable-blocks/style.css 451 B 0 B
build/media-utils/index.js 5.29 kB 0 B
build/notices/index.js 1.79 kB 0 B
build/nux/index.js 3.4 kB 0 B
build/nux/style-rtl.css 671 B 0 B
build/nux/style.css 668 B 0 B
build/plugins/index.js 2.56 kB 0 B
build/primitives/index.js 1.5 kB 0 B
build/priority-queue/index.js 788 B 0 B
build/redux-routine/index.js 2.85 kB 0 B
build/rich-text/index.js 14 kB 0 B
build/server-side-render/index.js 2.67 kB 0 B
build/shortcode/index.js 1.69 kB 0 B
build/token-list/index.js 1.28 kB 0 B
build/url/index.js 4.06 kB 0 B
build/viewport/index.js 1.85 kB 0 B
build/warning/index.js 1.14 kB 0 B
build/wordcount/index.js 1.17 kB 0 B

compressed-size-action

@TimothyBJacobs
Copy link
Member

I think it might make sense to completely drop the tests now that the endpoint will be in core shortly. That matches what we did with the block types controller.

@TimothyBJacobs
Copy link
Member

Test failure looks legit

@azaozz
Copy link
Contributor

azaozz commented Jun 27, 2020

I'm probably missing something but how is "moving of end points" from Gutenberg to trunk supposed to work exactly? :)

As far as I see there are couple of ways to do this:

  1. Update the end points, change namespace from __experimental to wp/v2, etc. in both php and js (this is pretty much in this PR). Then ensure it's working as expected, merge the PR and publish the updated packages. Then make a core trunk patch and commit the updated packages together with moving the end points (php) from Gutenberg to trunk, and at the same time merge another PR that removes the end points from Gutenberg. For a short while the end points will be duplicate in core trunk and Gutenberg.

  2. Update the end points, change namespace from __experimental to wp/v2, etc. in both php and js but do not merge the PR. Then make a patch and commit only the end points (php) to core trunk. Then update the PR, removing the end points from Gutenberg. At this point the PR should be using trunk's end points. Then, if all is good, merge the PR, release the new packages and update them in core trunk.

Seems 2 is the better way, and that's what is being done here. Then seems that for this PR to move forward all of the php changes have to be committed to trunk first. Then the js changes can be merged, new packages released, etc.

@TimothyBJacobs
Copy link
Member

I'm probably missing something but how is "moving of end points" from Gutenberg to trunk supposed to work exactly? :)

It's a mess :) I don't think we've done it since 5.0 and the process there was kinda messy IIRC as we started merging things into 5.0 in Core.

For a short while the end points will be duplicate in core trunk and Gutenberg.

Then update the PR, removing the end points from Gutenberg. At this point the PR should be using trunk's end points.

I've been told, and don't really like, that the Gutenberg plugin needs to continue to work for 5.3 and 5.4 users. So we need to duplicate the endpoints in Gutenberg and in WordPress. The endpoints won't load in Gutenberg if the user is running trunk since the class is only included if it doesn't already exist ( see lib/load.php ).

I think this is really error prone, and not sustainable for the REST API endpoints. We'd need to backport any changes to those endpoints from Trunk back to GB so things wouldn't be out of sync.

Seems 2 is the better way, and that's what is being done here.

I prefer that as well.

@azaozz
Copy link
Contributor

azaozz commented Jun 27, 2020

I've been told, and don't really like, that the Gutenberg plugin needs to continue to work for 5.3 and 5.4 users.

I think this is really error prone, and not sustainable for the REST API endpoints. We'd need to backport any changes to those endpoints from Trunk back to GB so things wouldn't be out of sync.

Ah, I see. Yeah, seems quite messy... It's not just the end point php classes, (eventually) other newer code will have to be backported to support older WP. For example wp_get_original_image_path() used in WP_REST_Image_Editor_Controller was introduced in WP 5.3.

Then in 2 above instead of removing the end points from Gutenberg, they will have to be replaced with what's in core trunk before merging the PR. Still seems like the better way to do this.

@StevenDufresne
Copy link
Contributor

I think we also want to update:

@ellatrix
Copy link
Member

Didn't we agree to keep this endpoint experimental?

@TimothyBJacobs
Copy link
Member

Didn't we agree to keep this endpoint experimental?

Not to my knowledge. WordPress/wordpress-develop#359 (comment)

@ellatrix
Copy link
Member

Ok, if it's ready then 👍

@ryelle
Copy link
Contributor Author

ryelle commented Jun 29, 2020

I think it might make sense to completely drop the tests now that the endpoint will be in core shortly. That matches what we did with the block types controller.

Sure, I've removed them in 477d38e

Then in 2 above instead of removing the end points from Gutenberg, they will have to be replaced with what's in core trunk before merging the PR. Still seems like the better way to do this.

That's what is happening here (combined with #23499). The endpoints here will be the same as what are in trunk, and they all now live at wp/v2/plugins etc. This will keep compat with WP 5.4 when 5.5 comes out — 5.5 will load these endpoints from core, while 5.4 will load them from the plugin. It's not ideal to duplicate, but we can remove them when WP 5.6 comes out (added a note about that b537c3f).

@TimothyBJacobs
Copy link
Member

Do we want to switch out to the userCan API at this point?

@ryelle
Copy link
Contributor Author

ryelle commented Jul 1, 2020

Do we want to switch out to the userCan API at this point?

I thought that would be a good idea for a separate PR, so that this one is just about the endpoint changes.

@TimothyBJacobs
Copy link
Member

I thought that would be a good idea for a separate PR, so that this one is just about the endpoint changes.

Sounds good!

@youknowriad youknowriad force-pushed the update/block-dir-endpoint-paths branch from b537c3f to b8bc247 Compare July 1, 2020 09:51
@youknowriad
Copy link
Contributor

Merging this to fix the tests on master.

@youknowriad youknowriad merged commit 7f8b9da into master Jul 1, 2020
@youknowriad youknowriad deleted the update/block-dir-endpoint-paths branch July 1, 2020 10:24
@github-actions github-actions bot added this to the Gutenberg 8.5 milestone Jul 1, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Feature] Block Directory Related to the Block Directory, a repository of block plugins
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants