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

Introduce Webpack #2230

Merged
merged 72 commits into from
Jul 30, 2019
Merged

Introduce Webpack #2230

merged 72 commits into from
Jul 30, 2019

Conversation

user512
Copy link
Contributor

@user512 user512 commented May 29, 2019

What github.com/wevote/WebApp/issues does this fix?

#757

Changes included this pull request?

Purpose

The purpose of this pull request is to refactor the build process for WebApp. WebApp currently uses gulp + browserify to build the source files. The primary product of this build process is the single bundle.js file which contains the app’s main javascript logic. The problem is that it can take over 2 minutes to generate this file. Even a small change to one file means that we must wait 2 minutes for the entire bundle.js to rebuild. With webpack, we have reduced the build time down to less than 70 seconds (initial build) & 3 seconds (hot reload) (using weback’s hot-rebuilding capabilities).

Old build process vs. new build process

The old build process (gulp) was configured in Gulpfile.js which defined a series of "tasks" to build the finished assets. The new build process (webpack) is configured in webpack.config.js. We translated each gulp task into a corresponding webpack configuration as follows:

Gulp task: "browserify" ==> Generates the bundle.js
Webpack: Generates the bundle.js automatically

Gulp task: "server" ==> Start the express server
Webpack: Webpack starts up the server automatically.

Gulp task: "compile-bootstrap" & "sass" ==> Include the bootstrap scss libraries converted into css + compile other sass files into css
Webpack: bootstrap libraries are imported directly into main.scss
a300c74 ==> This commit fixes compile bootstrap.
https://github.com/utsab/WebApp/blob/f78c25aeeccf90ce714164f8dc2abb57fee50232/src/js/index.js#L11
This line imports main.scss directly into index.js

The style-loader, css-loader, sass-loader loaders used in webpack.config.js -> import sass in js file, compile sass into css and inject a <style> tag

Gulp task: "lint-css"
Webpack: We did not configure webpack to run the css linter. There was some confusion about this one. It looks like the css linter is run as a git commit hook. The gulp lint-css watcher does not seem to do anything.

Gulp task: "clean:build" ==> clears out the build directory before rebuilding
Webpack: The CleanWebpackPlugin in webpack.config.js does the same

Gulp task: "copy-fonts" ==> move all fonts file into build/fonts
Webpack: file-loader ==> The file-loader resolves import/require() on a file into a url and emits the file into the output directory.

Gulp task: "copy-index" ==> copies the index.html from src to build
Webpack: Done by HtmlWebpackPlugin in webpack.config.js

Gulp task: "copy-css" ==> translates all the src/css/**/*.scss files into the build directory as css
Webpack: The scss files are imported directly into the relevant js files

Gulp task: "copy-img" ==> move all images into build/images
Webpack: Done by file-loader. images are now explicitly imported into the relevant js files (see example below)

Gulp task: "copy-javascript" ==> move all files in src/javascript into build/javascript
Webpack: Done by copy-webpack-plugin

New ways to import images: Import images as variables and use variable in jsx See example: 7a1ce2a

Watchify is now replaced by Webpack hot reload

How to import image going forward (with example)

7a1ce2a

Build time comparison and hot reload time (with screenshot, benchmarking)

Initial build time with gulp:

Screen Shot 2019-05-28 at 8 47 26 PM

WIth Webpack it's down to 76 seconds

Screen Shot 2019-05-28 at 8 47 21 PM

Before webpack, one line changes would take > 1 minutes to rebuild

Screen Shot 2019-05-28 at 8 47 17 PM

After webpack, one line changes would take < 5 seconds to rebuild

Screen Shot 2019-05-28 at 8 47 12 PM

In other words, after making a change, webpack hot-rebuilds in 2.4 seconds whereas the old build process with gulp would have taken 84 seconds to rebuild. This represents a 35x improvement in build time.

Known issue: broken images and solution with reason not fixing at the moment

Note: since this project is moving fast and new images are added everyday, we can’t keep up with fixing all the images.
We fixed majority of the images and would like to recommend merging this in after some testing to ensure no major problem and we can patch newly added images.

Testing Cordova build

Follow this instruction to test Cordova build
https://github.com/wevote/WeVoteCordova#install-our-code-and-the-cordova-libraries
However on the symlink part, since we no longer have fonts., css/ and javascript/, we just have to symlink index.html, bundle.js and img/.

rm bundle.js
ln -s /Users/your-username/MyProjects/WebApp/build/js/bundle.js bundle.js
rm index.html
ln -s /Users/your-username/MyProjects/WeVoteCordova/www/index.html index.html
ln -s /Users/your-username/MyProjects/WebApp/build/img img

Screen Shot 2019-05-28 at 8 47 06 PM

TO TEST

  • Test Cordova build with correct bundle.js (See Cordova Build section below)
  • Verify Google Analytic JavaScript successfully loaded (See Google Analytic section below)
    Note: We have only tested in local environment, highly suggest testing on an AWS instance for close to production behavior.

Cordova Build

The webpack development build (npm start) generates a bundle.js that contains eval syntax for the purpose of speeding up the development build process. However, Cordova security policy will reject this development build bundle.js.

We have tested that Cordova will work fine with webpack production build's bundle.js (npm run prod).

Google Analytic

The new webpack build process copy files from src/javascript to build/javascript to mimic old Gulp build process behavior. However, when working on this pull request, we can never test if this has any negative impact on Google Analytic related feature. Highly suggest testing Google Analytic to ensure this is still working as expected.

Copy link

@accesslint accesslint bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There are accessibility issues in these changes.

src/js/components/Widgets/RatingPopover.jsx Outdated Show resolved Hide resolved
src/js/components/Ballot/OfficeItemReadyToVote.jsx Outdated Show resolved Hide resolved
src/js/components/Ballot/MeasureItemReadyToVote.jsx Outdated Show resolved Hide resolved
src/js/components/Widgets/RatingPopover.jsx Outdated Show resolved Hide resolved
src/js/components/Ballot/OfficeItemReadyToVote.jsx Outdated Show resolved Hide resolved
src/js/components/Ballot/OfficeItemCompressedRaccoon.jsx Outdated Show resolved Hide resolved
src/js/components/Ballot/OfficeItemCompressedRaccoon.jsx Outdated Show resolved Hide resolved
src/js/components/Ballot/OfficeItemCompressedRaccoon.jsx Outdated Show resolved Hide resolved
src/js/components/Ballot/OfficeItemCompressedRaccoon.jsx Outdated Show resolved Hide resolved
src/js/components/Ballot/OfficeItemCompressedRaccoon.jsx Outdated Show resolved Hide resolved
@user512 user512 changed the title Webpack WIP - Introduce Webpack May 29, 2019
@DaleMcGrew
Copy link
Member

Thank you @user512 and @utsab! This is excellent work. I will discuss our review process with @SailingSteve at our next standup. I am excited to try it out myself!

@user512 user512 marked this pull request as ready for review June 5, 2019 04:18
@user512 user512 changed the title WIP - Introduce Webpack Introduce Webpack Jun 5, 2019
Copy link

@accesslint accesslint bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👏 You fixed the issue(s)! Great work.

@SailingSteve
Copy link
Member

SailingSteve commented Jul 25, 2019

Note to self, would be nice to eliminate these:

Steves-MacBook-Pro-32GB-Oct-2018:WebAppUtsab stevepodell$ npm install
npm WARN deprecated superagent@3.8.3: Please note that v5.0.1+ of superagent removes User-Agent header by default, therefore you may need to add it yourself (e.g. GitHub blocks requests without a User-Agent header).  This notice will go away with v5.0.2+ once it is released.
npm WARN deprecated gulp-util@3.0.8: gulp-util is deprecated - replace it, following the guidelines at https://medium.com/gulpjs/gulp-util-ca3b1f9f9ac5
npm WARN deprecated intl-relativeformat@2.2.0: This package has been deprecated, please see migration guide at 'https://github.com/formatjs/formatjs/tree/master/packages/intl-relativeformat#migration-guide'
npm WARN deprecated browserslist@1.7.7: Browserslist 2 could fail on reading Browserslist >3.0 config used in other tools.
npm WARN deprecated graceful-fs@3.0.11: please upgrade to graceful-fs 4 for compatibility with current and future versions of Node.js
npm WARN deprecated core-js@1.2.7: core-js@<2.6.5 is no longer maintained. Please, upgrade to core-js@3 or at least to actual version of core-js@2.
npm WARN deprecated natives@1.1.6: This module relies on Node.js's internals and will break at some point. Do not use it, and update to graceful-fs@4.x.

@SailingSteve
Copy link
Member

SailingSteve commented Jul 30, 2019

HTTPS with WebPack

Steves-MacBook-Pro-32GB-Oct-2018:WebAppUtsab stevepodell$ ./ngrok http 3000 -host-header="localhost:3000"

Works -- allows you to login with Twitter, but redirects back to localhost:3000, not to the tunnel -- that is fixable. Facebook does not immediately work.

@DaleMcGrew
Copy link
Member

Approved after extensive review by @SailingSteve. 👍

@DaleMcGrew DaleMcGrew merged commit 0754b85 into wevote:develop Jul 30, 2019
@SailingSteve
Copy link
Member

Moved post merge bugs listing to #2465

@SailingSteve SailingSteve mentioned this pull request Jul 31, 2019
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.

None yet