✨ Add post-build vendor safety check to prevent Vite define corruption#4252
✨ Add post-build vendor safety check to prevent Vite define corruption#4252clubanderson merged 1 commit intomainfrom
Conversation
|
/lgtm |
✅ Deploy Preview for kubestellarconsole ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
|
@clubanderson: you cannot LGTM your own PR. DetailsIn response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. |
|
👋 Hey @clubanderson — thanks for opening this PR!
This is an automated message. |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: clubanderson The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
There was a problem hiding this comment.
Pull request overview
Adds a post-build guardrail in the web/ frontend build pipeline to detect Vite define-based corruption of dependency bundles (the “undefined(...) / void 0(” production crash class seen in #4248), ensuring the build fails before deployment if corrupted vendor chunks are produced.
Changes:
- Add
postbuildscript that scans built assets for thevoid 0(pattern indicatingundefined(...)calls in minified output. - Wire the safety check into
npm run buildviapackage.jsonpostbuild. - Minor cleanup in
Layout.tsximport list.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| web/src/components/layout/Layout.tsx | Removes an unused UI constant import. |
| web/scripts/check-vendor-safety.mjs | New post-build script to detect vendor bundle corruption patterns. |
| web/package.json | Adds postbuild hook to run the new safety check after builds. |
| import { join } from 'node:path' | ||
|
|
||
| const ASSETS_DIR = join(import.meta.dirname, '..', 'dist', 'assets') |
There was a problem hiding this comment.
import.meta.dirname is not a Node-standard ESM API and can be undefined depending on the Node version/flags. Since this runs as part of postbuild, a missing import.meta.dirname would crash the build before the check runs. Prefer deriving the script directory from import.meta.url (via fileURLToPath) or resolve dist/assets from process.cwd() to keep this compatible with the Node versions used in CI.
| import { join } from 'node:path' | |
| const ASSETS_DIR = join(import.meta.dirname, '..', 'dist', 'assets') | |
| import { join, dirname } from 'node:path' | |
| import { fileURLToPath } from 'node:url' | |
| const __dirname = dirname(fileURLToPath(import.meta.url)) | |
| const ASSETS_DIR = join(__dirname, '..', 'dist', 'assets') |
| const vendorFiles = readdirSync(ASSETS_DIR).filter( | ||
| (name) => name.startsWith('vendor-') && name.endsWith('.js'), | ||
| ) |
There was a problem hiding this comment.
The file filter only matches vendor-*.js, but the Vite config splits dependencies into multiple *-vendor chunks (e.g. react-vendor, ui-vendor, etc.). If define corruption happens in those chunks, this check will miss it and the build will still succeed. Consider scanning all *-vendor-*.js chunks (and/or all .js files in dist/assets) rather than only the vendor- prefix.
🔄 Auto-Applying Copilot Code ReviewCopilot code review found 1 code suggestion(s) and 1 general comment(s). @copilot Please apply all of the following code review suggestions:
Also address these general comments:
Push all fixes in a single commit. Run Auto-generated by copilot-review-apply workflow. |
Applied both fixes in e2523e5:
Build and lint verified passing. |
Adds a postbuild script that scans vendor bundles for `void 0(` — the minified form of `undefined()` — which indicates Vite's define config replaced `console.*` calls in dependency code. This would have caught the production crash from PR #4239 / #4248 before deployment. Also removes unused DEV_BAR_HEIGHT_PX import blocking tsc. Signed-off-by: Andrew Anderson <andy@clubanderson.com>
e2523e5 to
ef876db
Compare
|
Thank you for your contribution! Your PR has been merged. Check out what's new:
Stay connected: Slack #kubestellar-dev | Multi-Cluster Survey |
📝 Summary of Changes
Adds a
postbuildscript (scripts/check-vendor-safety.mjs) that runs after everynpm run build. It scans all vendor-related JS bundles indist/assets/forvoid 0(— the minified form ofundefined()— which indicates Vite'sdefineconfig replacedconsole.*calls in vendor/dependency code.This would have caught the production crash from #4248 (caused by #4239) before the build ever deployed.
How it works
If vendor corruption is detected, the build fails with:
Changes Made
web/scripts/check-vendor-safety.mjs— post-build script that scans vendor bundles forvoid 0(corruption patternpostbuildhook inweb/package.jsonto run the safety check after every buildweb/src/components/layout/Layout.tsximport.meta.dirname→dirname(fileURLToPath(import.meta.url))for broad Node.js ESM compatibilityvendor-*.jsprefix toname.includes('vendor'), covering all Vite manual chunk names (react-vendor,ui-vendor,charts-vendor,vendor, etc.)Checklist
Please ensure the following before submitting your PR:
git commit -s)Screenshots or Logs (if applicable)
👀 Reviewer Notes
name.includes('vendor')to catch all Vite chunk naming patterns (vendor-,react-vendor-,ui-vendor-,charts-vendor-, etc.) rather than only thevendor-prefix.dirname(fileURLToPath(import.meta.url))instead ofimport.meta.dirnamefor compatibility with Node.js versions used in CI (v12.11+).