SUL23-491: Make global message a client-side component #167
Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
| const client = new GraphQLClient(process.env.NEXT_PUBLIC_DRUPAL_BASE_URL + "/graphql", { | ||
| ...requestConfig, | ||
| next: { | ||
| revalidate: 5, // revalidate every 5 seconds to catch updates as soon as possible. |
There was a problem hiding this comment.
I'm not sure that this is the right number here; there may be a better balance between BE requests and performance. I just set it to 5 sec for testing purposes.
There was a problem hiding this comment.
This definitely is not good. This will cause every single page load to fetch every bit of data from Drupal. Resulting in millions of requests and slow page loads.
There was a problem hiding this comment.
Can I narrow this down so it only gets the config page for the global message instead of everything?
There was a problem hiding this comment.
That will still increase requests for nearly every single page every user visits because it will fetch new config page data every 5 seconds.
| import {getConfigPage} from "@/lib/gql/fetcher"; | ||
| import {StanfordGlobalMessage} from "@/lib/gql/__generated__/drupal.d"; | ||
| import {JSX} from "react"; | ||
| "use client" |
There was a problem hiding this comment.
Do this on client, not the server.
There was a problem hiding this comment.
There should be no reason for a client component or fetching from the client.
|
I'm not sure this is the best solution here, but it seems more efficient than invalidating the caches for all the pages every time the message changes or gets activated/deactivated. I'm certainly open to argument if there is a better way to do this. |
|
On production, if you turn on a global message (or turn it off, or change it), those changes don't show up on the front-end unless you revalidate the build cache for all the pages. |
|
Ok, this explains more why we are over the usage on Vercel. Invalidating the entire site is very bad for performance on both the front end and the backend. We need to find better ways than doing that. Typically it's just invalidating the cache for the config pages. |
|
This PR is superceded by #168 which fixes the problem without extra overhead. |
READY FOR REVIEW
Summary
Review By (Date)
Criticality
Urgency
Review Tasks
Setup tasks and/or behavior to test
.nextdirectory out of your FE to clear the build cache.yarn run devSite Configuration Sync
Front End Validation
Backend / Functional Validation
Code
Code security
General
Affected Projects or Products
Associated Issues and/or People
@mentionthem here)Resources