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

PoC: Pulling JavaScript to the frontend from a Core Block. #29537

Closed
wants to merge 2 commits into from

Conversation

vcanales
Copy link
Member

@vcanales vcanales commented Mar 3, 2021

In order to have an accessible Mobile Responsive Navigation Block, we might have to load some JavaScript in the frontend — bringing in a library such as micromodals, or building something ourselves — to implement keyboard navigation of the Dropdown Menu.

We can load JS in the frontend when the Block is rendered, calling wp_enqueue_script as a part of the render_callback function, but in order to load that JS file, it has to be created somewhere within the build/ directory.

We can’t use the existing index.js file from the block-library package because:

  • It’s not meant to be loaded in the frontend, and even if it were,
  • It contains the JS for all the blocks in the package.

To create this file separately, I came up with adding it as an entry point in webpack, so that it would be created as a part of the build process as a standalone file that can be pulled from a place within the plugins URL.

This is a proof of concept, so the path for the file containing the JS for the Navigation Block is hardcoded. There is room to create a standard directory or file, from which we could load this sort of arbitrary JS for any core block that would need it.

There's also the alternative of loading this sort of JavaScript through the script-loader, but this would detach it from the Gutenberg Plugin.

I realize JS is being avoided in the frontend purposefully, but as far as I can tell the alternative I know to create Dropdown using only HTML and CSS can't be made accessible without JS. If there are other accessible JS-free alternatives, that would also be very helpful!

Types of changes

Checklist:

  • My code is tested.
  • My code follows the WordPress code style.
  • My code follows the accessibility standards.
  • I've tested my changes with keyboard and screen readers.
  • My code has proper inline documentation.
  • I've included developer documentation if appropriate.
  • I've updated all React Native files affected by any refactorings/renamings in this PR.

@vcanales vcanales added the [Type] Experimental Experimental feature or API. label Mar 3, 2021
@vcanales vcanales self-assigned this Mar 3, 2021
@github-actions
Copy link

github-actions bot commented Mar 3, 2021

Size Change: 0 B

Total Size: 1.41 MB

ℹ️ View Unchanged
Filename Size Change
build/a11y/index.js 1.14 kB 0 B
build/annotations/index.js 3.78 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 664 B 0 B
build/block-directory/index.js 8.62 kB 0 B
build/block-directory/style-rtl.css 1 kB 0 B
build/block-directory/style.css 1.01 kB 0 B
build/block-editor/index.js 127 kB 0 B
build/block-editor/style-rtl.css 12.4 kB 0 B
build/block-editor/style.css 12.4 kB 0 B
build/block-library/blocks/archives/editor-rtl.css 61 B 0 B
build/block-library/blocks/archives/editor.css 60 B 0 B
build/block-library/blocks/audio/editor-rtl.css 58 B 0 B
build/block-library/blocks/audio/editor.css 58 B 0 B
build/block-library/blocks/audio/style-rtl.css 112 B 0 B
build/block-library/blocks/audio/style.css 112 B 0 B
build/block-library/blocks/block/editor-rtl.css 161 B 0 B
build/block-library/blocks/block/editor.css 161 B 0 B
build/block-library/blocks/button/editor-rtl.css 475 B 0 B
build/block-library/blocks/button/editor.css 474 B 0 B
build/block-library/blocks/button/style-rtl.css 479 B 0 B
build/block-library/blocks/button/style.css 479 B 0 B
build/block-library/blocks/buttons/editor-rtl.css 315 B 0 B
build/block-library/blocks/buttons/editor.css 315 B 0 B
build/block-library/blocks/buttons/style-rtl.css 364 B 0 B
build/block-library/blocks/buttons/style.css 363 B 0 B
build/block-library/blocks/calendar/style-rtl.css 208 B 0 B
build/block-library/blocks/calendar/style.css 208 B 0 B
build/block-library/blocks/categories/editor-rtl.css 84 B 0 B
build/block-library/blocks/categories/editor.css 83 B 0 B
build/block-library/blocks/categories/style-rtl.css 79 B 0 B
build/block-library/blocks/categories/style.css 79 B 0 B
build/block-library/blocks/code/style-rtl.css 90 B 0 B
build/block-library/blocks/code/style.css 90 B 0 B
build/block-library/blocks/columns/editor-rtl.css 190 B 0 B
build/block-library/blocks/columns/editor.css 190 B 0 B
build/block-library/blocks/columns/style-rtl.css 421 B 0 B
build/block-library/blocks/columns/style.css 421 B 0 B
build/block-library/blocks/cover/editor-rtl.css 605 B 0 B
build/block-library/blocks/cover/editor.css 605 B 0 B
build/block-library/blocks/cover/style-rtl.css 1.24 kB 0 B
build/block-library/blocks/cover/style.css 1.24 kB 0 B
build/block-library/blocks/embed/editor-rtl.css 486 B 0 B
build/block-library/blocks/embed/editor.css 486 B 0 B
build/block-library/blocks/embed/style-rtl.css 401 B 0 B
build/block-library/blocks/embed/style.css 400 B 0 B
build/block-library/blocks/file/editor-rtl.css 199 B 0 B
build/block-library/blocks/file/editor.css 198 B 0 B
build/block-library/blocks/file/style-rtl.css 248 B 0 B
build/block-library/blocks/file/style.css 248 B 0 B
build/block-library/blocks/freeform/editor-rtl.css 2.45 kB 0 B
build/block-library/blocks/freeform/editor.css 2.45 kB 0 B
build/block-library/blocks/gallery/editor-rtl.css 704 B 0 B
build/block-library/blocks/gallery/editor.css 705 B 0 B
build/block-library/blocks/gallery/style-rtl.css 1.11 kB 0 B
build/block-library/blocks/gallery/style.css 1.1 kB 0 B
build/block-library/blocks/group/editor-rtl.css 160 B 0 B
build/block-library/blocks/group/editor.css 160 B 0 B
build/block-library/blocks/group/style-rtl.css 57 B 0 B
build/block-library/blocks/group/style.css 57 B 0 B
build/block-library/blocks/heading/editor-rtl.css 129 B 0 B
build/block-library/blocks/heading/editor.css 129 B 0 B
build/block-library/blocks/heading/style-rtl.css 76 B 0 B
build/block-library/blocks/heading/style.css 76 B 0 B
build/block-library/blocks/html/editor-rtl.css 281 B 0 B
build/block-library/blocks/html/editor.css 281 B 0 B
build/block-library/blocks/image/editor-rtl.css 717 B 0 B
build/block-library/blocks/image/editor.css 716 B 0 B
build/block-library/blocks/image/style-rtl.css 476 B 0 B
build/block-library/blocks/image/style.css 478 B 0 B
build/block-library/blocks/latest-comments/editor-rtl.css 159 B 0 B
build/block-library/blocks/latest-comments/editor.css 158 B 0 B
build/block-library/blocks/latest-comments/style-rtl.css 269 B 0 B
build/block-library/blocks/latest-comments/style.css 269 B 0 B
build/block-library/blocks/latest-posts/editor-rtl.css 137 B 0 B
build/block-library/blocks/latest-posts/editor.css 137 B 0 B
build/block-library/blocks/latest-posts/style-rtl.css 523 B 0 B
build/block-library/blocks/latest-posts/style.css 522 B 0 B
build/block-library/blocks/list/editor-rtl.css 65 B 0 B
build/block-library/blocks/list/editor.css 65 B 0 B
build/block-library/blocks/list/style-rtl.css 63 B 0 B
build/block-library/blocks/list/style.css 63 B 0 B
build/block-library/blocks/media-text/editor-rtl.css 191 B 0 B
build/block-library/blocks/media-text/editor.css 191 B 0 B
build/block-library/blocks/media-text/style-rtl.css 535 B 0 B
build/block-library/blocks/media-text/style.css 532 B 0 B
build/block-library/blocks/more/editor-rtl.css 434 B 0 B
build/block-library/blocks/more/editor.css 434 B 0 B
build/block-library/blocks/navigation-link/editor-rtl.css 626 B 0 B
build/block-library/blocks/navigation-link/editor.css 627 B 0 B
build/block-library/blocks/navigation-link/style-rtl.css 897 B 0 B
build/block-library/blocks/navigation-link/style.css 895 B 0 B
build/block-library/blocks/navigation/editor-rtl.css 1.12 kB 0 B
build/block-library/blocks/navigation/editor.css 1.13 kB 0 B
build/block-library/blocks/navigation/style-rtl.css 204 B 0 B
build/block-library/blocks/navigation/style.css 205 B 0 B
build/block-library/blocks/nextpage/editor-rtl.css 395 B 0 B
build/block-library/blocks/nextpage/editor.css 395 B 0 B
build/block-library/blocks/page-list/editor-rtl.css 170 B 0 B
build/block-library/blocks/page-list/editor.css 170 B 0 B
build/block-library/blocks/page-list/style-rtl.css 167 B 0 B
build/block-library/blocks/page-list/style.css 167 B 0 B
build/block-library/blocks/paragraph/editor-rtl.css 157 B 0 B
build/block-library/blocks/paragraph/editor.css 157 B 0 B
build/block-library/blocks/paragraph/style-rtl.css 247 B 0 B
build/block-library/blocks/paragraph/style.css 248 B 0 B
build/block-library/blocks/post-author/editor-rtl.css 209 B 0 B
build/block-library/blocks/post-author/editor.css 209 B 0 B
build/block-library/blocks/post-author/style-rtl.css 183 B 0 B
build/block-library/blocks/post-author/style.css 184 B 0 B
build/block-library/blocks/post-comments-form/style-rtl.css 250 B 0 B
build/block-library/blocks/post-comments-form/style.css 250 B 0 B
build/block-library/blocks/post-content/editor-rtl.css 139 B 0 B
build/block-library/blocks/post-content/editor.css 139 B 0 B
build/block-library/blocks/post-excerpt/editor-rtl.css 73 B 0 B
build/block-library/blocks/post-excerpt/editor.css 73 B 0 B
build/block-library/blocks/post-featured-image/editor-rtl.css 338 B 0 B
build/block-library/blocks/post-featured-image/editor.css 338 B 0 B
build/block-library/blocks/post-featured-image/style-rtl.css 100 B 0 B
build/block-library/blocks/post-featured-image/style.css 100 B 0 B
build/block-library/blocks/preformatted/style-rtl.css 63 B 0 B
build/block-library/blocks/preformatted/style.css 63 B 0 B
build/block-library/blocks/pullquote/editor-rtl.css 183 B 0 B
build/block-library/blocks/pullquote/editor.css 183 B 0 B
build/block-library/blocks/pullquote/style-rtl.css 318 B 0 B
build/block-library/blocks/pullquote/style.css 318 B 0 B
build/block-library/blocks/query-loop/editor-rtl.css 83 B 0 B
build/block-library/blocks/query-loop/editor.css 82 B 0 B
build/block-library/blocks/query-loop/style-rtl.css 315 B 0 B
build/block-library/blocks/query-loop/style.css 317 B 0 B
build/block-library/blocks/query-pagination-numbers/editor-rtl.css 122 B 0 B
build/block-library/blocks/query-pagination-numbers/editor.css 121 B 0 B
build/block-library/blocks/query-pagination/editor-rtl.css 270 B 0 B
build/block-library/blocks/query-pagination/editor.css 262 B 0 B
build/block-library/blocks/query-pagination/style-rtl.css 168 B 0 B
build/block-library/blocks/query-pagination/style.css 168 B 0 B
build/block-library/blocks/query-title/editor-rtl.css 86 B 0 B
build/block-library/blocks/query-title/editor.css 86 B 0 B
build/block-library/blocks/query/editor-rtl.css 795 B 0 B
build/block-library/blocks/query/editor.css 794 B 0 B
build/block-library/blocks/quote/editor-rtl.css 61 B 0 B
build/block-library/blocks/quote/editor.css 61 B 0 B
build/block-library/blocks/quote/style-rtl.css 169 B 0 B
build/block-library/blocks/quote/style.css 169 B 0 B
build/block-library/blocks/rss/editor-rtl.css 201 B 0 B
build/block-library/blocks/rss/editor.css 202 B 0 B
build/block-library/blocks/rss/style-rtl.css 290 B 0 B
build/block-library/blocks/rss/style.css 290 B 0 B
build/block-library/blocks/search/editor-rtl.css 165 B 0 B
build/block-library/blocks/search/editor.css 165 B 0 B
build/block-library/blocks/search/style-rtl.css 342 B 0 B
build/block-library/blocks/search/style.css 344 B 0 B
build/block-library/blocks/separator/editor-rtl.css 99 B 0 B
build/block-library/blocks/separator/editor.css 99 B 0 B
build/block-library/blocks/separator/style-rtl.css 236 B 0 B
build/block-library/blocks/separator/style.css 236 B 0 B
build/block-library/blocks/shortcode/editor-rtl.css 512 B 0 B
build/block-library/blocks/shortcode/editor.css 512 B 0 B
build/block-library/blocks/site-logo/editor-rtl.css 201 B 0 B
build/block-library/blocks/site-logo/editor.css 201 B 0 B
build/block-library/blocks/site-logo/style-rtl.css 115 B 0 B
build/block-library/blocks/site-logo/style.css 115 B 0 B
build/block-library/blocks/social-link/editor-rtl.css 164 B 0 B
build/block-library/blocks/social-link/editor.css 165 B 0 B
build/block-library/blocks/social-links/editor-rtl.css 776 B 0 B
build/block-library/blocks/social-links/editor.css 776 B 0 B
build/block-library/blocks/social-links/style-rtl.css 1.32 kB 0 B
build/block-library/blocks/social-links/style.css 1.33 kB 0 B
build/block-library/blocks/spacer/editor-rtl.css 317 B 0 B
build/block-library/blocks/spacer/editor.css 317 B 0 B
build/block-library/blocks/spacer/style-rtl.css 48 B 0 B
build/block-library/blocks/spacer/style.css 48 B 0 B
build/block-library/blocks/table/editor-rtl.css 478 B 0 B
build/block-library/blocks/table/editor.css 478 B 0 B
build/block-library/blocks/table/style-rtl.css 402 B 0 B
build/block-library/blocks/table/style.css 402 B 0 B
build/block-library/blocks/tag-cloud/editor-rtl.css 118 B 0 B
build/block-library/blocks/tag-cloud/editor.css 118 B 0 B
build/block-library/blocks/tag-cloud/style-rtl.css 94 B 0 B
build/block-library/blocks/tag-cloud/style.css 94 B 0 B
build/block-library/blocks/template-part/editor-rtl.css 552 B 0 B
build/block-library/blocks/template-part/editor.css 551 B 0 B
build/block-library/blocks/term-description/editor-rtl.css 90 B 0 B
build/block-library/blocks/term-description/editor.css 90 B 0 B
build/block-library/blocks/text-columns/editor-rtl.css 95 B 0 B
build/block-library/blocks/text-columns/editor.css 95 B 0 B
build/block-library/blocks/text-columns/style-rtl.css 166 B 0 B
build/block-library/blocks/text-columns/style.css 166 B 0 B
build/block-library/blocks/verse/editor-rtl.css 50 B 0 B
build/block-library/blocks/verse/editor.css 50 B 0 B
build/block-library/blocks/verse/style-rtl.css 87 B 0 B
build/block-library/blocks/verse/style.css 87 B 0 B
build/block-library/blocks/video/editor-rtl.css 504 B 0 B
build/block-library/blocks/video/editor.css 503 B 0 B
build/block-library/blocks/video/style-rtl.css 187 B 0 B
build/block-library/blocks/video/style.css 187 B 0 B
build/block-library/common-rtl.css 1.1 kB 0 B
build/block-library/common.css 1.1 kB 0 B
build/block-library/editor-rtl.css 9.46 kB 0 B
build/block-library/editor.css 9.47 kB 0 B
build/block-library/index.js 148 kB 0 B
build/block-library/reset-rtl.css 374 B 0 B
build/block-library/reset.css 376 B 0 B
build/block-library/style-rtl.css 8.95 kB 0 B
build/block-library/style.css 8.95 kB 0 B
build/block-library/theme-rtl.css 700 B 0 B
build/block-library/theme.css 701 B 0 B
build/block-serialization-default-parser/index.js 1.87 kB 0 B
build/block-serialization-spec-parser/index.js 3.06 kB 0 B
build/blocks/index.js 48.3 kB 0 B
build/components/index.js 284 kB 0 B
build/components/style-rtl.css 16.2 kB 0 B
build/components/style.css 16.2 kB 0 B
build/compose/index.js 11.2 kB 0 B
build/core-data/index.js 16.7 kB 0 B
build/customize-widgets/index.js 5.99 kB 0 B
build/customize-widgets/style-rtl.css 378 B 0 B
build/customize-widgets/style.css 379 B 0 B
build/data-controls/index.js 830 B 0 B
build/data/index.js 8.87 kB 0 B
build/date/index.js 31.9 kB 0 B
build/deprecated/index.js 787 B 0 B
build/dom-ready/index.js 577 B 0 B
build/dom/index.js 4.97 kB 0 B
build/edit-navigation/index.js 16.4 kB 0 B
build/edit-navigation/style-rtl.css 2.52 kB 0 B
build/edit-navigation/style.css 2.52 kB 0 B
build/edit-post/index.js 307 kB 0 B
build/edit-post/style-rtl.css 7.05 kB 0 B
build/edit-post/style.css 7.04 kB 0 B
build/edit-site/index.js 27.2 kB 0 B
build/edit-site/style-rtl.css 4.51 kB 0 B
build/edit-site/style.css 4.5 kB 0 B
build/edit-widgets/index.js 20.1 kB 0 B
build/edit-widgets/style-rtl.css 3.15 kB 0 B
build/edit-widgets/style.css 3.15 kB 0 B
build/editor/index.js 41.9 kB 0 B
build/editor/style-rtl.css 3.9 kB 0 B
build/editor/style.css 3.9 kB 0 B
build/element/index.js 4.61 kB 0 B
build/escape-html/index.js 735 B 0 B
build/format-library/index.js 6.75 kB 0 B
build/format-library/style-rtl.css 637 B 0 B
build/format-library/style.css 639 B 0 B
build/hooks/index.js 2.28 kB 0 B
build/html-entities/index.js 622 B 0 B
build/i18n/index.js 4.01 kB 0 B
build/is-shallow-equal/index.js 699 B 0 B
build/keyboard-shortcuts/index.js 2.53 kB 0 B
build/keycodes/index.js 1.95 kB 0 B
build/list-reusable-blocks/index.js 3.15 kB 0 B
build/list-reusable-blocks/style-rtl.css 629 B 0 B
build/list-reusable-blocks/style.css 628 B 0 B
build/media-utils/index.js 5.34 kB 0 B
build/notices/index.js 1.85 kB 0 B
build/nux/index.js 3.4 kB 0 B
build/nux/style-rtl.css 731 B 0 B
build/nux/style.css 727 B 0 B
build/plugins/index.js 2.89 kB 0 B
build/primitives/index.js 1.42 kB 0 B
build/priority-queue/index.js 791 B 0 B
build/react-i18n/index.js 1.46 kB 0 B
build/redux-routine/index.js 2.83 kB 0 B
build/reusable-blocks/index.js 3.78 kB 0 B
build/reusable-blocks/style-rtl.css 225 B 0 B
build/reusable-blocks/style.css 225 B 0 B
build/rich-text/index.js 13.3 kB 0 B
build/server-side-render/index.js 2.58 kB 0 B
build/shortcode/index.js 1.7 kB 0 B
build/token-list/index.js 1.27 kB 0 B
build/url/index.js 3.02 kB 0 B
build/viewport/index.js 1.86 kB 0 B
build/warning/index.js 1.14 kB 0 B
build/wordcount/index.js 1.22 kB 0 B

compressed-size-action

@@ -121,6 +121,14 @@ function render_block_core_navigation( $attributes, $content, $block ) {

unset( $attributes['rgbTextColor'], $attributes['rgbBackgroundColor'] );

wp_enqueue_script(
'core_block_navigation_load_modals',
plugins_url( 'gutenberg/build/modals/index.js' ),
Copy link
Contributor

@youknowriad youknowriad Mar 4, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd love for this to be something like frontend_script in the register_block_type server side (kind of like the editor_script we already have), having the declarativness should allow us later to support lazy loading these scripts only when the block is used.

cc @gziolo @aristath might have ideas on this PR too.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What would be the difference with script that already exists and can be rendered on both the front-end and in the editor? How about CSS? Should it follow it?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not aware of it, could you point me to an example?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What would be the difference with script that already exists and can be rendered on both the front-end and in the editor? How about CSS?

General tradeoffs here are loading unnecessary JS/CSS on the frontend (we typically end up needing some specific editor only handling for a block) vs potentially having drifting implementations.

Copy link
Member

@gziolo gziolo Mar 4, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

By script I mean https://github.com/WordPress/gutenberg/blob/da8555d258edbe676fa079fb51252f918ae032e1/docs/designers-developers/developers/block-api/block-metadata.md#script.

The idea was to have script and editorScript, where script works with both the front-end and the editor because you are building the same experience. The same applies to style vs editorStyle.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The WPDefinedAsset type is a subtype of string, where the value represents a path to a JavaScript or CSS file relative to where block.json file is located.

The JS file should be next to block.json and CSS files in the build folder:

Screen Shot 2021-03-05 at 07 14 42

So it'd be:

{
  "script": "file:./front.js "
}

The existing usages of editorStyle within Gutenberg are all pointing to the name of a registered style.

It's only temporary because @aristath couldn't make it work with the file reference because some features were missing in the WordPress core. There might be more reasons, but we definitely want to use file references everywhere.

There is also an important consideration. The folder structure needs to be portable to the WordPress core as it is going to use the same block.json and other files.

Copy link
Member

@gziolo gziolo Mar 5, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One more thing, in #22754, we added logic that automatically enqueues the file defined with script on the front-end when the block is rendered. It was also backported to the WordPress core:

https://github.com/WordPress/wordpress-develop/blob/0db14900ff4d5f3996a70ef3b596cc62c95c5192/src/wp-includes/class-wp-block.php#L228-L230

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's only temporary because @aristath couldn't make it work with the file reference because some features were missing in the WordPress core. There might be more reasons, but we definitely want to use file references everywhere.

I've added logic to webpack so that script.js gets built in the correct place, and tested loading it through block.json — it works fine, so I've removed the script registration for now.

Pathing feel a bit hardcod-y, but it works fine as the only blocks we're interested in here are in the block-library package.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One more thing, in #22754, we added logic that automatically enqueues the file defined with script on the front-end when the block is rendered. It was also backported to the WordPress core:

The following comment indicates that the optimization made there does not keep assets loaded through block.json from being loaded everywhere for Core Blocks.

The assets associated with core block's are always enqueued (source). Thus, they won't be able to take advantage of the optimizations here. At a minimum, the core blocks should be updated to reference the script and style asset handles themselves, rather than creating an ad hoc enqueuing implementation. This should serve as a simplifying refactoring, and ensure consistent behavior of assets enqueuing for all blocks.

  • A separate effort should seek to try to split the core blocks' script and style assets to per-block assets. In the meantime, there's not much benefit to these optimizations for core blocks.

I compared both approaches, and what the comment mentions checks out — when defining the script property for a Core Block, JS is loaded regardless of the blocks presence in the post.

As the changes in this PR are meant to affect only Core Blocks, it seems like the best course of action is still loading these files through the block's render_callback; this ensures that the JavaScript is loaded only if the block enqueuing it is present.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I compared both approaches, and what the comment mentions checks out — when defining the script property for a Core Block, JS is loaded regardless of the blocks presence in the post.

Do you mean that you confirmed with tests that the script defined with script in block.json enqueue on the front-end even when it's not rendered? I'm sure it worked properly at least for CSS with the changes that @aristath landed in #25220. Ah, I see now, it only applies to FSE themes or themes that opt-in for the behavior with load_separate_block_styles. By the way, we should change the name of the filter before backporting it to the WordPress core to cover the scripts part as well.

@@ -93,6 +94,13 @@ module.exports = {
entry: gutenbergPackages.reduce( ( memo, packageName ) => {
const name = camelCaseDash( packageName );
memo[ name ] = `./packages/${ packageName }`;

const modalsPath = `${ memo[ name ] }/src/navigation/modals`;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe we should do something a bit more generic. Basically add packages/block-library/src/*/frontend.js as entry points, so that any block that has a "frontend.js" on its folder get that frontend script automatically registered and enqueued properly.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Agreed, that is what I was thinking; creating a standard directory, or file, that if present will be picked up by webpack and output to build/ so that we can use it later.

I'll add that generic behavior here, so that we can see it working.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It would be important to come up with a good pattern here as we will have to promote it in official docs when adding support to @wordpress/scripts. This is how it's handled for CSS at the moment:

https://github.com/WordPress/gutenberg/tree/trunk/packages/scripts#using-css

index.css – all imported CSS files are bundled into one chunk named after the entry point, which defaults to index.js, and thus the file created becomes index.css. This is for styles used only in the editor.
style-index.css – imported style.css file(s) (applies to SASS and SCSS extensions) get bundled into one style-index.css file that is meant to be used both on the front-end and in the editor.

There is also index.js in the build folder, so it feels like in @wordpress/scripts it should be script-index.js.

Although, maybe we don't need to care about it for core blocks. Yes, I guess it's fine to pick whatever makes sense. It probably would be better to go with script.js if we were to continue using script from register_block_type.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe we should do something a bit more generic. Basically add packages/block-library/src/*/frontend.js as entry points, so that any block that has a "frontend.js" on its folder get that frontend script automatically registered and enqueued properly.

Added something like that in 828e502

Right now it:

  • Looks for script.js files within all blocks in the block-library package.
  • Builds that file and puts it in ./build/block-library/blocks/<block-name>/script.js, and
  • The block.json file at that location takes care of pulling that JS everywhere; editor and frontend.

@gziolo
Copy link
Member

gziolo commented Mar 5, 2021

It's still not clear or me why this functionality would be only loaded on the frontend? What about the preview mode in the editor, shouldn't it also be included there so you could check the experience? I think it's particularly important for the full site editing mode where there isn't an easy way to preview your changes outside of the editor.

@jasmussen
Copy link
Contributor

It's still not clear or me why this functionality would be only loaded on the frontend?

Good point. I think for the specific task at hand, a hamburger menu, we have imagined an editing interface that probably doesn't require the modal interface, but rather creating a facsimile that works on-select. However in the name of making things generic, it does make sense for the script to load in both, even if we end up not using it in the editor.

@vcanales
Copy link
Member Author

vcanales commented Mar 5, 2021

It's still not clear or me why this functionality would be only loaded on the frontend?

As it is right now, this loads a script.js file in the editor, preview mode, and frontend.

@megphillips91
Copy link

I strongly request making a 'standardized' 'right' way to enqueue JS needed in the front end of the blocks so that they can only enqueue the JS if they are existing on the page and adding that information into the WP Block Editor documentation

Also, it would be super helpful if the front end JS had access to the attributes of the block itself. The thought process for developers accustomed to working in a more component based way (like React itself and thinking in "state") would appreciate having that access - even if it was a way to sort of mock up an initial state of the block on the front end and use that information to react to events.

Take for instance a button which pops up a form. This is a great example of a common use case which could greatly benefit from

  1. only loading the front end JS if the block is on the page
  2. having access to attributes such as the link href of the original button so that once the form is submitted the script can send the visitor onto the destination page. It just makes sense and maybe having those attributes could save on performance...

Thank you,

@youknowriad
Copy link
Contributor

Just wanted to bring this comment here #29606 (comment) that suggests that lazy loading frontend JS is already in place (something I didn't know about)

@gziolo
Copy link
Member

gziolo commented Mar 9, 2021

@youknowriad, see my comment:
#29537 (comment)

It is in place, but only for FSE themes or themes that opt-in for the behavior with load_separate_block_styles. Otherwise, all scripts and styles registered are loaded for all pages/posts.

@aristath
Copy link
Member

@youknowriad, see my comment:
#29537 (comment)

It is in place, but only for FSE themes or themes that opt-in for the behavior with load_separate_block_styles. Otherwise, all scripts and styles registered are loaded for all pages/posts.

Yeah, to be more specific this is the code that uses that filter to change the behavior:

gutenberg/lib/compat.php

Lines 105 to 119 in da8555d

if ( gutenberg_should_load_separate_block_styles() ) {
/**
* Avoid enqueueing block assets of all registered blocks for all posts, instead
* deferring to block render mechanics to enqueue scripts, thereby ensuring only
* blocks of the content have their assets enqueued.
*
* This can be removed once minimum support for the plugin is outside the range
* of the version associated with closure of the following ticket.
*
* @see https://core.trac.wordpress.org/ticket/50328
*
* @see WP_Block::render
*/
remove_action( 'enqueue_block_assets', 'wp_enqueue_registered_block_scripts_and_styles' );
}

Since this affects both styles & scripts I submitted a new PR in #29703 to rename it from _styles to _assets. It makes more sense that way and we can use it for both 👍

Copy link
Contributor

@gwwar gwwar left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice progress so far! I did some quick manual testing for the following cases and verified that the script only loads when the nav block is added.

No Navigation Block One in Post Two in Post Two in Post + One in Header
No navigation block one in post two in post two in post + one in header

@vcanales do you mind adding more script.js files for more blocks for the PoC? I think it'd be helpful to see what bundles get generated for different block combinations.

Also tagging @sgomes @josephscott in case y'all had thoughts on loading JS on the frontend for core Gutenberg blocks.

@sgomes
Copy link
Contributor

sgomes commented Mar 15, 2021

Also tagging @sgomes @josephscott in case y'all had thoughts on loading JS on the frontend for core Gutenberg blocks.

Thank you for looping me in, @gwwar! 👍

I don't know enough about all the constraints to recommend a particular solution, but it looks to me that the principles to follow would be:

  • Only load when needed (i.e., when the block is displayed)
  • Make sure it only gets loaded once, even when there are multiple copies of the block
  • Keep things small

In general, all of those criteria seem to be fulfilled by this solution.

Things like shared dependencies are also taken care of by this approach, assuming that the webpack configuration is set up to share chunks between different entry points.

The only issue I see with this approach is that it involves tying into the Gutenberg build system, which would essentially make this functionality available to core blocks only, as I understand it. If loading JS for a block proves to be a common need (which so far doesn't seem to be the case), then it may be a good idea to bake a standard way of doing this into the Gutenberg API. Once that's present, it could be leveraged by external blocks, but also perhaps used to lazily load any libraries that might only be in use in a core block or two at the moment? Not sure if that's currently the case for any core blocks.

@vcanales
Copy link
Member Author

vcanales commented Mar 16, 2021

The only issue I see with this approach is that it involves tying into the Gutenberg build system, which would essentially make this functionality available to core blocks only, as I understand it.

That is correct; right now it's only targeting Core Blocks.

If loading JS for a block proves to be a common need (which so far doesn't seem to be the case), then it may be a good idea to bake a standard way of doing this into the Gutenberg API.

I believe plugin authors could also find this useful. Right now their only option is manually adding the JavaScript through the .php file within the block, where people are also running into the issue of JS being loaded everywhere instead of when the block is present when following the instructions found on several sources. Here are some examples of this. Not only it's on them to find answers, they also have to build these files separately from the build process of the plugin, as there isn't a way to hook their plugin to our build process.

This PoC only looks within our Block Library to build these Frontend JS files, but by moving this from our main webpack config file into @wordpress/scripts, it might be possible to achieve plugin support — I haven't tried that yet, but I'll make it into the next step, after adding some other tests and proofs for the current state of the PR (as of f4b0d01.)

@vcanales vcanales changed the title Draft: Pulling JavaScript to the frontend from a Core Block. PoC: Pulling JavaScript to the frontend from a Core Block. Mar 16, 2021
@gziolo
Copy link
Member

gziolo commented Mar 18, 2021

We discussed this PR during the weekly WordPress Core JS chat (link requires registration at https://make.wordpress.org/chat/):

https://wordpress.slack.com/archives/C5UNMSU4R/p1615908666037500

I wanted to share my thoughts on how I think we can make sure it can land pretty soon. First of all, we already discussed that register_block_type allows providing two types of script handles:

  • script that gets loaded on the front-end and in the editor
  • editor_script that gets loaded only in the editor

This PR shows us that we might also need to have a way to define a script handle that loads only on the front-end. Let's say we want to use React to render some components. In that case, we would need to call render from @wordpress/element that isn't necessary for the editor because we can hook into the render tree with edit implementation. I propose that we add support for frontend in the register_block_type implementation in the WordPress core.

I'm aware it might take some time to land changes in the WordPress core, so that's why there needs to be a plan for the Gutenberg plugin. The short term solution can look as follows:

  1. We update the webpack config for the Gutenberg plugin to look for packages/block-library/src/*/frontend.js files, and the build process would create an entry point of each of them.
  2. The block would need to provide a PHP implementation of render_callback that returns unmodified content. @mkaz and @nerrad mentioned the strategy during the meeting where they used wp_enqueue_script to load the script when the block is rendered.

@youknowriad
Copy link
Contributor

The block would need to provide a PHP implementation of render_callback that returns unmodified content. @mkaz and @nerrad mentioned the strategy during the meeting where they used wp_enqueue_script to load the script when the block is rendered.

Can't we use the "render_block" hook and just avoid any custom code in the block itself?

@nerrad
Copy link
Contributor

nerrad commented Mar 18, 2021

Can't we use the "render_block" hook and just avoid any custom code in the block itself?

This might be a better option here that covers more scenarios (registering from block.json, registering a block with just name and attributes etc) - assuming I'm understanding the suggestion correctly.

@gziolo
Copy link
Member

gziolo commented Mar 18, 2021

The block would need to provide a PHP implementation of render_callback that returns unmodified content. @mkaz and @nerrad mentioned the strategy during the meeting where they used wp_enqueue_script to load the script when the block is rendered.

Can't we use the "render_block" hook and just avoid any custom code in the block itself?

Sure, that's more or less the same. We only need to make sure we backport the filter to the WordPress core. Well, I think we will have to update the build processing in the core regardless, to account for new JS files. Whatever works best here.

@vcanales vcanales mentioned this pull request Mar 19, 2021
5 tasks
vcanales added a commit that referenced this pull request Mar 19, 2021
jasmussen pushed a commit that referenced this pull request Mar 22, 2021
jasmussen pushed a commit that referenced this pull request Mar 23, 2021
@vcanales
Copy link
Member Author

@youknowriad
@nerrad This might be a better option here that covers more scenarios (registering from block.json, registering a block with just name and attributes etc) - assuming I'm understanding the suggestion correctly.

In my manual testing, I've noticed that loading scripts from block.json works, but it also loads the script even when the block is not present.

With the following in the block.json fie of the Navigation Block:

{
	"style": "wp-block-navigation",
	"script": "file:./frontend.js"
}

And this in packages/block-library/src/navigation/frontend.js

console.log( 'This was loaded from frontend.js in the navigation block' );

I see this when loading a post that doesn't have a Navigation bock:

image

This feels like a bug, because the asset should be pulled on render, and not every time.
(https://github.com/WordPress/wordpress-develop/blob/055deeabff74e1a327fd7142f719622f7b6ff81e/src/wp-includes/class-wp-block.php#L228-L230)

I've got to admit though, that I am not sure if this is the only place where files defined through block.json/"script" are being pulled from.

@vcanales
Copy link
Member Author

vcanales commented Mar 23, 2021

I can share a PR with you where the changes proposed here are used, and are pretty much a pre-requisite:

#30047

In packages/block-library/src/navigation/index.php, within the render_callback function.

        $script_path = __DIR__ . '/navigation/frontend.js';

 	if ( file_exists( $script_path ) ) {
 		wp_enqueue_script(
 			'core_block_navigation_load_frontend_scripts',
 			plugins_url( 'frontend.js', $script_path ),
 			array(),
 			false,
 			true
 		);
 	}

I have also removed usage examples from this PR, as I feel they were confusing the scope of what we're trying to accomplish here.
The intention here is to figure out how to build these files, and — for now — leaving how they're actually loaded up to the user of this API.

I believe it's viable to leave autoloading for a subsequent effort. For instance, we have to figure out why files defined through "script" are being loaded regardless of whether the block as been rendered or not, as pointed out in #29537 (comment).

jasmussen pushed a commit that referenced this pull request Mar 24, 2021
@vcanales
Copy link
Member Author

Closing in favor of #30341

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Type] Experimental Experimental feature or API.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

9 participants