search

mitodl / ocw-hugo-themes

A Hugo theme for building OCW websites
BSD 3-Clause "New" or "Revised" License
5 stars 4 forks source link

separate webpack bundles #55

Open gumaerc opened 3 years ago

gumaerc commented 3 years ago

As a user of OCW Next, I would like to have the pages I visit not load unnecessary data.

Acceptance Criteria:

Additional info

Work similar to this has already been done in ocw-course-hugo-theme to support having specific styles on Instructor Insights pages: https://github.com/mitodl/ocw-course-hugo-theme/pull/20

noisecapella commented 3 years ago

Not objecting but I'm a bit skeptical. What is the use case for the end user? The "As a ___, I'd like to ...." part at the top of the issue template is meant to describe the perspective of the person the feature would be most useful to. If the goal is to fix the search page header, we don't need to separate the CSS bundles of the home and search pages since we can just use different CSS classes and handle both conditions in the same CSS file. I think separate webpack bundles are most useful if we want to reduce code size and thus increase performance, but I don't know if we need to do that (yet) here.

gumaerc commented 3 years ago

@noisecapella Yea, that's a good point. Initially, I was thinking it would be good to take care of the two problems at once but this is really its own separate thing; still worth doing but not necessary to fix the header. I put through this PR to address the header issue, and I'll leave this issue up because I still believe we should separate the bundles for performance.

alicewriteswrongs commented 3 years ago

I think separating out the search bundle from others has pretty clear utility for reducing the assets downloaded per-page. The react app for the search UI pulls in a bunch of stuff that we're not using anywhere else. As to the stylesheets, I would be in favor of trying to lean on re-used styles across pages as much as possible, and keeping it to a single bundle if we can. Saving a little bit of download time on one page is not worth making our CSS more complex by writing a lot of per-page styles instead of creating general classes and relying on bootstrap for as much layout and styling as we can.

There is also the question that always comes up with bundle splitting, which is that if there's something big that's going to be included in both bundles then bundle splitting doesn't actually help very much in real-world scenarios, unless we also pull out a common bundle (webpack has functionality to do this, we did it in Micromasters at one point). Then we're starting to increase complexity more and more, increasing the number of requests that need to be made to load the page, etc.

But with the JS code there is a lot of stuff which is used only on the search page (i.e. react and all of the react dependencies, our own react code, etc) and some things which are used by both. Currently all of that react code is being downloaded on the homepage needlessly, so there I think there is a much more clear case for splitting out a separate search bundle.

tl/dr: I'm in favor of splitting the JS code into a general bundle loaded on all pages and another which just contains the search code but I don't think there's much benefit to splitting up the CSS.

alicewriteswrongs commented 3 years ago

back in the hugo-course-publisher days I had a branch which I didn't open a PR for that split out the search bundle. it led to a worthwhile reduction in the size of the main bundle.

pdpinch commented 3 years ago

@gumaerc is this still relevant?

gumaerc commented 3 years ago

@pdpinch Yes, although some work has already been done in this regard with the move to ocw-hugo-themes. The webpack configuration there now generates separate webpack bundles for base-theme, www and course, resulting in 3 separate JS / CSS files that are included conditionally on pages as needed. This already excludes the search code from course pages as the www bundle is not loaded there which is already a pretty big win on performance, but we could go further and isolate search into its own bundle as Alice describes and ensure that it doesn't load on the home page for an even quicker load time there.

alicewriteswrongs commented 2 years ago

As part of doing this work, I think it would be worthwhile to look into how much mileage we could get out of leveraging Hugo's built-in JS build support (https://gohugo.io/hugo-pipes/js) and whether we could get rid of Webpack if we leveraged that. esbuild (which Hugo's JS support is built on) is very very fast, low / zero configuration, and supports typescript and most modern JS build stuff that you'd like to do out of the box.