Static site build process #802
Conversation
There was a problem hiding this comment.
Great work! Thanks for doing this :-)
In general, I think we should separate out our handlebar and localization compilation scripts from the webpack.config.js, which I mentioned in my comment below. We can invoke that script within webpack.config.js.
Also, an additional TODO's that we may want to add:
- Replicate our Django
makemessagescommand that parses our templates for new translation strings and adds them to our.pofiles.
webpack.config.js
Outdated
| var path = require('path'); | ||
| var glob = require('glob'); | ||
| var fs = require('fs'); | ||
| require("babel-polyfill") | ||
| var fs = require('fs-extra'); |
There was a problem hiding this comment.
All of theses var's should be either let or const here, as this is running in node, which has ES6 support
webpack.config.js
Outdated
| // ----------------------------------------------------------------------------- | ||
|
|
||
| var source, template; | ||
| var d = time( |
There was a problem hiding this comment.
It's a little confusing when variables are re-declared. Let's try to declare them once.
webpack.config.js
Outdated
| // Handlebars executable | ||
| var handlebarsExec = path.resolve( | ||
| __dirname, | ||
| "node_modules/handlebars/bin/handlebars" |
There was a problem hiding this comment.
You shouldn't have to reach into node_modules to access executables. Shouldn't the executable be available as handlebars?
There was a problem hiding this comment.
Also, this variable is only used once, around line 333. If we're going to use this variable, I think it makes sense to declare/set it closer to where it's being used. I think it would make the code easier to follow.
webpack.config.js
Outdated
| }); | ||
|
|
||
| // Helper for injecting precompiled templates to the index.html file | ||
| var compiledTemplatesOutputPath; |
There was a problem hiding this comment.
I'm a little confused here... this variable is already set above (line 101) - so what's the point of this declaration?
This ties into my earlier comment about declaring our variables multiple times.
webpack.config.js
Outdated
| ); | ||
|
|
||
| // Localization | ||
| var baseLocaleDir = path.resolve( |
webpack.config.js
Outdated
| __dirname, | ||
| "src/base/locale" | ||
| ); | ||
| var flavorLocaleDir = path.resolve( |
There was a problem hiding this comment.
Same issue here - this var isn't being used until around line 245. So let's not set it until we need it.
webpack.config.js
Outdated
| }; | ||
|
|
||
| // Timing | ||
| var time = function(fn) { |
There was a problem hiding this comment.
We should probably get rid of this timing function - I don't see the benefit of having it in the master branch. It's fine for taking benchmarks, but otherwise it's creating extra complexity through the use of callbacks.
Also, you can try making this an async function, and calling it with the await keyword. That should save you from using the callbacks.
webpack.config.js
Outdated
| // content into the index-xx.html file | ||
| // ----------------------------------------------------------------------------- | ||
|
|
||
| fs.readdirSync(flavorLocaleDir).forEach((langDir) => { |
There was a problem hiding this comment.
I think we should move this whole call into a separate script. The handlebars compilation can also be a separate script. It would make this webpack.config.js file easier to read and more modular. We'll need to do this anyway if we want to watch file changes and only run specific process of the static site build.
webpack.config.js
Outdated
| // Localize jstemplates | ||
| fs.readdirSync(outputJSTemplatesPath).forEach((template) => { | ||
| if (template.endsWith("html")) { | ||
| var templ = path.resolve(outputJSTemplatesPath, template); |
There was a problem hiding this comment.
Can we use a different variable name than templ here? It's kinda confusing as we already have a template variable
webpack.config.js
Outdated
| log("Finished copying jstemplates assets", d); | ||
|
|
||
| // Localize jstemplates | ||
| fs.readdirSync(outputJSTemplatesPath).forEach((template) => { |
There was a problem hiding this comment.
This is really confusing - there is already a template variable in the outer scope of this function. I suggest we should avoid shadowing it by naming our template variables to something more useful.
|
Regarding certain static assets (CSS, JS, and Handlebars templates), I was thinking if possible we should try to host the base assets in one common S3 bucket and the flavor assets on each flavor bucket. This would be easy with the CSS bundle, since we can achieve the flavor override behavior in the browser as long as we load each flavor's But if we can achieve this structure, then updates to the base project CSS, JS, or templates will only need to trigger an update to a single S3 bucket. In the current build, on the other hand, base project updates would need to trigger a re-build on every single flavor to take full effect. The situation is trickier with image assets. Because images and markers are referenced in multiple places (throughout the config and Handlebars templates), we would need to change image paths throughout several files to reference base assets stored in one place and flavor override assets stored on a given flavor bucket. As part of our deployment, then, we might want some kind of git hook that detects if a change to an image directory was made, and triggers the appropriate update of image assets across S3 flavor buckets. Just some thoughts... |
goldpbear
force-pushed
the
static-site
branch
from
3ba8993 to
b59ae17
Compare
- This commit sets up a static site build process designed to eliminate the need for the front-end Django server. The build handles localization, Handlebars precompilation, and static asset management - The following dev dependecies are introduced: - recursive-copy - js-yaml - node-gettext - gettext-parser - object-walk - handlebars - wax-on Addresses: #655
Localized versions of the index.html file are moved to locale directories and served from there.
- convert index.html templates to Handlebars - merge language_picker.html template into index.html - convert en locale code to en-us in configs - remove duplicate config keys
This is only totally working in production mode.
Localized versions of the index.html file are moved to locale directories and served from there.
- (follows on #806) - put language html files in correct filename format - fix language switching url paths - fix various static asset filepaths - remove url rewrites in webpack-dev-server (default 404 behavior catches everything) - change outputBasePath to static/css/images to be consistent with existing paths
- var -> const/let - move let/const declarations closer to where they're used - other misc cleanup
- all stored paths are to the dev api - add api_root path - paths can be overridden using env variables using existing syntax
- use xgettext and msgmerge to update existing flavor translations - generate updated po catalogs from flavor config, jstemplates, and pages files - rename existing django.po files to more generic messages.po
Add makemessages functionality to static site build
- remove an __init__.py - don't use old dist folder to store temp build files
# Conflicts: # package-lock.json
Deploy script for the static site
Addresses: #655
This might actually be ready now! :)
Add static site build process. The approach here is to create standalone
index.htmlfiles for each language for a given flavor. The finalindex.htmlfiles contain localized and serialized config data, localized and precompiled client-side Handlebars templates, and resolved flavor template inheritances.All build work is performed in
static-build.js. See that file for a heavily annotated explanation of the build process.The build process handles the following, previously performed by Django:
jstemplatesandpageslocalizationIt adds the following functionality:
jstemplatesandpagesprecompilationKnown issues with the current build:
There's a scope resolution problem with one of the Handlebars templates, where the following expression inplace-survey-form.htmlcan't be evaluated:{{get_value ../../../.. name}}. This is preventing place detail views from rendering, and needs further investigationLanguage switching is currently not implementedThe list view is busted--need to investigateSocial auth login functionality previously routed through the proxy needs to be moved wholesale to the api serverNotification email delivery needs to be ported to the api serverOther todos:
Better build process organization. Everything currently lives inwebpack.config.jsIncorporate base project .po files into localization processDeployment environment detailsWhen localizing, iterate over languages listed in the config, not over all.pofiles in thelocaledirectory. This will save significant build work for some flavors.DEBUGsetting inbase.hbsshould probably correspond to theNODE_ENVsetting./static/css/images/markers/marker-parks.png. We should refactor to something like{{STATIC_URL}}css/images/markers/marker-parks.png.po file generation/updating (previously performed with a Django utility?)font assets are not copied towwwfolder yetmove dataset urls to configs, perhaps with the option of overwriting urls with.envsettingsuse yaml-loader to watch and rebuild config files during developmentupdate docshash bundle namesNOTE: Only thepboaklandflavor has been refactored for the static site build. To test the build you'll need the following.envkeys:UPDATE: all flavors have been ported
To build the static site and start the dev server, run the following:
This will build only the
en_USlocale. To build all locales for production (or for testing a given locale), run:The build process adds dev dependencies on the following packages:
js-yaml(for converting config files to JSON)node-gettext(for localization)gettext-parser(for localization)object-walk(for parsing the config)handlebars(for replacing Django templates)wax-on(for adding template inheritance to Handlebars)fs-extra(for easy synchronous file copying and replacing)webpack-dev-server(for replacing Django's dev server)