Simplify asset story #1438
Simplify asset story #1438
Conversation
|
Thanks for submitting a PR! Note that draft PR's are not reviewed. If you would like a review, please mark your pull request as ready for review in the GitHub user interface. Pull requests that are abandoned in draft may be closed due to inactivity. |
timacdonald
force-pushed
the
assets
branch
5 times, most recently
from
31bcb72
to
2d7c6b6
Compare
timacdonald
force-pushed
the
assets
branch
2 times, most recently
from
b4260c0
to
aff2f8a
Compare
| js: | ||
| tab-width: 4 | ||
| use-tabs: false | ||
| print-width: 120 | ||
| double-quotes: false | ||
| trailing-commas: es5 | ||
| semicolons: true | ||
| arrow-parens: always | ||
| bracket-same-line: false | ||
| bracket-spacing: true | ||
| finder: | ||
| exclude: | ||
| - "dist" | ||
| - "node_modules" | ||
| - "vendor" | ||
| name: | ||
| - "*.js" | ||
| - "*.jsx" | ||
| css: | ||
| tab-width: 4 | ||
| use-tabs: false | ||
| print-width: 120 | ||
| double-quotes: false | ||
| finder: | ||
| exclude: | ||
| - "dist" | ||
| - "node_modules" | ||
| - "vendor" | ||
| name: | ||
| - "*.css" | ||
| - "*.scss" | ||
| - "*.less" |
There was a problem hiding this comment.
StyleCI was having issues with the files in dist.
I've copied this config from their docs (this is what is configured with js: true or css: true and added dist to the exclude.
| "md5": "^2.3.0", | ||
| "moment": "^2.30.1", | ||
| "moment-timezone": "^0.5.45", | ||
| "phpunserialize": "^1.3.0", | ||
| "resolve-url-loader": "^5.0.0", |
There was a problem hiding this comment.
This is a webpack plugin. We don't need it anymore as we use Vite.
| import.meta.glob(['../img/**']); | ||
|
|
There was a problem hiding this comment.
This was here for the Favicon. We don't need it anymore as we inline the favicon.
| @if (! $assetsAreCurrent) | ||
| <div class="alert alert-warning"> | ||
| The published Horizon assets are not up-to-date with the installed version. To update, run:<br/><code>php artisan horizon:publish</code> | ||
| </div> | ||
| @endif | ||
|
|
There was a problem hiding this comment.
Assets are always up to date ✌️
| ], | ||
| resolve: { | ||
| alias: { | ||
| vue: "vue/dist/vue.esm.js", |
There was a problem hiding this comment.
Removed this alias and used it inline instead to make it more explicit. This was kind of hidden otherwise.
|
@driesvints, if you have a release script for this you will need to update the build command to |
|
@timacdonald our repos with asset compilation use the compile assets workflow. I've made the necessary adjustments but just FYI it's this: 8d31ff1 |
|
Thanks! |
Horizon was migrated to Vite +
laravel-vite-plugin. Thelaravel-vite-pluginis not meant for packages. There was some additional work done to support the asset URL. We ended up having a lot of workarounds to make things work.This PR rethinks Horizon's asset story completely to try and simplify things.
Main changes
Inline assets on the dashboard
CSS, JS, and the favicon are now delivered inline. The horizon dashboard is a single-page app, so there is no real-world drawbacks here IMO.
The total transfer size is ~280kb over the wire. Yes we lose browser caching of the assets, but we also had to introduce a lot of manual work to add cache busting and 280kb is nothing.
This just works™️, assets are always up to date, we no longer have to publish assets whenever we change them, and applications that use a CDN for assets are no longer having to deliver assets via their web servers - which was an issue that has been raised.
Remove
laravel-vite-pluginand usages of theVitefacade.As I mentioned, the
laravel-vite-pluginis meant for Laravel apps. It doesn't really offer anything for packages and in some ways makes things harder.We just use raw Vite now when working on Horizon.
Minor changes
/public/buildto/distConsiderations
npm run watchand it will rebuild the assets whenever they change.