fix various issues with limiting streams #3035
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
SteffenDE
force-pushed
the
stream_limit_fixes
branch
from
198dd8c
to
596c674
Compare
phoenixframework@997f15c introduces an issue where limits were not correctly enforced when adding items in bulk. It also fixes an issue with limits being applied differently on resets, because we previously did not remove stream elements when they were being re-added later. I think this was initially introduced to avoid issues with live components not being correctly rendered because of the PHX_SKIP optimization. To counteract this, we now remove all elements on reset to force them being added in the correct order, but also store the live component DOM nodes to correctly restore them. References phoenixframework#2686.
SteffenDE
force-pushed
the
stream_limit_fixes
branch
from
596c674
to
ce4b303
Compare
SteffenDE
commented
Jan 21, 2024
SteffenDE
commented
Jan 21, 2024
SteffenDE
commented
Jan 21, 2024
|
❤️❤️❤️🐥🔥 |
SteffenDE
added a commit
to SteffenDE/phoenix_live_view
that referenced
this pull request
Feb 11, 2024
This is a followup to phoenixframework#3070 and phoenixframework#3035. When adjusting the stream component restore functionality in phoenixframework#3070, I accidentally broke live components in streams, but the tests did not detect this because they only checked the id of the stream items and not their content. Therefore, this commit also adjusts the tests. Another problem that was introduced in phoenixframework#3035 by not leaving re-inserted items in the DOM is that nested streams inside live components inside streams would not be restored properly. This commit addresses this by recursively running morphdom on the restored stream items.
chrismccord
pushed a commit
that referenced
this pull request
Feb 14, 2024
This is a followup to #3070 and #3035. When adjusting the stream component restore functionality in #3070, I accidentally broke live components in streams, but the tests did not detect this because they only checked the id of the stream items and not their content. Therefore, this commit also adjusts the tests. Another problem that was introduced in #3035 by not leaving re-inserted items in the DOM is that nested streams inside live components inside streams would not be restored properly. This commit addresses this by recursively running morphdom on the restored stream items.
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.
997f15c introduces an issue where limits were not correctly enforced when adding items in bulk, this PR addresses this.
It also fixes an issue with limits being applied differently on resets, because we previously did not remove stream elements when they were being re-added later. I think this was initially introduced to avoid issues with live components not being correctly rendered because of the PHX_SKIP optimization. To counteract this, we now remove all elements on reset to force them being added in the correct order, but also store the live component DOM nodes to correctly restore them.
References #2686.