This repository was archived by the owner on Sep 30, 2024. It is now read-only.
chore(svelte): Refactor infinity query implementation#63824
Merged
Conversation
In preparation for other work, this commit substantially refactors the "infinity query" store implementation. The internals have been changed completely which allows us to simplify its public API. - Simpler configuration. Query responses and next variables are now processed and merged in a single function. - Restoration support. So far pages/components had to implement restoring the state of an infinity store on their own. Now the restoration strategy is part of the configuration. Pages/components only have to get an opaque snapshot via `store.capture()` and restore it via `store.restore(snapshot)`. - More predictable state. It wasn't always obvious if the store content stale data e.g. during restoring data. Now `data` will only be set when the data is 'fresh'. - Smarter 'incremental restoration' strategy. This strategy makes multiple requests to restore the previous state. It makes multiple requests because normally requests are cached and there this is fast. When the data is not cached though there is a noticable delay due to waterfall requests. Now we use a simple heuristic to determine whether or not GraqhQL data might be cached. If not we make a single request to restore the state.
ea434d9 to
ff807c1
Compare
fkling
commented
Jul 15, 2024
Comment on lines
226
to
229
Contributor
Author
There was a problem hiding this comment.
By not relying on implicit accumulated state we have more control over the current state.
Comment on lines
-101
to
-106
| $: if (!!commitHistoryQuery) { | ||
| // Reset commit history when the query observable changes. Without | ||
| // this we are showing the commit history of the previously selected | ||
| // file/folder until the new commit history is loaded. | ||
| commitHistory = null | ||
| } |
Contributor
Author
There was a problem hiding this comment.
This "just works" now.
camdencheek
approved these changes
Jul 16, 2024
camdencheek
left a comment
Member
There was a problem hiding this comment.
This is great! This seems to address a big chunk of feedback I had after using InfinityQuery for the new references panel
Comment on lines
-38
to
-39
| // Wait until DOM was update before updating the scroll position | ||
| await tick() |
Comment on lines
-101
to
-106
| $: if (!!commitHistoryQuery) { | ||
| // Reset commit history when the query observable changes. Without | ||
| // this we are showing the commit history of the previously selected | ||
| // file/folder until the new commit history is loaded. | ||
| commitHistory = null | ||
| } |
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 subscribe to this conversation on GitHub.
Already have an account?
Sign in.
2 participants
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.
In preparation for other work, this commit substantially refactors the "infinity query" store implementation. The internals have been changed completely which allows us to simplify its public API.
store.capture()and restore it viastore.restore(snapshot).datawill only be set when the data is 'fresh'.For review I suggest to turn whitespace changes off.
Test plan
Manual testing, unit tests.