Skip to content
This repository has been archived by the owner on Dec 23, 2017. It is now read-only.

Code splitting with factor-bundle. #327

Merged
merged 14 commits into from
Jul 23, 2015

Conversation

jmcarp
Copy link
Contributor

@jmcarp jmcarp commented Jul 14, 2015

We currently bundle all JS (vendor libraries, internal modules, and initialization code) into a single bundle. There are a few problems with this:

  • Users have to download all of our JS (including D3, DataTables, etc.) just to render the homepage.
  • We can't write scripts that are specific to a single page, so every script has to handle every page for which it might be relevant. The data tables logic is a good example: tables.js includes a large and growing switch statement keyed on attributes of the table DOM node to figure out what kind of table to draw. The code would be simpler to read and write with per-page initialization files.

This is a proof of concept using factor-bundle to write small per-page initialization files for a few pages. Check out committees.js and candidates.js, which initialize data tables for the committees and candidates pages respectively. Each file only has to initialize data tables for a single page, and we can move all the page-specific logic out of tables.js, which becomes a simpler module of reusable functions.

This isn't complete and not ready for merge, but I can get it into shape if we want to use it. Thoughts @noahmanger @msecret?

@msecret
Copy link
Contributor

msecret commented Jul 14, 2015

@jmcarp this is looking great so far. So this will be done for every page the has lots of different requirements, or all pages?

@jmcarp
Copy link
Contributor Author

jmcarp commented Jul 14, 2015

I was thinking we'd only add page-specific JS files for pages that need significant customization. So there probably wouldn't be JS files for the homepage or other pages without data tables or charts.

@msecret
Copy link
Contributor

msecret commented Jul 14, 2015

Yea I'd agree with that approach

@jmcarp
Copy link
Contributor Author

jmcarp commented Jul 22, 2015

This is now up to date with develop and seems to be working fine. I tried to structure the changes to avoid merge conflicts with other ongoing features, and I'm happy to resolve any conflicts that come up. Pinging @msecret and @noahmanger for review.

@jmcarp jmcarp changed the title [WIP] Rough sketch of code splitting with factor-bundle. Code splitting with factor-bundle. Jul 22, 2015
@msecret
Copy link
Contributor

msecret commented Jul 22, 2015

@jmcarp this looks good. Just so I'm clear, common.js is all the code not part of pages? When merge conflicts are fixed, I'm ready to merge.

@jmcarp
Copy link
Contributor Author

jmcarp commented Jul 23, 2015

@msecret: merge conflicts are fixed.

Running the updated build-js task creates n+2 script files: one for each page in static/js/pages, one for init.js, and a bundle of common requirements called common.js. The factor-bundle plugin collects all requirements that are used in more than one of the entry points and puts them into common.js, although that threshold is configurable.

@jmcarp jmcarp mentioned this pull request Jul 23, 2015
msecret pushed a commit that referenced this pull request Jul 23, 2015
Code splitting with factor-bundle.
@msecret msecret merged commit 415e50d into fecgov:develop Jul 23, 2015
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants