Skip to content

Conversation

@gastonfournier
Copy link
Contributor

About the changes

This tries to respect etag from the server and send that as If-None-Match header when fetching.

@gastonfournier gastonfournier requested a review from Tymek October 22, 2025 07:56
@gastonfournier gastonfournier self-assigned this Oct 22, 2025
@gastonfournier gastonfournier moved this from New to In Progress in Issues and PRs Oct 23, 2025
if (fetchOptions.headers) {
Object.entries(fetchOptions.headers).forEach(([key, value]) => {
headers[key.toLowerCase()] = value;
if (value != null) {
Copy link
Contributor

Choose a reason for hiding this comment

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

good change with null value. a test covering that could be useful

Copy link
Contributor

@Tymek Tymek left a comment

Choose a reason for hiding this comment

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

Ok, in theory this should work. If we get 304 only after sending "if none match", that's good. It also opens doors to other types of caching, for example time-based, in the future.

Comment on lines +47 to +49
mockFetch.mockResolvedValue(
createResponse({ version: 1, features: [] }, { etag: "etag-1" })
);
Copy link
Contributor

Choose a reason for hiding this comment

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

I don't think there should be a need to change all previous tests

@github-project-automation github-project-automation bot moved this from In Progress to Approved PRs in Issues and PRs Oct 23, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: Approved PRs

Development

Successfully merging this pull request may close these issues.

2 participants