Client: Apply payload and report what values alone could not do - #2274
Merged
Conversation
marcoroth
added
the
reactivity
github-actions
Bot
added
ruby
|
View your CI Pipeline Execution ↗ for commit fb2a798
💡 Verify your cache is correct by running tasks in a sandbox. Read docs ↗ ☁️ Nx Cloud last updated this comment at |
🌿 Interactive Playground and Documentation PreviewA preview deployment has been built for this pull request. Try out the changes live in the interactive playground: 🌱 Grown from commit ✅ Preview deployment has been cleaned up. |
commit: |
`SlotIndex` could write one slot at a time and nothing more, so every caller with a payload wrote the same walk over it, and the three that did each got it wrong differently first. `apply` takes what `DynamicsCompiler` produces as it comes. A payload names its template, its version and which rendering it is, so nothing has to be said about where it goes, and a partial's values nest inside the slot that rendered it and are handed to that partial's own region. The values compiler stamps the occurrence at the start of the rendering rather than the end, where the region marker counts it, so a template rendering itself agrees with itself. What values cannot do is the other half of the answer. A version that does not match applies nothing, since indices compiled against another template would land somewhere plausible and wrong. A branch the page never had is built when the server parked its markup and reported otherwise. A collection whose rows have changed still fills the rows both sides agree on and reports the rest. A caller reads `deferred` to know what to fetch, and the round trip now goes through `apply` rather than through a loop written for the test. Two row bugs had to go first, both of them `slot.rows` being built at scan time and never maintained. `prune()` walked a region's slots but never its rows, so a row whose markers had left the page stayed indexed and `reconcile` kept asking for it to be removed. And `reconcile` read its order from Map insertion order, fixed at first scan, so asked for the order the page was already in it reported every row as needing a move. `prune()` rebuilds each collection from the rows still connected, in document order, and `reconcile` reads the document. `fillSlots` handled only comment-delimited slots while `blankSlots` handled element anchors too, so `materialize` could not fill a `data-herb-child` or an attribute slot in a parked branch. Nothing caught it because the one test that materializes such a branch passed no values and wrote them afterwards. (cherry picked from commit ac3ee1e9ddb951d69babc49b59e91c9659385ca8) (cherry picked from commit 81b0dbc)
…ler runs it `DynamicsCompiler` put its slot visitor at the front of the stack while a handler compiling the same template to markup puts it at the back. Every visitor in between sees a different tree in each case, and one that adds a dynamic node to one and not the other moves every index after it. Which is the whole failure the visitor was made the source of indices to prevent. Whatever the caller passes runs first now, and the slot visitor last, so the two compiles number the same tree. (cherry picked from commit 85bfad2b073adc9c7a10a5024f9fe1dbe3be8977) (cherry picked from commit c62981c)
This file contains hidden or 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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
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.
This pull request adds
apply, which takes whatDynamicsCompilerproduces as it comes, and reports what values alone could not do.SlotIndexcould write one slot at a time and nothing more, so every caller with a payload wrote the same walk over it, and the three that did each got it wrong differently first. A payload names its template, its version and which rendering it is, so nothing has to be said about where it goes, and a partial's values nest inside the slot that rendered it and are handed to that partial's own region. One call covers a page however many templates it was built from.What values cannot do is the other half of the answer. A version that does not match applies nothing, since indices compiled against another template would land somewhere plausible and wrong. A branch the page never had is built when the server parked its markup, and deferred when it did not. A collection whose rows have changed still fills the rows both sides agree on and reports the rest. A caller reads
deferredto know what to fetch.DynamicsCompileralso put its slot visitor at the front of the visitor stack while a handler compiling the same template to markup puts it at the back. Every visitor in between saw a different tree in each case, and one that adds a dynamic node to one and not the other moves every index after it, which is the whole failure the visitor was made the source of indices to prevent. Whatever the caller passes runs first now, and the slot visitor last, so the two compiles number the same tree.Two row bugs had to go first, both of them
slot.rowsbeing built at scan time and never maintained.prune()walked a region's slots but never its rows, so a row whose markers had left the page stayed indexed andreconcilekept asking for it to be removed. Andreconcileread its order from Map insertion order, fixed at first scan, so asked for the order the page was already in it reported every row as needing a move.prune()rebuilds each collection from the rows still connected, in document order, andreconcilereads the document.