18F / atf-eregs

Container and styles for an ATF eRegs instance
https://regulations.atf.gov/
Other
9 stars 20 forks source link

Missing CSS and JS source maps? #338

Open cmc333333 opened 8 years ago

cmc333333 commented 8 years ago

Neither the CSS nor JS appear to have a source map from their compiled form.

Some of compile_frontends output:

Running "browserify:dev" (browserify) task
>> Bundle static/regulations/js/built/regulations.js created.

Running "browserify:dist" (browserify) task
>> Bundle static/regulations/js/built/regulations.js created.

Running "uglify:dist" (uglify) task
>> 1 file created.

Running "less:dev" (less) task
File static/regulations/css/style.css.map created.
File static/regulations/css/style.css created
cmc333333 commented 8 years ago

This helps a little:

diff --git a/Gruntfile.js b/Gruntfile.js
index 2756ae7..b8ddd5c 100644
--- a/Gruntfile.js
+++ b/Gruntfile.js
@@ -96,6 +96,9 @@ module.exports = function(grunt) {
       dist: {
         files: {
           '<%= env.frontEndPath %>/js/built/regulations.min.js': ['<%= env.frontEndPath %>/js/built/regulations.js']
+        },
+        options: {
+          sourceMap: true
         }
       }
     },
jmcarp commented 8 years ago

As far as I can tell, the issue is that the patch above has uglify generate source maps from the bundled browserify output, but doesn't know about the original entry points that browserify saw. One quick fix would be to use the terribly-named minifyify browserify plugin, like so:

options: {
  ...
  plugin: [
    [function(b) {
      b.plugin('minifyify', {
        output: env.frontEndPath + '/js/built/regulations.foo.map.js'
      });
    }]
  ],
  ...

The point of minifyify is to solve exactly this problem: bundling and minifying while generating sensible source maps. We should also be able to use uglifyify, which is pretty similar but bundles the source map into the minified script file by default, which means we'd also have to use something like exorcist to extract the source maps to a new file in production.

jmcarp commented 8 years ago

I put together a quick working version in https://github.com/jmcarp/regulations-site/commit/a3d6aed1aa8cba7e5c9c45daf2d8f920f27d8a68. If this looks like a reasonable approach, I'll send a PR--lmk @cmc333333.

cmc333333 commented 8 years ago

Makes sense to me in the abstract, but I don't know the space well enough. Make the PR and have @xtine or @jbarnicle confirm? The only bit that troubles me is the debugging mode on in the "dist" instead of "dev".