feat: add build cache settings #1669

sambhavgupta0705 · 2023-05-14T12:30:35Z

Description

tried adding the cache settings for the website

Related issue(s)

fixes #843

netlify · 2023-05-14T12:30:48Z

✅ Deploy Preview for asyncapi-website ready!

Built without sensitive environment variables

Name	Link
🔨 Latest commit	`6c653f3`
🔍 Latest deploy log	https://app.netlify.com/sites/asyncapi-website/deploys/647480b5694c280008caa54f
😎 Deploy Preview	https://deploy-preview-1669--asyncapi-website.netlify.app
📱 Preview on mobile	Toggle QR Code... Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site settings.

This reverts commit 00eaff8.

This reverts commit ee8c4c1.

Signed-off-by: Sambhav Gupta <[email protected]>

github-actions · 2023-05-14T15:17:47Z

⚡️ Lighthouse report for the changes in this PR:

Category	Score
🔴 Performance	48
🟢 Accessibility	98
🟢 Best practices	100
🟢 SEO	100
🔴 PWA	30

Lighthouse ran on https://deploy-preview-1669--asyncapi-website.netlify.app/

derberg · 2023-05-15T07:43:09Z

can you explain what you added and why and basing on what docs?

also, why we need netlify cli in the dependencies ?

sambhavgupta0705 · 2023-05-15T08:00:15Z

can you explain what you added and why and basing on what docs?

I went through the netlify blogs and found that we need to install the plugins for nextjs site deployed on netlify to enable caching
https://github.com/netlify/next-runtime#use-with-next-export

also, why we need netlify cli in the dependencies ?

Sorry I forgot to mention I need to check the deploy status so needed to install it .I will remove it before the final merge

derberg

if you just need netlify cli for local testing then just remove it not and use it through npx

derberg · 2023-05-15T12:08:02Z

netlify.toml

@@ -1,6 +1,7 @@
 [[headers]]
  for = "/*"
  [headers.values]
+    Cache-Control = "public, max-age=3600"


the scope of the issue is to enable cache for build,

why is this header added? what will it change?

it is added to control how caching should be performed
The value public, max-age=3600 specifies that the response can be cached by both public and private caches
,and the caching age is 3600 seconds

@sambhavgupta0705

yes but how is it helping with cache build? this is a setting for the server and client side caching. So imho totally unrelated and probably something we should not add

well it just tell the server how long the response should be considered fresh and cacheable.So we can remove it as it doesn't help us that much in build

netlify.toml

derberg · 2023-05-15T12:15:15Z

cache is working

this is build info from your PR. First one, and logs were saying no cache found - which makes sense

then this is a rerun, and logs mentioned cache is use

so we have 1min improvement. Looks like it works. So please just answer the questions to the PR

sambhavgupta0705 · 2023-05-15T16:06:27Z

cache is working

nice I was worried what would be the outcome :)

Signed-off-by: Sambhav Gupta <[email protected]>

sambhavgupta0705 · 2023-05-15T17:20:59Z

@derberg PTAL

sambhavgupta0705

I have added the comments

netlify.toml

sambhavgupta0705 · 2023-05-15T16:02:34Z

netlify.toml

@@ -1,6 +1,7 @@
 [[headers]]
  for = "/*"
  [headers.values]
+    Cache-Control = "public, max-age=3600"


it is added to control how caching should be performed
The value public, max-age=3600 specifies that the response can be cached by both public and private caches
,and the caching age is 3600 seconds

netlify.toml

derberg · 2023-05-16T11:01:18Z

netlify.toml

@@ -1,6 +1,7 @@
 [[headers]]
  for = "/*"
  [headers.values]
+    Cache-Control = "public, max-age=3600"


yes but how is it helping with cache build? this is a setting for the server and client side caching. So imho totally unrelated and probably something we should not add

netlify.toml

sambhavgupta0705 · 2023-05-17T17:13:21Z

@derberg all the things are done form my side ,
Should we check that the cache is working or not once more?

derberg

@sambhavgupta0705 lgtm, just update description that this PR fixes linked issue so it is closed after merge

@akshatnema anything from your side?

akshatnema · 2023-05-23T06:15:17Z

@derberg My concern over the implementation is that aren't we targeting the posts.json content to cache it inside website? With the current implementation inside this PR, I can't figure what content are we caching inside Netlify?

derberg · 2023-05-23T07:53:57Z

@akshatnema this PR and related issue is focused on build cache, not website cache. It is not focused on website performance after deployment but the build performance and its speed. In other words, this PR enables cache inside Netlify build cache, to cache not only posts.json but also other source files, to speed up website rebuild.

So website is not faster thanks to this PR. What is faster is the rebuild of for example preview in the PRs and contribution flow is improved

akshatnema · 2023-05-23T08:07:12Z

@derberg Yeah, I completely understand that it is not building in any cache inside website runtime, but it should build a cache for posts.json and shouldn't run the build of the posts.json every time the build has been initiated.

Also, I see the following error regarding the npm packages:

So, probably, we have to upgrade the node version of Netlify.

@sambhavgupta0705 You have to use Node v16.15.0 and npm v8.5.5 here.

derberg · 2023-05-23T10:38:43Z

@sambhavgupta0705 the change needs to be done in netlify.toml file

@akshatnema regarding posts.json, I don't think there is any specific solution that we can add that will enable that. Building posts.json is a custom script, so what you are suggesting is like adding if into script, that in case for example there were no changes in docs detected, then docs script should basically not run. Or did you have some other idea?

akshatnema · 2023-05-28T11:36:26Z

@derberg For now, I can't able to find anything in the Netlify docs to cache any existing files. Hope we had any CMS or DB for the posts in future so that we can set some caching regarding those in future. So approved ✅

akshatnema · 2023-05-28T11:36:48Z

@sambhavgupta0705 Kindly resolve the conflicts.

Signed-off-by: Sambhav Gupta <[email protected]>

sambhavgupta0705 · 2023-05-28T17:24:03Z

@akshatnema done 👍🏻

derberg

Thanks!

derberg · 2023-05-29T12:37:26Z

/rtm

sambhavgupta0705 added 2 commits May 14, 2023 17:44

feat: added cache settings

ee8c4c1

caching deployment

00eaff8

sambhavgupta0705 requested review from derberg, mcturco, akshatnema, magicmatatjahu and asyncapi-bot-eve as code owners May 14, 2023 12:30

sambhavgupta0705 marked this pull request as draft May 14, 2023 12:32

sambhavgupta0705 added 5 commits May 14, 2023 19:40

Revert "caching deployment"

bd451ef

This reverts commit 00eaff8.

Revert "feat: added cache settings"

9fbc75d

This reverts commit ee8c4c1.

fix: cache settings

ae8841e

environment variable changes

6ddc084

cache control refactored

2aad057

Signed-off-by: Sambhav Gupta <[email protected]>

sambhavgupta0705 marked this pull request as ready for review May 14, 2023 15:17

derberg reviewed May 15, 2023

View reviewed changes

netlify.toml Show resolved Hide resolved

sambhavgupta0705 and others added 2 commits May 15, 2023 21:38

removed package.json

0fbbb2b

Signed-off-by: Sambhav Gupta <[email protected]>

Merge branch 'master' into cache

055508f

sambhavgupta0705 commented May 16, 2023

View reviewed changes

derberg requested changes May 16, 2023

View reviewed changes

sambhavgupta0705 and others added 2 commits May 17, 2023 22:41

removed the cache control from netlify.toml

b36142e

Merge branch 'master' into cache

7e3e4a9

sambhavgupta0705 added 3 commits May 18, 2023 08:21

Merge branch 'master' into cache

193207a

Merge branch 'master' into cache

08a6087

Merge branch 'master' into cache

088c30f

derberg changed the title ~~feat: added cache settings~~ feat: add build cache settings May 22, 2023

derberg previously approved these changes May 22, 2023

View reviewed changes

akshatnema linked an issue May 23, 2023 that may be closed by this pull request

Consider enabling Next.js caching for Netlify builds #843

Closed

netlify error fix

5f5c69f

sambhavgupta0705 dismissed derberg’s stale review via 5f5c69f May 23, 2023 08:29

Merge branch 'master' into cache

c74b252

sambhavgupta0705 and others added 2 commits May 23, 2023 19:26

nfixed node and npm version

287593b

Merge branch 'master' into cache

e416faf

Merge branch 'master' into cache

8174b2a

Signed-off-by: Sambhav Gupta <[email protected]>

Merge branch 'master' into cache

6c653f3

derberg approved these changes May 29, 2023

View reviewed changes

asyncapi-bot added ready-to-merge and removed ready-to-merge labels May 29, 2023

derberg merged commit 80eca5e into asyncapi:master May 29, 2023

asyncapi-bot added the bounty AsyncAPI Bounty program related label label Aug 21, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: add build cache settings #1669

feat: add build cache settings #1669

sambhavgupta0705 commented May 14, 2023 •

edited by derberg

Loading

netlify bot commented May 14, 2023 •

edited

Loading

github-actions bot commented May 14, 2023 •

edited

Loading

derberg commented May 15, 2023

sambhavgupta0705 commented May 15, 2023 •

edited

Loading

derberg left a comment

derberg May 15, 2023

sambhavgupta0705 May 15, 2023

derberg May 16, 2023

derberg May 16, 2023

sambhavgupta0705 May 17, 2023

derberg commented May 15, 2023

sambhavgupta0705 commented May 15, 2023

sambhavgupta0705 commented May 15, 2023

sambhavgupta0705 left a comment

sambhavgupta0705 May 15, 2023

derberg May 16, 2023

sambhavgupta0705 commented May 17, 2023 •

edited

Loading

derberg left a comment

akshatnema commented May 23, 2023

derberg commented May 23, 2023

akshatnema commented May 23, 2023 •

edited

Loading

derberg commented May 23, 2023

akshatnema commented May 28, 2023

akshatnema commented May 28, 2023

sambhavgupta0705 commented May 28, 2023

derberg left a comment

derberg commented May 29, 2023

feat: add build cache settings #1669

feat: add build cache settings #1669

Conversation

sambhavgupta0705 commented May 14, 2023 • edited by derberg Loading

netlify bot commented May 14, 2023 • edited Loading

✅ Deploy Preview for asyncapi-website ready!

github-actions bot commented May 14, 2023 • edited Loading

derberg commented May 15, 2023

sambhavgupta0705 commented May 15, 2023 • edited Loading

derberg left a comment

Choose a reason for hiding this comment

derberg May 15, 2023

Choose a reason for hiding this comment

sambhavgupta0705 May 15, 2023

Choose a reason for hiding this comment

derberg May 16, 2023

Choose a reason for hiding this comment

derberg May 16, 2023

Choose a reason for hiding this comment

sambhavgupta0705 May 17, 2023

Choose a reason for hiding this comment

derberg commented May 15, 2023

sambhavgupta0705 commented May 15, 2023

sambhavgupta0705 commented May 15, 2023

sambhavgupta0705 left a comment

Choose a reason for hiding this comment

sambhavgupta0705 May 15, 2023

Choose a reason for hiding this comment

derberg May 16, 2023

Choose a reason for hiding this comment

sambhavgupta0705 commented May 17, 2023 • edited Loading

derberg left a comment

Choose a reason for hiding this comment

akshatnema commented May 23, 2023

derberg commented May 23, 2023

akshatnema commented May 23, 2023 • edited Loading

derberg commented May 23, 2023

akshatnema commented May 28, 2023

akshatnema commented May 28, 2023

sambhavgupta0705 commented May 28, 2023

derberg left a comment

Choose a reason for hiding this comment

derberg commented May 29, 2023

sambhavgupta0705 commented May 14, 2023 •

edited by derberg

Loading

netlify bot commented May 14, 2023 •

edited

Loading

github-actions bot commented May 14, 2023 •

edited

Loading

sambhavgupta0705 commented May 15, 2023 •

edited

Loading

sambhavgupta0705 commented May 17, 2023 •

edited

Loading

akshatnema commented May 23, 2023 •

edited

Loading