Interactivity API: add tests for the getElement() attributes #63126
+119
−25
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
luisherranz
added
[Type] Code Quality
|
Size Change: +241 B (+0.01%) Total Size: 1.75 MB
ℹ️ View Unchanged
|
30 tasks
|
I gave it some thought, and I don't have any definite answers but rather just some loose thoughts. I'm still getting re-acquainted with the internals of the Interactivity API after a long time away from it, so there's a chance I got sth wrong :)
|
|
I have been reviewing this with @DAreRodz carefully and we think the current logic is fine since it only contains the attributes of the original element. In that case, it seems correct to keep calling it attributes and not props. Now then, since these properties are present in the Virtual DOM, they are actually the attributes that the Interactivity API is aware of. Initially, this might seem incorrect because if someone modifies the DOM imperatively, they could become unsynchronized. But after careful consideration, we believe it's safer this way because it will force people not to make imperative modifications to the DOM but to use the Interactivity API ( Similarly, we believe that making the attributes reactive is not a good idea since it's a client-side feature and could cause problems with server-side rendering. Thus, we think the best approach is to keep them read-only and to ensure that the modification of the DOM is done with a part of the state linked to the In summary, there isn't much more to do in this pull request except to add some tests to confirm the current behavior. |
luisherranz
changed the title
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?
It is still unclear how the attribute object returned by
getElement()works. We should reconsider how it functions:data-wp-bind)?Why?
We've only considered the main use case (read
dataattributes on getters) so far, and it'd be good to think about all its implications to finalize this API.How?
I've only added a couple of e2e tests so far. We can add more and change the implementation as we decide how to move forward.