Allow multiple view scripts per block #36176
Merged
Conversation
This file contains 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
|
This is still an early WIP/POC (not yet functional) |
|
Size Change: +20 B (0%) Total Size: 1.22 MB
ℹ️ View Unchanged
|
That’s nice. We still can improve the initial implementation planned for 5.9 in WordPress core without too much hassle. I think the biggest challenge here is to figure out how to update webpack config so it is able to detect more entry points when there are multiple view scripts. |
gziolo
added
[Feature] Block API
11 tasks
Yeah, I haven't figured out that part yet 😅 Any ideas? |
We might need a more complex pattern matching for view scripts because |
|
In #37661, I improved the discovery of scripts used by individual custom blocks by scanning the |
aristath
force-pushed
the
add/multiple-view-scripts
branch
2 times, most recently
from
107b772
to
a280634
Compare
gziolo
force-pushed
the
add/multiple-view-scripts
branch
from
ea3d659
to
8e5fc20
Compare
We paired yesterday with @SantosGuillamot to check where we are with this PR. We managed to find a way to update webpack config for Gutenberg so it produces build files for every JavaScript file starting with |
aristath
force-pushed
the
add/multiple-view-scripts
branch
from
8e5fc20
to
c1efed0
Compare
aristath
changed the title
|
That is awesome @gziolo and @SantosGuillamot, it now works as expected! |
aristath
requested review from
TimothyBJacobs,
spacedmonkey,
tellthemachines,
ajitbohra,
ajlende and
mkaz
as code owners
gziolo
reviewed
Mar 26, 2022
There was a problem hiding this comment.
Nice to see it nearly ready to land. I left some detailed feedback after I compared the script handling with WP core.
We should also update the documentation for block metadata with a note that it’s going to be possible to pass an array:
Something similar to previous notes:
Note: An option to pass also an array of editor styles exists since WordPress
5.9.0.
One more thing. For styles (excluding the first one that goes through legacy handling) we had a special render filters wired so they enqueue styles when block is present on the page. Is it possible to add something similar here?
aristath
force-pushed
the
add/multiple-view-scripts
branch
from
14b36dc
to
6e9f303
Compare
I think that's the only remaining thing that doesn't apply to the Navigation block that has manual handling inside the It's completely different from how we handle styles where we conditionally enqueue all styles listed (excluding the first one which goes through the default handling in WP core) with Lines 571 to 594 in 23f11a1 Should we also introduce a similar helper function called |
|
🤔 Is the Navigation block going against the grain here and not following conventions or is this a special case? 👀 |
I would say the opposite. The Navigation and File blocks were updated first so the special case for @getdave, can you confirm that the proposed changes to the frontend handling on the Navigation block doesn't break anything? I'm not sure whether @aristath added those changes for development purposes or it's a planned improvement that optimizes client-side code loaded depending on user selection. |
Co-authored-by: Greg Ziółkowski <[email protected]>
aristath
force-pushed
the
add/multiple-view-scripts
branch
from
1f7647e
to
afbfbe3
Compare
Good point. I removed the changes from this PR, and moved them to #40135 👍 |
gziolo
approved these changes
Apr 7, 2022
gziolo
added
the
Backport to WP 6.7 Beta/RC
bph
added
the
Needs Dev Note
37 tasks
This was referenced Apr 8, 2022
gziolo
removed
the
Backport to WP 6.7 Beta/RC
|
As noted in #39889 (comment), we didn't manage to backport these changes on time to include in WordPress 6.0 release. Let's plan to have it included in WordPress 6.1. I will update the code to ensure it reflects the revised plan. |
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.
Description
Part of #33542.
This PR allows using multiple view scripts per block, by using an array for the block's
viewScript.This will allow enqueueing smaller files where appropriate, and conditionally load scripts where needed.
Included in the PR is the implementation for enqueues, as well as modifications to the navigation block, so it only loads the modals script on the frontend when a modal is used.
How has this been tested?
npm run build, confirming that both view script files get generatedUPDATE: The navigation block changes were moved to a separate PR: #40135
Checklist:
*.native.jsfiles for terms that need renaming or removal).