Remove unused assets and optimize loading #50
Conversation
| <!-- ***** BEGIN LICENSE BLOCK ***** | ||
| - Part of the Jellyfin project (https://jellyfin.media) | ||
| - | ||
| - All copyright belongs to the Jellyfin contributors; a full list can | ||
| - be found in the file CONTRIBUTORS.md | ||
| - | ||
| - This work is licensed under the Creative Commons Attribution-ShareAlike 4.0 International License. | ||
| - To view a copy of this license, visit http://creativecommons.org/licenses/by-sa/4.0/. | ||
| - ***** END LICENSE BLOCK ***** --> |
There was a problem hiding this comment.
Shall we remove this block in both files?
We already have it in the original jellyfin-ux repo and we might save a few kilobytes.
There was a problem hiding this comment.
Licensing stuff is not my strong side though
There was a problem hiding this comment.
I'm not sure, I think someone on the general matrix chat mentioned that we should have licensing at the start of all the files. Hopefully someone with more knowledge of licensing can help.
There was a problem hiding this comment.
Ideally, the SVG files should be optimized during build, which would remove the licensing block anyway.
I don't know the build process for this repo, but I'm guessing it's packing everything with Webpack, so using something like image-webpack-loader, which uses imagemin (the same tool we use to optimize images on jellyfin-web) should do the trick.
There was a problem hiding this comment.
@joshuaboniface Leaving and removing the blocks in production in 24ede02
There was a problem hiding this comment.
ShareAlike also mandates that any derivative is also properly licensed under the same license so it should be noted on the license/about page. Cause even the minified SVG is STILL CC-BY-SA.
There was a problem hiding this comment.
@EraYaN Adding a header at the bottom of the LICENSE.md file with a copy-paste of the license included in the SVG is enough?
Sorry, I'm not a license expert
There was a problem hiding this comment.
You can pass options to the SVGO plugin of Imagemin to keep comments. The option in question is removeComments (true by default)
There was a problem hiding this comment.
Went with removeComments=false
|
@jellyfin/web Not sure if this is the best method for optimizing images using webpack (I'm a noob with webpack and no way I get file-loader and rest of goodies to work), feel free to commit and improve this. It works™ at least :P |
|
LGTM. Let me know when the licensing stuff is figured out. @MrTimscampi you might want to take a look at this as you're better at webpack than me :p. |
webpack.config.js
Outdated
| ["svgo", { | ||
| plugins: [ | ||
| { | ||
| removeViewBox: false |
There was a problem hiding this comment.
| removeViewBox: false | |
| removeComments: false, | |
| removeViewBox: false |
There was a problem hiding this comment.
This will ensure that the license is kept.
webpack.config.js
Outdated
| const webpack = require("webpack"); | ||
|
|
||
| let config = { | ||
| context: path.resolve(__dirname, "src"), | ||
| entry: "./app.js", | ||
| output: { | ||
| filename: "bundle.js", | ||
| path: path.resolve(__dirname, "dist"), | ||
| path: path.resolve(__dirname, "dist") | ||
| }, | ||
| plugins: [ | ||
| new CleanWebpackPlugin(), | ||
| new CopyWebpackPlugin([{ |
There was a problem hiding this comment.
This should probably be replaced by proper injection at some point for cache busting. More info here: https://webpack.js.org/guides/output-management/
webpack.config.js
Outdated
| @@ -1,22 +1,38 @@ | |||
| const path = require("path"); | |||
| const CopyWebpackPlugin = require("copy-webpack-plugin"); | |||
| const { CleanWebpackPlugin } = require('clean-webpack-plugin'); | |||
| const ImageminPlugin = require("imagemin-webpack"); | |||
| const webpack = require("webpack"); | |||
|
I have rebased this over at https://github.com/hawken93/jellyfin-chromecast/tree/optimize-loading |
Remove unused assets and use SVGs for all the graphic resources used, reducing bandwidth usage and removing the artifacts that appear when the logo, for example, is being downloaded when the app is launched.