feat(svelte): Add compare page#63850
Conversation
9d41e38 to
08f670f
Compare
There was a problem hiding this comment.
This is a prop instead of an event because we want to provide a default handler. Also events become props in Svelte 5 anyway.
There was a problem hiding this comment.
Made this an inline element. It doesn't seem to affect any existing callsites.
There was a problem hiding this comment.
Moved all those up one level to be available to the compare page.
There was a problem hiding this comment.
This seems like it could store a potentially very large amount of data in session storage
There was a problem hiding this comment.
diffQuery is an infinity store. The data that's captured depends on the restore strategy used. In this specific case it will be something like {count: x, variables: {first: x}}. What is stored is intentionally opaque to the page. https://github.com/sourcegraph/sourcegraph/blob/08f670f3556ff3895c31a691a6d51f31ba310e87/client/web-sveltekit/src/routes/%5B...repo%3Dreporev%5D/-/compare/%5B...spec%5D/%2Bpage.ts#L78-L83
There was a problem hiding this comment.
Ooooh, gotcha. That's neat. I was thinking this was storing the full dataset. Didn't quite understand the restore strategy part
There was a problem hiding this comment.
The tldr is that we either "paginate" until we loaded the same amount of data (but that can be slow because we have to this sequentially to get the cursor, if the data is not cached), or we simply fetch the first N items and replace any current state with it.
Which of those to use depends on the pagination API provided by the backend. E.g. for some we fetch the first N, N+X, N+2X, etc items, which transfers duplicate data but that also means the call for N+2X items is cached and we can just request that.
With cursor based we usually do first N, first N after C1, first N after C2, etc. If we did first 3N then that request wouldn't be cached and we would fetch the data unnecessarily. In that case simply "repeating" the original queries is faster. If those are not cached though we want to only do the first 3N request...
All of that might change when/if we change our GraphQL caching strategy, but then it should be easy to use a different restore strategy too.
This commit adds the Svelte version of the compare page. Now that we have a revision picker this was easy to implement. A couple of notes: - The URL parameter is a rest parameter ([...spec]) because revisions can contain slashes, and we don't encode those. - I changed the API of the revision selector so that it works on pages that don't have `resolvedRevision` available (also we never needed the `repo` property from that object), and to provide custom select handlers. - Moved the revision to lib since it's used on various different pages. - Unlike the React version this version - Provides forward/backward buttons to browse the commit list - Uses infinity scrolling to load the next diffs - The `filePath` parameter is supported (for backwards compatibility) but we don't use it anywhere atm.
08f670f to
a5751e9
Compare
This commit adds the Svelte version of the compare page. Now that we have a revision picker this was easy to implement.
A couple of notes:
resolvedRevisionavailable (also we never needed therepoproperty from that object), and to provide custom select handlers.filePathparameter is supported (for backwards compatibility) but we don't use it anywhere atm.sg-sk-compare-page.mp4
(note that the avatar size has changed since the video was recorded; it's larger now)
Test plan
Manual testing.