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

Build process #1825

Merged
merged 33 commits into from May 9, 2018
Merged

Build process #1825

merged 33 commits into from May 9, 2018

Conversation

Sekhmet
Copy link
Contributor

@Sekhmet Sekhmet commented May 4, 2018

Fixes #1761
Fixes #1601

This should improve workflow when working on the server and make better test environment for E2E tests.

What has changed:

  • development server uses SSR by default, so there shouldn't be problem with something working in development, but turns out to be broken with SSR.
  • React components are hot reloadable (so state is preserved across changes).
  • Reduce number of scripts to only three related to building (dev for development, build for building production app, and start to deploy it).
  • SSR and client is hot reloadable - you don't have to wait for 2 minutes to test changes related to SSR.
  • Migrated to Webpack 4.
  • Migrated from ExtractTextPlugin to MiniCSSExtractPlugin.
  • Reduced development bundle from 50MB to 2MB which makes remote debugging possible.

Changes

  • Migrate to webpack 4.
  • Add dev script with hot reloading on both server and client.
  • Add build script.
  • Remove old scripts and replace them with dev, build and start.
  • Enable client-side HMR:
    • Make modules pure.
    • Enable HMR in webpack.
    • Add React HMR.
  • Update Heroku build.
  • Update Dockerfile.
  • Update docs.

Test plan

dev script

Note: styles are loaded using style-loader on development, so there is blink of not styled content after refreshing. It's slight inconvenience, but it allows styles to be hot-reloadable.

  • Checkout this branch.
  • Run yarn.
  • Run yarn dev.
  • Wait for compilation to finish.
  • Visit http://localhost:3000.
  • Regular, server-side rendered page should show up.
  • Make sure SSR works.
  • Make some change to React component (for example change beta text inside Topnav.js)
  • Text should update without full reload.
  • Make some change to styles (for example change font-size for Topnav__brand)
  • Style should update without full reload.
  • Make some change to reducer (for example change one boolean value inside of feedReducer.js).
  • Entire page should refresh.
  • Make some change to server code (eg. comment app.get('/*', ssrHandler); out in app.js).
  • Refresh the page.
  • 404 page should show up (Cannot GET /).

build and start script

  • Checkout this branch.
  • Run yarn.
  • Run yarn build.
  • Wait for compilation to finish.
  • Run yarn start.
  • Visit http://localhost:3000.
  • Regular, server-side rendered page should show up.
  • Make sure SSR works.

Compare bundle size:
This PR: https://busy-master-pr-1825.herokuapp.com/statistics.html
Staging: https://staging.busy.org/js/statistics.html
Overall bundle size decreased by small amount (few KB).

@bonustrack bonustrack temporarily deployed to busy-master-pr-1825 May 5, 2018 10:08 Inactive
@Sekhmet Sekhmet temporarily deployed to busy-master-pr-1825 May 5, 2018 10:44 Inactive
@Sekhmet Sekhmet temporarily deployed to busy-master-pr-1825 May 5, 2018 11:06 Inactive
@Sekhmet Sekhmet temporarily deployed to busy-master-pr-1825 May 5, 2018 12:53 Inactive
@Sekhmet Sekhmet temporarily deployed to busy-master-pr-1825 May 5, 2018 12:55 Inactive
@Sekhmet Sekhmet mentioned this pull request May 5, 2018
@Sekhmet Sekhmet had a problem deploying to busy-master-pr-1825 May 5, 2018 20:47 Failure
@Sekhmet Sekhmet temporarily deployed to busy-master-pr-1825 May 5, 2018 20:57 Inactive
@Sekhmet Sekhmet temporarily deployed to busy-master-pr-1825 May 5, 2018 22:25 Inactive
@Sekhmet Sekhmet temporarily deployed to busy-master-pr-1825 May 7, 2018 12:06 Inactive
@Sekhmet Sekhmet removed the PR: WIP label May 7, 2018
@Sekhmet Sekhmet requested review from jm90m and bonustrack May 7, 2018 12:44
Copy link
Contributor

@bonustrack bonustrack left a comment

Choose a reason for hiding this comment

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

Great work that simplify a lot stuff! Tested in local and everything seem to work well, SSR, hot reload, build etc

Copy link
Contributor

@jm90m jm90m left a comment

Choose a reason for hiding this comment

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

works well for me now, even when making a change, their is no more memory leak

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

3 participants