-
Notifications
You must be signed in to change notification settings - Fork 2
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.
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
Patterns deployment cache #1088
Conversation
@@ -3,15 +3,15 @@ FROM node:19.8.1-alpine as builder | |||
COPY . /frontend-shared | |||
RUN cd /frontend-shared && \ | |||
yarn install --frozen-lockfile && \ | |||
yarn build-pattern-lib | |||
NODE_ENV=production yarn build-pattern-lib |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We cannot set ENV NODE_ENV=production
for the whole docker stage, because then yarn install
does not install dev dependencies, which are needed for bundling.
.replace('pattern-library.css', cssBundleName); | ||
|
||
// eslint-disable-next-line no-undef | ||
fs.writeFileSync('templates/index.html', Buffer.from(indexContent)); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Here templates/index.html
has become an actual template instead of just a static file. If that is the case then I think we should make that obvious by rendering it with a template renderer when served or at build time. Assuming we're continuing to serve this as static content, then the output should go into the build/
directory rather than modifying the source file.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The dumbest possible template renderer is to use some obvious placeholder like {{ js_bundle }}
and do a string search and replace all. We might as well use something like Mustache though and at least get proper escaping.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Here templates/index.html has become an actual template instead of just a static file. If that is the case then I think we should make that obvious by rendering it with a template renderer when served or at build time. Assuming we're continuing to serve this as static content, then the output should go into the build/ directory rather than modifying the source file.
We recently renamed this file to change the mustache-specific extension to .html
because it was static. If it's a "template again," we should once again change the source file name, methinks.
Sure! #1077 is like the simplest possible approach in which we can have a reasonable solution for the caching issue. However, in this thread Rob and I discussed further options, and this is one of them. Honestly, I think we can just go with #1077, because it solves it in a good enough way that the extra effort's probably not worth it, but that's sometimes easier to see with a draft PR in front of you. |
I have realized this can be achieved by using the existing |
This PR introduces a number of changes in order to improve and more granularly control the patterns deployment cache in Cloudinary:
index.html
file should never be cached, as it contains references for the hashed bundles (alternatively we could change this and use a small cache period, like 5min)There are rollup plugins to do this automatically, but since we are bundling CSS separately, we would need to orchestrate plugins and bundlings, so this seemed like a simple solution.
Considerations
frontend-build
'sbuildCSS
function.I tested it locally, and there are a couple of possible approaches for this:
bundleCSS
function to accept an option to generate a hash or not.I prefer this option, because it basically requires adding this snippet, and a couple changes here and there:
We could also get rid of it using other building mechanisms, but for now this is the simplest option I could find.
Testing steps
TODO