search

PauseAI / pauseai-website

Website for PauseAI.info
https://pauseai.info
Other
12 stars 39 forks source link

Exclude prerendered routes from edge function #210

Closed Reginred closed 2 months ago

Reginred commented 2 months ago

I patched the Netlify adapter to make the edge function only handle dynamic routes, which seems to pretty consistently improve performance (including some metrics that currently "need improvement" according to PageSpeed Insights), partly because it allows Netlify to handle caching. It also reduces the number of billed function calls (which is probably negligible right now, but still) and generally seems like a better way to do things.

It would be good to test the performance on the paid account and generally make sure everything works via a Deploy Preview before merging. I'll put links to the deployments I used for testing below.

Discussion on Discord: https://discordapp.com/channels/1100491867675709580/1226136907046588476/1281710876549709836

Preview on free account: https://adapter-netlify-selective--deft-starlight-2e4321.netlify.app/

PageSpeed Insights for preview on free account: https://developers.google.com/speed/pagespeed/insights/?url=https://adapter-netlify-selective--deft-starlight-2e4321.netlify.app/

Current approach on free account: https://deft-starlight-2e4321.netlify.app/

PageSpeed Insights for current approach on free account: https://developers.google.com/speed/pagespeed/insights/?url=https://deft-starlight-2e4321.netlify.app/

netlify[bot] commented 2 months ago

Deploy Preview for pauseai ready!

Name Link
Latest commit 951a37c50f2f0bd115797e349df739468c665944
Latest deploy log https://app.netlify.com/sites/pauseai/deploys/66ede7e5cc677900084acb17
Deploy Preview https://deploy-preview-210--pauseai.netlify.app
Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

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

anthonybailey commented 2 months ago

I get the intent but have no idea about Netlify details.

Is it possible to capture here (and where non-obvious, explain) how your new adapter diffs against the (presumed default) @sveltejs/adapter-netlify ?

Reginred commented 2 months ago

It calls the original build function of adapter-netlify before doing anything, so nothing is removed, but it overwrites the default pattern ^/.*$ (matches all requests) in the manifest.json with one that is composed of the patterns for the dynamic routes. This shouldn't impact performance too much if Netlify properly compiles simple regexs like this, which the tests seem to support.

Reginred commented 2 months ago

Alright, I just noticed that this breaks the 404 page. Back to the drawing board.

anthonybailey commented 2 months ago

Right, I see: this updates the .netlify/edge-functions/manifest.json file on disk. I assume the build runs on start up or deploy or similar (netlify noob here - any overview in standard docs re where builds feature in deploy/start up and what happens on the network when pages are served could be handy.) Can you copy in here the new output that gets logged by the builder?

How does that new EdgeManifest type get used?

You expect this to avoid a bunch of calls to the server, right? The two PageInsights reports look near identical so I assume they don't know about any Netlify caching upstream(?) How have you confirmed it works as intended / measured the time/calls saving?

Final dumb question: what's the approx proportion of dynamic to cacheable pages? (Ideally, both number of different routes, and scaled by page popularity i.e. across typical traffic.)

Push back if any of this is stupid or difficult: as advertised, I'm coming at this fresh. I'm asking mostly because it will interplay with pending localization plans.

Thanks!

On Fri, 20 Sept 2024, 22:48 Reginred, @.***> wrote:

It calls the original build function of adapter-netlify before doing anything, so nothing is removed, but it overrides the default pattern ^/.*$ (matches all requests) with one that is composed of the patterns for the dynamic routes. This shouldn't impact performance too much if Netlify properly compiles simple regexs like this, which the tests seem to support.

— Reply to this email directly, view it on GitHub https://github.com/PauseAI/pauseai-website/pull/210#issuecomment-2364662193, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAAXXDI4JLLAFR7T36JRSXDZXSJ3ZAVCNFSM6AAAAABOS37LMKVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDGNRUGY3DEMJZGM . You are receiving this because you commented.Message ID: @.***>