Skip to content
This repository has been archived by the owner on Sep 30, 2024. It is now read-only.

Use cache-first as fetchPolicy for webhook logs #45537

Merged
merged 1 commit into from
Dec 12, 2022

Conversation

pjlast
Copy link
Contributor

@pjlast pjlast commented Dec 12, 2022

Closes #44720

The default policy is 'network-only': https://sourcegraph.com/github.com/sourcegraph/sourcegraph/-/blob/client/http-client/src/graphql/apollo/client.ts?L68

This is different from what the Apollo docs will tell you, because this is our own implementation.

This is what's causing the double requests. However, this should not reload the entire page, it simply determines how the final data is fetched. From what I can gather, the reloading (or rather, the replacing of the combined data with the initial data) is a bug in apollo <3.6 when using the network-only policy (and we're using apollo version 3.5): apollographql/apollo-client#6916

One option is to update the apollo client version, but this feels risky given that we don't know if there could be other regressions that happen because of it. Instead, this PR sets the fetchPolicy to cache-first instead (which is recommended in that thread, and also how we handle it in other parts of the code base).

Test plan

Confirmed to work manually

App preview:

Check out the client app preview documentation to learn more.

@pjlast pjlast requested review from a team December 12, 2022 13:34
@cla-bot cla-bot bot added the cla-signed label Dec 12, 2022
Copy link
Contributor

@sashaostrikov sashaostrikov left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!
PR description diff is 10 times longer than the code diff 😁
We should document this stuff somewhere, because it is very implicit IMHO

@sg-e2e-regression-test-bob

Bundle size report 📦

Initial size Total size Async size Modules
0.00% (0.00 kb) 0.00% (+0.04 kb) 0.00% (+0.04 kb) 0.00% (0)

Look at the Statoscope report for a full comparison between the commits 31f1294 and 7794a49 or learn more.

Open explanation
  • Initial size is the size of the initial bundle (the one that is loaded when you open the page)
  • Total size is the size of the initial bundle + all the async loaded chunks
  • Async size is the size of all the async loaded chunks
  • Modules is the number of modules in the initial bundle

@pjlast pjlast enabled auto-merge (squash) December 12, 2022 13:48
@pjlast pjlast merged commit b6f417a into main Dec 12, 2022
@pjlast pjlast deleted the pjlast/44720-fix-webhook-logs-pagination branch December 12, 2022 13:50
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

webhooks: fix pagination on SiteAdminWebhookPage
3 participants