Skip to content

fix(cache): remove cache rules from fetch() #7270

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Merged
ovflowd merged 1 commit into from
Nov 21, 2024
Merged

fix(cache): remove cache rules from fetch() #7270

ovflowd merged 1 commit into from
Nov 21, 2024

Conversation

@avivkeller
Copy link
Member

@avivkeller avivkeller commented Nov 21, 2024

Description

Interesting, I think I know what might be happening. Forcing a refresh on local development did work, but that's because it invalidates the browser cache...

Posted by @ovflowd on Slack.

Validation

Hopefully, the preview for this should show all blog posts, and once merged, the main page should do the same.

Fixes https://openjs-foundation.slack.com/archives/CVAMEJ4UV/p1732148207813379

@avivkeller avivkeller requested a review from Copilot November 21, 2024 01:58
@avivkeller avivkeller requested a review from a team as a code owner November 21, 2024 01:58
@vercel
Copy link

vercel bot commented Nov 21, 2024 •
edited
Loading

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Updated (UTC)
nodejs-org ✅ Ready (Inspect) Visit Preview Nov 21, 2024 2:07am

Copilot AI reviewed Nov 21, 2024
View reviewed changes
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Copilot reviewed 1 out of 1 changed files in this pull request and generated no suggestions.

@avivkeller avivkeller changed the title blog: remove cache rule from fetch() fix(cache): remove cache rules from fetch() Nov 21, 2024
ovflowd
ovflowd reviewed Nov 21, 2024
View reviewed changes
@bmuenzenmeyer bmuenzenmeyer added the github_actions:pull-request Trigger Pull Request Checks label Nov 21, 2024
@github-actions github-actions bot removed the github_actions:pull-request Trigger Pull Request Checks label Nov 21, 2024
@github-actions
Copy link
Contributor

github-actions bot commented Nov 21, 2024 •
edited
Loading

Lighthouse Results

URL Performance Accessibility Best Practices SEO Report
/en 🟠 87 🟢 100 🟢 100 🟢 91 🔗
/en/about 🟢 99 🟢 100 🟢 100 🟢 91 🔗
/en/about/previous-releases 🟢 100 🟢 100 🟢 100 🟢 92 🔗
/en/download 🟢 100 🟢 100 🟢 100 🟢 91 🔗
/en/blog 🟢 99 🟢 100 🟢 96 🟢 92 🔗

@github-actions
Copy link
Contributor

github-actions bot commented Nov 21, 2024

Unit Test Coverage Report

Lines Statements Branches Functions
Coverage: 92%
 90.79% (631/695) 72.44% (184/254) 94.53% (121/128)

Unit Test Report

Tests Skipped Failures Errors Time
143 0 💤 0 ❌ 0 🔥 5.511s ⏱️

bmuenzenmeyer
bmuenzenmeyer approved these changes Nov 21, 2024
View reviewed changes
Copy link
Contributor

@bmuenzenmeyer bmuenzenmeyer left a comment

Choose a reason for hiding this comment

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

the content looks right to me on the preview - do we have a sense for whether or not we can truly test this prior to release? the deployments themselves dont have a cache or a new release coming in.

also, I want to keep an eye on that lighthouse - hopefully it's an outlier 87

@avivkeller
Copy link
Member Author

avivkeller commented Nov 21, 2024 •
edited
Loading

also, I want to keep an eye on that lighthouse - hopefully it's an outlier 87

I just ran a lighthouse test (https://pagespeed.web.dev/analysis/https-nodejs-npvr1vjpb-openjs-vercel-app-en/yc38j12yqb). I got a 96 on desktop, 73 on mobile. Although the values seemed to fluctuate

@bmuenzenmeyer
Copy link
Contributor

bmuenzenmeyer commented Nov 21, 2024

next discussion - is this critical enough a bug to fast-track? i would default towards no

@avivkeller
Copy link
Member Author

avivkeller commented Nov 21, 2024

I don't think so.

@ovflowd
Copy link
Member

ovflowd commented Nov 21, 2024

I don't think so.

I do think so! Blog posts are missing on production /blog/ listing which is bad.

@ovflowd
Copy link
Member

ovflowd commented Nov 21, 2024

I'm fast tracking!

@ovflowd ovflowd added this pull request to the merge queue Nov 21, 2024
@ovflowd ovflowd added the fast-track Fast Tracking PRs label Nov 21, 2024
@ovflowd ovflowd removed this pull request from the merge queue due to a manual request Nov 21, 2024
@ovflowd ovflowd merged commit fd3c538 into nodejs:main Nov 21, 2024
21 checks passed
bmuenzenmeyer pushed a commit that referenced this pull request Nov 21, 2024
@avivkeller @bmuenzenmeyer
fix(cache): remove cache rules from fetch() (#7270)
7dbeb77
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

Copilot code review Copilot Copilot left review comments

@bmuenzenmeyer bmuenzenmeyer bmuenzenmeyer approved these changes

@ovflowd ovflowd ovflowd approved these changes

Assignees

No one assigned

Labels

fast-track Fast Tracking PRs

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@avivkeller @bmuenzenmeyer @ovflowd