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

Uglifyjs configuration for elm 0.19 #315

Merged

Conversation

andys8
Copy link
Contributor

@andys8 andys8 commented Nov 1, 2018

Description

This PR addressed the changed discussed in #314. There might be still room in terms of shaving some bytes off, but this looks fine so far. I basically did comparisons to make sure we're not moving in the wrong direction.

# UglifyJsPlugin with settings of this PR
17.280 as  1 Nov 21:50 main.516c338b.chunk.js

# Unminified results (by disabling UglifyJsPlugin)
110.583 as  1 Nov 21:54 main.516c338b.chunk.js 

# elm-minify executed on unminified results
17.289 as  1 Nov 21:55 main.516c338b.chunk.min.js

# master without DEAD_CODE_ELIMINATION
29.660 as  1 Nov 22:20 main.9e0ba63c.chunk.js

# master with DEAD_CODE_ELIMINATION
17.876 as  1 Nov 22:21 main.9e0ba63c.chunk.js

Changes

  • Removes DEAD_CODE_ELIMINATION env
  • Changed chapter in docs
  • Sourcemaps disabled by default in prod build
  • Uglify settings
    • Based on elm docs
    • 2 Passes

Testing

  • Build application and executed it
  • Ran npm run test

Solves #314

config/webpack.config.prod.js Outdated Show resolved Hide resolved
@andys8 andys8 changed the title Dead code elimination 0.19 Uglifyjs configuration for elm 0.19 Nov 1, 2018
@andys8
Copy link
Contributor Author

andys8 commented Nov 1, 2018

Appveyor build looks flakey. Ignoring it for now, because it looks unrelated.

Copy link
Owner

@halfzebra halfzebra left a comment

Choose a reason for hiding this comment

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

Good stuff! 👍

Just one little thing, I think the source-map setup should be kept in its original state. It would be better to make it configurable from the config file, but it's a task for a different PR.

config/webpack.config.prod.js Outdated Show resolved Hide resolved
@andys8
Copy link
Contributor Author

andys8 commented Nov 3, 2018

Okay, no prob. I'll change it back.

@andys8 andys8 force-pushed the feature/dead-code-elimination-0.19 branch from 8d05cdf to 6e0380b Compare November 3, 2018 23:42
Copy link
Owner

@halfzebra halfzebra left a comment

Choose a reason for hiding this comment

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

Looks good, thanks for working on this! 👍

@halfzebra halfzebra merged commit ffd259e into halfzebra:master Nov 4, 2018
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.

2 participants