Switch to vite - #19317
Open
snipe wants to merge 3 commits into
Open
Conversation
Vulnerable Libraries (2)
More info on how to fix Vulnerable Libraries in JavaScript. 👉 Go to the dashboard for detailed results. 📥 Happy? Share your feedback with us. |
Not up to standards ⛔🔴 Issues
|
| Category | Results |
|---|---|
| UnusedCode | 2 medium |
| BestPractice | 1 medium |
| Security | 1 critical 1 medium |
🟢 Metrics 0 complexity
Metric Results Complexity 0
NEW Get contextual insights on your PRs based on Codacy's metrics, along with PR and Jira context, without leaving GitHub. Enable AI reviewer
TIP This summary will be updated as you push new changes.
Member
Author
|
The Guardrails alerts here don't have an actionable fix, but also...
So Guardrails is telling us to upgrade to versions that literally do not exist yet. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Snipe-IT's frontend build has been moved from laravel-mix (webpack) to Vite. The production build ships correctly to both open-source users and hosted customers with committed
public/build/assets, and the local iteration story isnpm run watchplus a browser refresh. The one thing Vite is famous for that we do NOT yet have working isnpm run devwith HMR. This document explains the choices we made to get here, why the codebase constraints forced most of them, and what a follow-up refactor would look like to unlock the rest of Vite's benefits.Why we're TRYING to migrate
The stack that makes this hard
Snipe-IT's frontend was built in the era when jQuery-plus-inline-scripts was the standard pattern. Every Blade template has fragments like:
Those inline scripts run during document parse, BEFORE the deferred module bundle executes. That single fact shapes almost every compromise below.
What we shipped, and why each piece exists
1. jQuery and moment are loaded as blocking classic script tags before
@viteresources/views/layouts/default.blade.phpincludes:immediately before the
@vite([...])directive. Rationale: hundreds of inline<script>$('.foo')...</script>blocks scattered through Blades expectwindow.jQueryto be defined the moment the parser reaches them. Module scripts have defer semantics, so any bundle-provided jQuery arrives too late. The files are static-copied topublic/build/vendor/on every build byvite-plugin-static-copy.2. Bundle-side imports of
jqueryandmomentroute to.cjsshim modulesvite.config.jsaliases both packages at tiny shim files:Rationale: without the shim, the bundle imports a SECOND copy of jQuery. Plugins imported into the bundle (select2, colorpicker, datetimepicker, jquery-ui) would register on that second copy, and inline
$('.thing').plugin()calls in the Blades (which use the FIRST copy onwindow) would throw "plugin is not a function".The
.cjsextension is deliberate. Rolldown treats CJS modules as eagerly evaluated on first import, somodule.exports = window.jQuerycaptures the real value at the moment the bundle initializes. An equivalent.jsfile withexport default window.jQuerycompiled into a lazily-initialized binding that stayed undefined long enough for jQuery UI's UMD wrapper to blow up during load.3. select2's UMD installer is invoked explicitly
Rationale: select2's UMD wrapper detects "browser with global jQuery" and self-installs. Under bundlers it instead detects CJS and exposes an installer function, expecting the consumer to call it. Vite/rolldown treat select2 as CJS, so
$.fn.select2never got attached until we called the installer manually.4. Two bundles, not one
resources/assets/js/app.jsandresources/assets/less/vite-main.lesscompile into the main bundle.resources/assets/js/bootstrap-table.jsandresources/assets/less/vite-bootstrap-table.lesscompile into a separate bundle.Rationale: the bootstrap-table stack is ~675KB. It's only used on pages that render a bootstrap-table. Keeping it in its own bundle keeps the weight off login, dashboard, forms, and every other non-list page. This mirrors the split that existed under mix.
5. tableExport, jsPDF, DejaVu font loader, FileSaver, and xlsx are loaded as classic script tags
These packages do
$jscomp.getGlobal(this)or reference barejQueryat module top level. Under ESM strict mode, top-levelthis === undefined, so those calls throw. Loading them as classic script tags viaresources/views/partials/bootstrap-table.blade.phpgives them the ambient browserthis === windowthey expect. They are static-copied topublic/build/vendor/on build.6. Chart.js and select2 i18n locales are also static-copied and loaded via classic script tags
Chart.js is a 350KB UMD blob used on exactly two pages (dashboard and reports/index). It is loaded directly via
<script src="build/vendor/Chart.min.js">on those two pages, bypassing@vite.select2 i18n has one JS file per locale. The default layout picks the current locale at render time and loads the appropriate file directly, since bundling every locale would waste bandwidth.
7. Output filenames are pinned; no content hashes
vite.config.jssets:Rationale: we commit
public/build/to git so open-source users and our hosted customers get a working app straight from the repo without needing Node. Content-hashed filenames would churn every rebuild and create massive commit diffs plus orphaned files in git history.Trade-off: no automatic cache-busting on user upgrades. Users may need a hard-refresh once after a version bump. Same behavior as mix's
?id=<hash>query strings if you ignore the query string entirely.8. Sourcemaps are committed
build.sourcemap: true. This keeps the browser DevTools console clean (no 404s trying to fetch missing.mapfiles). Precedent: mix committedall.js.maptoo. Cost: roughly 4MB of sourcemap files underpublic/build/assets/on every build.9.
npm run watchprovided;npm run devpresent but unfixednpm run buildproduces a full production build.npm run watchisvite build --watch, which rebuilds on save using the same code path asnpm run build. Devs iterating locally run watch in one terminal and hard-refresh the browser after each save. No dev-server timing surprises because the output is identical to production.npm run devis the true Vite dev server with HMR. It is wired up but currently throws several errors on load because dev-server module resolution doesn't match production bundling. See the next section.Why HMR doesn't work today
The Vite dev server serves each module as its own ES module over HTTP.
app.jsandbootstrap-table.jsare separate entries whose imports resolve in parallel with no guaranteed ordering between them. The production build hides this by concatenating everything into ordered bundles.Specific breakage seen in
npm run dev:$.widget(...)at module top level throws because$.widgetcomes from jquery-ui, imported inapp.js, which has not necessarily finished loading whenbootstrap-table.jsexecutes..cjsshim behaves differently under esbuild's dev-mode pre-bundle than under rolldown's production build.e.define undefinedfor the same class of timing reason.Importing binding name 'default' cannot be resolved by star export entrieson one of the imports where esbuild-dev treats a module as re-export-only.public/hotfile if the dev server exits ungracefully. Laravel's@vitereads this file and continues sending page requests to the dead dev server. Removing the file fixes it.None of these are Vite bugs. They are all consequences of asking a jQuery-plus-inline-script codebase to work in an ES-module dev environment.
What's next: unlocking the rest of Vite
The root cause of every dev-mode issue is the inline-script pattern. Every Blade fragment like:
introduces a timing dependency between document parse and the deferred bundle. The fix is to move initialization OUT of Blades and INTO the bundle, using
data-*attributes on elements to carry per-instance configuration.The pattern to migrate toward
Before:
After:
In
snipeit.js(or a dedicated init module):Bootstrap-table already uses this pattern (
<table data-toggle="table" data-url="...">). Bootstrap-table is the model, not the target.What we get once the migration is far enough along
npm run devworks reliably. Devs get true HMR with sub-100ms feedback on JS and CSS edits.<script src="build/vendor/jquery.min.js">in the layout can be deleted..cjsshims can be deleted.select2Installer(...)call can be deleted.Rough thresholds:
npm run devprobably becomes usable with occasional glitches.How to phase the migration
Do NOT tackle it as a big-bang project. Too much surface area, too much risk of regressions. Instead:
data-toggleinitializers already built out (select2, datepicker, colorpicker, etc.) so contributors can consistently reach for the pattern instead of copying inline scripts from other files.data-*attribute (rare), use a partial that populateswindow.SnipeIT = { ... }once per page and read from it in the bundle.Deployment model unchanged
git cloneor download a release zip and get working assets frompublic/build/. They do not need Node.public/build/as part of merging. No pre-commit hooks or CI automation enforce this, matching the mix-era workflow.Files touched by the migration
Added:
vite.config.jsresources/assets/js/app.js(ESM entry replacing the top of snipeit.js)resources/assets/js/bootstrap-table.js(ESM entry for the table bundle)resources/assets/less/vite-main.less(CSS entry mirroring mix's.styles(...)chain)resources/assets/less/vite-bootstrap-table.less(CSS entry for the table bundle)resources/assets/js/jquery-window-shim.cjsresources/assets/js/moment-window-shim.cjsresources/views/partials/success-sound-js.blade.php(handles browser autoplay policy for the success sound)Removed:
webpack.mix.jspublic/mix-manifest.jsonpublic/js/dist/,public/css/dist/, andpublic/css/build/Modified:
package.json(Vite devDeps in, mix devDeps out;dev/watch/build/prod/productionscripts).gitignore(removed/public/buildso built assets are tracked;/public/hotstill ignored)CLAUDE.md(stack docs updated from mix to Vite)mix('...')now uses@vite([...])app/Providers/AppServiceProvider.php(CSP nonce applied to Vite-emitted script and link tags)resources/assets/js/snipeit.js(undeclared globals fixed for ESM strict mode,$.fn.datepickerreference guarded)Test plan
npm ci && npm run buildproducespublic/build/manifest.jsonand hashed-free asset files with no errors.php artisan teststill passes.<script>and<link>emitted by@vitecarries anonceattribute matching the CSP nonce pattern used by other inline scripts.