Interactivity API: Increase directive wp-each-child priority.
#62293
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
cbravobernal
added
[Type] Bug
sirreal
added
Backport to WP 6.7 Beta/RC
|
That's a nice and easy fix. 😄 Not sure if we should add tests to this, as the directive priority levels are already tested in
Regarding this approach (I suggested it 😅), I think it would be unnecessarily complex and wouldn't solve the problem in the browser if, for some reason, the HTML is broken (I mean, HTML that contains an element with |
|
The following accounts have interacted with this PR and/or linked issues. I will continue to update these lists as activity occurs. You can also manually ask me to refresh this list by adding the If you're merging code through a pull request on GitHub, copy and paste the following into the bottom of the merge commit message. To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook. |
luisherranz
previously requested changes
Jun 5, 2024
|
Flaky tests detected in 0d9edda. 🔍 Workflow run URL: https://github.com/WordPress/gutenberg/actions/runs/9379574276
|
|
Checking in on PRs with the Backport to Gutenberg RC. I'll release 18.5 later today, do you think this'll make it in time? |
The e2e test is a little tricky. I'm not sure we will make it. |
|
This pull request has changed or added PHP files. Please confirm whether these changes need to be synced to WordPress Core, and therefore featured in the next release of WordPress. If so, it is recommended to create a new Trac ticket and submit a pull request to the WordPress Core Github repository soon after this pull request is merged. If you're unsure, you can always ask for help in the #core-editor channel in WordPress Slack. Thank you! ❤️ View changed files❔ packages/e2e-tests/plugins/interactive-blocks/directive-each/render.php |
|
Size Change: +645 B (+0.04%) Total Size: 1.75 MB
ℹ️ View Unchanged
|
DAreRodz
approved these changes
Jun 5, 2024
cbravobernal
dismissed
luisherranz’s stale review
Changes applied. Only a test is required, that should not block the release.
|
I just cherry-picked this PR to the release/18.5 branch to get it included in the next release: af2e67a |
vcanales
added a commit
that referenced
this pull request
Jun 5, 2024
* Increase directive priority * Increase directive priority * Add e2e test * Refactor e2e test --------- Co-authored-by: cbravobernal <cbravobernal@git.wordpress.org> Co-authored-by: DAreRodz <darerodz@git.wordpress.org> Co-authored-by: luisherranz <luisherranz@git.wordpress.org> Co-authored-by: vcanales <vcanales@git.wordpress.org>
vcanales
removed
the
Backport to Gutenberg RC
ellatrix
pushed a commit
that referenced
this pull request
Jun 11, 2024
* Increase directive priority * Increase directive priority * Add e2e test * Refactor e2e test --------- Co-authored-by: cbravobernal <cbravobernal@git.wordpress.org> Co-authored-by: DAreRodz <darerodz@git.wordpress.org> Co-authored-by: luisherranz <luisherranz@git.wordpress.org> Co-authored-by: vcanales <vcanales@git.wordpress.org>
ellatrix
pushed a commit
that referenced
this pull request
Jun 11, 2024
* Increase directive priority * Increase directive priority * Add e2e test * Refactor e2e test --------- Co-authored-by: cbravobernal <cbravobernal@git.wordpress.org> Co-authored-by: DAreRodz <darerodz@git.wordpress.org> Co-authored-by: luisherranz <luisherranz@git.wordpress.org> Co-authored-by: vcanales <vcanales@git.wordpress.org>
patil-vipul
pushed a commit
to patil-vipul/gutenberg
that referenced
this pull request
Jun 17, 2024
…Press#62293) * Increase directive priority * Increase directive priority * Add e2e test * Refactor e2e test --------- Co-authored-by: cbravobernal <cbravobernal@git.wordpress.org> Co-authored-by: DAreRodz <darerodz@git.wordpress.org> Co-authored-by: luisherranz <luisherranz@git.wordpress.org> Co-authored-by: vcanales <vcanales@git.wordpress.org>
ellatrix
removed
the
Backport to WP 6.7 Beta/RC
|
This was cherry-picked to the wp/6.6 branch. |
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.
What?
Co-authored with @DAreRodz, bug found by @zaguiini
Increase
data-wp-eachpriority to fix trying to process non existent elements in the dom.Why?
To add some context, the
data-wp-each-childdirective is intended to remove the node in which it appears. Why? Becausedata-wp-eachis the one that actually renders it.The issue is that it has the same priority level as most directives, and they are grouped into the same
Directivescomponent and evaluated together. Thus, even though the component returnsnull, all its hooks are evaluated, i.e., the ones thatdata-wp-bind,data-wp-onetc., register. Also, thedata-wp-eachserver-side processing maintains the directives that appear in child nodes. 😬That's why the element is
nullinside theuseInithook called indata-wp-bind; because it is. The thing is thatdata-wp-bindshould not be evaluated (nor other directives).How?
One quick fix for this issue is to increase the
data-wp-each-childpriority, being the first one in being evaluated.Other fix could be not to server side render the directives in the content created by the template tag, in the
data-wp-eachSSR processing. I did not measure it, but being that content already evaluated, adding the directives in the SSR should be triggering a not needed re-evaluation.Testing Instructions
wp-eachSSR, like this one:trunk, an error appears on the console.Screenshots or screencast
PR not applied
PR applied
Leaving the PR on draft until we decide which approach is better.