Web preset option to enable sourcemaps in production

[edmorley]

Currently the Neutrino web preset only enables sourcemaps (specifically cheap-module-eval-source-map) in development:
https://github.com/mozilla-neutrino/neutrino-dev/blob/e52919b5ebcd694857bb7a5244afe07ca1299fd1/packages/web/index.js#L187

I imagine some people wouldn't want sourcemaps in production (eg non-open-source), however perhaps it would be worth having an option to enable for those who do? (Which would save users having to figure out which sourcemap type is best for production by reading through all of https://webpack.js.org/configuration/devtool/)

[edmorley]

So create-react-app takes the approach of generating source maps by default in production too, and then telling the user to handle them as desired during app deployment (eg restricting access via the server config or just removing them) or else they can set an option to disable:
https://github.com/facebook/create-react-app/blob/bf3d73c2c6fd78d4eea23f5356fb26f3232fabee/packages/react-scripts/config/webpack.config.prod.js#L123-L125

Should we should do this too?

[eliperelman]

Yes please!

[edmorley]

I think we should also evaluate what source map mode we should use throughout. The webpack devtools docs are quite frustrating tbh (all the options!):
https://webpack.js.org/configuration/devtool/

Reading the source makes things a little clearer:
https://github.com/webpack/webpack/blob/9f5c1b4cf934823f5c117628ed10e10ba5e84890/lib/WebpackOptionsApply.js#L218-L268

It seems that the substrings have the following meanings:

• cheap -> don't calculate column numbers (so not suitable when minifying)
• module -> use the loader's source maps (ie: show the source prior to babel etc)
• {inline,hidden,nosources} -> control where the map goes and whether to hide content (so we can ignore these)
• eval on it's own -> generated code only (fastest)
• eval when also combined with one of the other options -> ???

(See also https://survivejs.com/webpack/building/source-maps/)

Considerations when comparing:

• ability to use debugger/breakpoints without bugs (there were lots of bugs in the past - guess we'll actually have to test to avoid out of date assumptions?)
• whether it's preferable to see the babel generated code or the original (pros and cons of both)
• speed (both initial and rebuild)

For reference these are the various devtool settings used by Neutrino and others...

	development	production
webpack 4 default	eval	none
Neutrino web	cheap-module-eval-source-map	none
Neutrino react-components	source-map	source-map
Neutrino library	source-map	source-map
Neutrino node	inline-sourcemap (with custom devtoolModuleFilenameTemplate)	source-map
neutrino-preset-mozilla-frontend-infra	cheap-module-source-map	source-map (or none on Travis)
create-react-app	cheap-module-source-map	source-map
razzle	cheap-module-source-map	none
next.js	cheap-module-source-map	none
gatsby	cheap-module-source-map	source-map
nwb	cheap-module-source-map	source-map

[edmorley]

Questions:

1. Should we enable sourcemaps by default in production? (I can't decide; does slow the build down considerably, since the source-map devtool type isn't cacheable)
2. Regardless of (1), should we provide an option so people don't have to use the manual API? (I think yes)
3. If we provide an option, should it be "on/off" (with us picking what we feel is the most suitable devtool type and the user having to use the manual API otherwise), or should we allow/make the user pick which type?

To give some alternatives as to how the options could be presented to the user:

Option A...

module.exports = {
  use: [
    [
      '@neutrinojs/react',
      {
        // Always enable sourcemaps, rather than the default of only in development
        // (Neutrino picks the most appropriate source-map for each mode)
        sourcemaps: true,
      }
    ]
  ]
};

Option B...

module.exports = {
  use: [
    [
      '@neutrinojs/react',
      {
        sourcemaps: {
          // Override Neutrino's default development sourcemap of 'cheap-module-source-map'
          development: 'eval',
          // Enable sourcemaps in production too.
          // Neutrino will use 'source-map' since not overridden.
          production: true
        }
      }
    ]
  ]
};

[eliperelman]

How about both, with sourcemaps: 'auto', then object expansion mapped to modes?

Maybe we name it devtool also for consistency?

devtool: 'auto' // we pick based on environment and preset

// user manually overrides
devtool: {
  development: 'cheap-eval-source-map',
  production: 'source-map',
  none: 'eval'
}

[edmorley]

Meant to say before, for Treeherder, enabling devtool: 'source-map' in the prod build increases the warm cache build time from ~15s to ~30s.

[eliperelman]

If we do enable source-maps by default in production, we should make a note about potential security risks of outputting those source maps publicly.

[edmorley]

Agreed. Though interestingly, CRA's only mention from that point of view (given they enable by default), is this code comment:
https://github.com/facebook/create-react-app/blob/581c453610f08ef67ed467029ec289cfebe52063/packages/react-scripts/config/webpack.config.prod.js#L97

And the GENERATE_SOURCEMAP option here (which only mentions disabling to prevent OOM, rather than any other reason):
https://github.com/facebook/create-react-app/blob/master/packages/react-scripts/template/README.md#advanced-configuration

[eliperelman]

This is blocked by #972.

[edmorley]

There are still design decisions here to be made - which are the real blocker IMO.

I think perhaps we should still leave sourcemaps off in production (if only so the out of the box speed is competitive), but it would be good to add an easy option to enable them (and at the opinionated optimal setting).

[edmorley]

I propose we:

• continue to default to cheap-module-eval-source-map in development (reviewing the differences it seems like the right choice)
• continue to default to no sourcemaps in production (for out of the box speed/privacy)
• call the new preset option devtool for consistency with webpack

Still to decide:

• whether sourcemaps should be enabled during testing
  --> I think they perhaps should, given Jest etc support using them for making assertions more useful
  --> should they use cheap-module-eval-source-map too / just inherit what's set for development?
• how the new devtools option should be structured - which depends on how much configurability we want to allow before users are expected to fall back to the Neutrino config API instead?