-
-
Notifications
You must be signed in to change notification settings - Fork 5.5k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Move fomantic and jQuery to main webpack bundle #11997
Conversation
Overall, this seems to actually decrease webpack build time because of the split up of the CSS loaders. BTW, to get the binary size reduction, one has to manually delete |
f0dec3f
to
80d61fb
Compare
6333aa2
to
8d0cf28
Compare
But the |
The index chunk will be bigger but the downloaded JS/CSS before page is ready is the same because we depend on jquery/fomantic in a render-blocking way. Overall size might even be a bit smaller because polyfills are added to each chunk (around 30kB of JS per chunk) and we get less of that with less chunks. Essentially, <script src="index.js"></script> is the same or faster than the previous: <script src="jquery.js"></script>
<script src="semantic.min.js"></script>
<script src="index.js"></script> |
Another nice benefit here is that we avoid unnecessary CSS reflows when the stylesheets load in because they now apply all at once. |
CI failed |
Failure at |
8d0cf28
to
9d5abf7
Compare
The CSS source-map issue seems to persist even with these changes, our Webpack config seems to refuse to create them |
For what it's worth, we should most likely also provide sourcemaps for other resources for easier debugging stuff like Monaco or Service Worker. |
Maybe it's because only So far I've only had it enabled for the index chunk because source maps do blow up binary size considerably and for CSS, I'm not sure of their benefit but I guess it now gets more imporant with fomantic bundled in. |
I can't get it working even with explicit |
Serviceworker I agree but huge dependencies Like Monaco, I don't. Those would be like 20mb source map with questionable benefit. |
d19854d
to
db7f857
Compare
|
Do it for development only? Certainly it's huge but without it debugging issues with Monaco might be hard. |
I don't see sourcemap generated for CSS nor does Firefox or Chromium load one. |
It works for me:
In Chrome I see some filenames:
I guess generally we don't need to debug our dependencies and I'm not eager to slow down development build time even more (it's already quite slow, currently 21s for me, goes to 25s with all maps enabled). |
Are you sure you don't have any local changes? It refuses to create map for CSS files for me no matter what. |
Weird, It seems to create it on |
Yes, just checked in a fresh clone. Try |
Ah, apparently I get a CSS sourcemap on |
Done and still happens only on |
Apparently it's NMFR/optimize-css-assets-webpack-plugin#53 (comment). When I set this, it outputs source maps for all CSS files and completely seems to disregard the SourceMapDevToolPlugin's BTW, I removed the serviceworker map again, it's like 10 lines of our own source code and 99% workbox code and I don't see a reason to reserve another username for that. |
Fixed CSS sourcemaps by using cssnano-webpack-plugin which looks to better integrate with SourceMapDevToolPlugin. Also included is a fix that prevents unnecessary fomantic rebuilds when |
0285bad
to
269091c
Compare
Squashed and update commit title and message. |
This saves around 3 MB binary size by not including useless fomantic files in the build. Also, this allows us to move jQuery into the main bundle as well which eliminates a few HTTP requests. Also included are webpack config changes: - split less and css loaders to speed up compliation - enable css sourcemaps - switch css minfier plugin to cssnano-webpack-plugin which works better for sourcemaps than the previous plugin
269091c
to
91927c5
Compare
ping LG-TM |
The filter was wrongly excluding the gitGraph.css file. Need to clean this up later so that imports are always relative to the source file (which is not the case for fonts right now). Regressed by: go-gitea#11997
The filter was wrongly excluding the gitGraph.css file. Need to clean this up later so that imports are always relative to the source file (which is not the case for fonts right now). Regressed by: #11997 Co-authored-by: zeripath <art27@cantab.net>
This saves around 3 MB binary size by not including useless fomantic files in the build. Also, this allows us to move jQuery into the main bundle as well which eliminates a few HTTP requests. Also included are webpack config changes: - split less and css loaders to speed up compliation - enable css sourcemaps - switch css minfier plugin to cssnano-webpack-plugin which works better for sourcemaps than the previous plugin Co-authored-by: techknowlogick <techknowlogick@gitea.io>
The filter was wrongly excluding the gitGraph.css file. Need to clean this up later so that imports are always relative to the source file (which is not the case for fonts right now). Regressed by: go-gitea#11997 Co-authored-by: zeripath <art27@cantab.net>
This saves around 3 MB binary size by not including useless fomantic files in the build. Also, this allows us to move jQuery into the main bundle as well which eliminates a few HTTP requests.
Also included are webpack config changes: