fix: autoreload stuff in case if frontend build is changed#269
fix: autoreload stuff in case if frontend build is changed#269
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughThe frontend build was reworked: Rollup plugins were added to auto-generate HTML and hash CSS/assets, build outputs moved/renamed, manual asset tags removed from the source HTML, nginx long-cache for /build/ was removed, and Changes
Sequence Diagram(s)sequenceDiagram
participant Dev as Developer/CI
participant Rollup as Rollup Build
participant Plugins as Plugins (cleandir, css-hash, html)
participant Out as public/ (build artifacts)
participant Nginx as nginx (runtime)
Dev->>Rollup: start build (dev/prod)
Rollup->>Plugins: run plugins during bundle
Plugins-->>Rollup: cleandir removes old public/build (generateBundle)
Rollup->>Out: emit JS chunks and `build/bundle.css`
Plugins->>Rollup: css-hash renames CSS to `build/bundle-<hash>.css` and updates bundle references
Plugins->>Out: html plugin generates `public/index.html` with injected CSS link and CSP-nonce script tag
Out->>Nginx: served at runtime with updated caching rules (no 1y /build/ expiry)
Nginx->>Browser: clients fetch hashed assets
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 inconclusive)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
|
Codecov Report✅ All modified and coverable lines are covered by tests.
Flags with carried forward coverage won't be shown. Click here to find out more.
🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Pull request overview
This PR updates the frontend build output to generate an index.html that references content-hashed JS/CSS assets, preventing stale client caches when the frontend build changes.
Changes:
- Add an
src/index.htmltemplate and generatepublic/index.htmlvia Rollup (@rollup/plugin-html) with injected hashed asset URLs. - Move Rollup output to
public/withbuild/subpaths, introduce CSS content-hashing, and cleanpublic/buildon production builds. - Update nginx caching rules and ignore generated
public/index.html.
Reviewed changes
Copilot reviewed 5 out of 6 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| frontend/src/index.html | Adds the HTML template that Rollup uses to generate the final public/index.html. |
| frontend/rollup.config.js | Generates HTML with correct asset URLs, adds hashed output naming, and cleans build artifacts. |
| frontend/public/index.html | Updated to hashed asset URLs (appears to be generated build output). |
| frontend/package.json | Adds build-time dependencies for HTML generation/cleaning. |
| frontend/package-lock.json | Locks newly added build-time dependencies. |
| frontend/nginx.conf.template | Removes redundant /build/ caching block (static asset caching still handled via extension-based rule). |
| frontend/.gitignore | Ignores public/index.html (intended to be generated). |
Files not reviewed (1)
- frontend/package-lock.json: Language not supported
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (4)
frontend/rollup.config.js (4)
220-233: LGTM! The HTML template plugin correctly injects hashed CSS and JS references with CSP nonce support.One minor observation: if
src/index.htmlis missing</head>or</body>tags, the injection silently does nothing. Consider adding a warning for easier debugging:Optional: Add warnings for missing injection points
if (cssAsset) { + if (!tmpl.includes('</head>')) { + console.warn('Warning: </head> not found in template, CSS not injected'); + } tmpl = tmpl.replace('</head>', ` <link rel="stylesheet" href="/${cssAsset.fileName}">\n</head>`); } if (entryChunk) { + if (!tmpl.includes('</body>')) { + console.warn('Warning: </body> not found in template, script not injected'); + } tmpl = tmpl.replace('</body>', ` <script nonce="**CSP_NONCE**" type="module" src="/${entryChunk.fileName}"></script>\n</body>`); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/rollup.config.js` around lines 220 - 233, The template function currently does silent no-ops if src/index.html lacks </head> or </body>; update the html plugin's template({ bundle }) to detect missing injection anchors and log warnings: after reading tmpl and computing cssAsset and entryChunk, check tmpl.includes('</head>') and tmpl.includes('</body>') and call console.warn (or use the rollup plugin logger) with clear messages referencing cssAsset.fileName and entryChunk.fileName so developers know which injection failed; keep the existing injection logic (replace) intact and only emit warnings when the corresponding tag is absent.
141-142: Minor inconsistency: chunks always use hashes, entry files only in production.In development mode,
entryFileNamesproducesbuild/[name].js(no hash) whilechunkFileNamesproducesbuild/[name]-[hash].js(with hash). This inconsistency is unlikely to cause functional issues, but you may want to align them for consistency:Optional: Consistent hashing strategy
entryFileNames: production ? 'build/[name]-[hash].js' : 'build/[name].js', - chunkFileNames: 'build/[name]-[hash].js', + chunkFileNames: production ? 'build/[name]-[hash].js' : 'build/[name].js',This would give stable chunk filenames during development, which may improve dev server caching.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/rollup.config.js` around lines 141 - 142, entryFileNames is conditional on the production flag while chunkFileNames always includes a hash, causing inconsistent filenames between entries and chunks in dev; update the rollup config so both entryFileNames and chunkFileNames use the same hashing strategy based on the production variable (e.g., make chunkFileNames conditional on production or add hashes to entryFileNames when production is false) by editing the entryFileNames and/or chunkFileNames settings in the rollup config to reference the production flag consistently.
11-11: Prefernode:protocol for Node.js built-in modules.Using the
node:prefix makes it explicit that this is a built-in module, improves readability, and follows modern Node.js conventions.Suggested fix
-import crypto from 'crypto'; +import crypto from 'node:crypto';For consistency, consider also updating the other Node.js built-ins:
-import fs from 'fs'; -import https from 'https'; -import path from 'path'; +import fs from 'node:fs'; +import https from 'node:https'; +import path from 'node:path';🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/rollup.config.js` at line 11, The import of the Node.js built-in in rollup.config.js uses the bare specifier "crypto"; update the import to use the node: protocol (e.g., replace the import statement that references 'crypto' with one that references 'node:crypto') and likewise update any other imports of Node.js built-ins in this file to their node:... equivalents to follow modern Node.js conventions and improve clarity.
207-207: Consider usingbuildStarthook for cleaning.The
generateBundlehook means old build artifacts remain on disk until new ones are about to be written. UsingbuildStartwould clean the directory at the beginning of the build, which is typically the expected behavior and ensures a truly clean state before generating new files.Suggested change
- production && cleandir('public/build', { hook: 'generateBundle' }), + production && cleandir('public/build', { hook: 'buildStart' }),However,
generateBundlemay be intentional if you want to preserve old files during watch mode rebuilds until new files are ready. If that's the case, this is fine as-is.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/rollup.config.js` at line 207, The current Rollup plugin invocation uses cleandir(..., { hook: 'generateBundle' }) which delays cleaning until generateBundle; change the hook option to 'buildStart' so the cleandir call (the production && cleandir(...) entry in rollup.config.js) runs at the start of the build and removes old artifacts before file generation; update the option value from 'generateBundle' to 'buildStart' on the cleandir call to implement this behavior (leave as-is only if you intentionally need to preserve old files during watch-mode rebuilds).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@frontend/rollup.config.js`:
- Around line 220-233: The template function currently does silent no-ops if
src/index.html lacks </head> or </body>; update the html plugin's template({
bundle }) to detect missing injection anchors and log warnings: after reading
tmpl and computing cssAsset and entryChunk, check tmpl.includes('</head>') and
tmpl.includes('</body>') and call console.warn (or use the rollup plugin logger)
with clear messages referencing cssAsset.fileName and entryChunk.fileName so
developers know which injection failed; keep the existing injection logic
(replace) intact and only emit warnings when the corresponding tag is absent.
- Around line 141-142: entryFileNames is conditional on the production flag
while chunkFileNames always includes a hash, causing inconsistent filenames
between entries and chunks in dev; update the rollup config so both
entryFileNames and chunkFileNames use the same hashing strategy based on the
production variable (e.g., make chunkFileNames conditional on production or add
hashes to entryFileNames when production is false) by editing the entryFileNames
and/or chunkFileNames settings in the rollup config to reference the production
flag consistently.
- Line 11: The import of the Node.js built-in in rollup.config.js uses the bare
specifier "crypto"; update the import to use the node: protocol (e.g., replace
the import statement that references 'crypto' with one that references
'node:crypto') and likewise update any other imports of Node.js built-ins in
this file to their node:... equivalents to follow modern Node.js conventions and
improve clarity.
- Line 207: The current Rollup plugin invocation uses cleandir(..., { hook:
'generateBundle' }) which delays cleaning until generateBundle; change the hook
option to 'buildStart' so the cleandir call (the production && cleandir(...)
entry in rollup.config.js) runs at the start of the build and removes old
artifacts before file generation; update the option value from 'generateBundle'
to 'buildStart' on the cleandir call to implement this behavior (leave as-is
only if you intentionally need to preserve old files during watch-mode
rebuilds).
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: bac4b4c3-5123-487a-9d2a-226150e641b8
⛔ Files ignored due to path filters (1)
frontend/package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (5)
frontend/.gitignorefrontend/nginx.conf.templatefrontend/package.jsonfrontend/rollup.config.jsfrontend/src/index.html
💤 Files with no reviewable changes (2)
- frontend/nginx.conf.template
- frontend/src/index.html
|
Summary by cubic
Ensure users always load the latest frontend after deploy by hashing assets and generating index.html with correct hashed references. Removes build caching and cleans old files to prevent stale bundles and enable auto-reload.
Refactors
Dependencies
Written for commit d0b485b. Summary will update on new commits.
Summary by CodeRabbit
Chores
Build & Deployment