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

Allow overriding webpack's devtool setting #6027

Merged
merged 1 commit into from Apr 9, 2018

Conversation

Projects
None yet
2 participants
@noisysocks
Member

noisysocks commented Apr 6, 2018

I've noticed that incremental builds are starting to get slow again which is a drain on (my) productivity.

Changing webpack's devtool setting from source-map to cheap-module-eval-source-map brings down the time for an incremental build on my machine from ~1500 ms to < 500 ms.

This PR allows one to override this setting with an environment variable so that folks that want to trade sourcemap quality for build speed can go ahead and do that.

I chose SOURCEMAP as the name of this variable so as to match Calypso, but it could just as easily be named DEVTOOL.

Useful reading: https://webpack.js.org/configuration/devtool/
Related PRs: #4537 #5931

How to test

Fire up your terminal and run this:

SOURCEMAP=eval npm run dev

Then, make some code changes. Notice that incremental builds are quick.

Questions

  1. Should we change the default? This page documents which options are ideal for development. I think we should move to eval-source-map since it produces source maps that are of the same quality as what we currently have, but with a faster incremental build time.
  2. Should we document this? I fear that mentioning this in CONTRIBUTING.md might be too intimidating.
Add SOURCEMAP environment variable
Allow overriding webpack's devtool confing setting with a new SOURCEMAP
environment variable.

@noisysocks noisysocks requested a review from WordPress/gutenberg-core Apr 6, 2018

@aduth

aduth approved these changes Apr 6, 2018

I'd considered this exact thing in #5931, including looking through a number of the other devtool options. As someone who frequently debugs with the sources tab, I'm very sensitive to the numerous quirks of varying options, and have found the existing option to work much better than all others.

The reason I had chosen not to introduce the option was a slight persuasive intent to encourage developers into becoming more familiar with debugging in DevTools 😄 I don't feel strongly though on this point, and if someone wants to use another option, we needn't go out of our way to prevent it. I'd be fine with these changes as-is.

Personally I'm okay if it's left undocumented. It's an advanced development workflow feature that doesn't need to be surfaced.

@noisysocks

This comment has been minimized.

Member

noisysocks commented Apr 9, 2018

👍 sounds good. Merging this as is. If we notice other developers becoming frustrated by slow incremental builds then we can re-evaluate whether the default should be changed to eval-source-map.

@noisysocks noisysocks merged commit 6a4d45b into master Apr 9, 2018

2 checks passed

codecov/project 44.51% remains the same compared to 0782f09
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details

@noisysocks noisysocks deleted the add/devtool-environment-variable branch Apr 9, 2018

@noisysocks noisysocks added this to the 2.7 milestone Apr 13, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment