Skip to content
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

Optimize webpack bundles #3457

Draft
wants to merge 68 commits into
base: master
Choose a base branch
from

Conversation

mwiencek
Copy link
Member

@mwiencek mwiencek commented Jan 27, 2025

Problem

We have a file, root/static/scripts/common.js, which is used as a Webpack entry point for code that gets run on every page. It bundles a ton of code which definitely doesn't need to run on every page. We have a similar one for edit pages.

The common-chunks bundle also includes too much code; we could allow more chunks to be duplicated in other bundles to reduce the size of common-chunks, and thus the amount of code that gets run on each page (at the cost of more files having to be downloaded/cached initially).

Solution

This mainly just adds additional entry points, or missing imports for the files that were listed in common.js, then removes common.js. I also did the same for edit.js. Most of this code was started last year, but I lost motivation to finish it because the React migration would have improved a lot of these issues too (by deleting a lot of the code being run). Since that's taking too long, I decided to submit it anyway (as it will still be useful in the long run).

Testing

Just browsing/using the affected pages locally.

@reosarevok
Copy link
Member

Is this a draft because you are planning to finish the edit stuff later, or because it is not ready to test/merge yet? Because I don't think we need to do it all in one go if this would be an improvement already.

@mwiencek mwiencek force-pushed the optimize-webpack-bundles branch from 304c74f to c69cd9e Compare March 29, 2025 01:45
@mwiencek mwiencek force-pushed the optimize-webpack-bundles branch from c69cd9e to 81e9f14 Compare March 29, 2025 01:52
mwiencek added 26 commits March 28, 2025 20:53
No breaking changes seem applicable to us.
This was added in 30e0316, but these events
have been supported in modern browsers for years by now. Also, the PR that
added this polyfill (metabrainz#1858) already included a workaround that didn't require
the events.
 * All JS files referencing `ko` should be importing knockout at this point.

 * I didn't find any free references to the `ko` global in .tt files either.
The inline requires were previously needed because TagEditor.js is loaded on
the server, and these libraries need a window to exist. But they've since been
mapped to empty.js on the server.
It was already being loaded in the respective $entity/index.js files in most
cases.
Also avoid loading the `CommonsImage` component entirely if disabled at the
DBDefs level -- this just leads to pointless requests and page bloat.
It was already being loaded in the respective $entity/index.js files, except
for genres, which is now fixed.
It's only used in the file it's defined in, so there's no need to expose it on
there.
AFAICT, the only use of these rating macros was in root/medium/tracklist.tt.
But the only use of *that*, in root/cdtoc/attach_list.tt, passed
`hide_ratings = 1`.
This is used for event art too, and "load" makes it clearer what it's for.
mwiencek added 29 commits March 29, 2025 12:18
It doesn't seem to be referenced anywhere.
There are no longer any references to this.
That allows us to get rid of the last reference to `MB.Control.Area` (via the
`MB` object); we can import it as `initializeArea` everywhere now.
AFAICT, this is not needed, as it doesn't export anything onto `window` or
`MB`, nor does it hydrate anything.
This should only be outputted once for the entire page, not for every release
event!
It was being overwritten with 'compound_field'.
It's currently passed by embedding some JSON within an inline script within TT.
This will allow us to get rid of those inline scripts.

I moved the primary functionality of `Form::Role::TO_JSON` into `Form::Utils`
so that it can easily be called on an individual field rather than the
top-level form only.
I don't think it's been needed here for a while. The external links editor
component is already responsible for calling it.
`MB.initializeDuplicateChecker` is not referencd anywhere.
Instead of indirectly accessing the edit/MB/edit.js exports via the `MB`
object, import the file directly.

This diff is best viewed with whitespace ignored.
This was the last utility from forms.js exposed on the `MB` object, so that
file can now be removed from edit.js.
Additionally,

 * remove it from the global `MB` object.
 * remove GuessCase.js from edit.js.
…cripts

Additionally, remove `initializeBubble` from the `MB.Control` object.
@mwiencek mwiencek force-pushed the optimize-webpack-bundles branch from 81e9f14 to dd90516 Compare March 29, 2025 17:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants