sprockets icon indicating copy to clipboard operation
sprockets copied to clipboard

Only compile source maps in the debug pipeline

Open TylerHorth opened this issue 8 years ago • 17 comments

Currently in sprockets 4 source maps are compiled in the default pipeline alongside everything else, then source map comments are appended in the debug pipeline.

This causes a massive performance degradation when compiling assets, even when source maps aren't being used. Shopify saw compilation time go from 1m10s to 7m20s when upgrading to sprockets 4. ~~Stripping~~Hacking out source map logic from sprockets 4 returned compilation time to 60s.

cc @bouk @rafaelfranca

TylerHorth avatar Oct 25 '16 21:10 TylerHorth

Ah, well, I think a lot of us will want source maps in prod for debugging. I suppose the question becomes: opt-in or opt-out? Probably decent arguments to be made either way.

vincentwoo avatar Oct 31 '16 01:10 vincentwoo

@vincentwoo Could you explain your use case in a bit more detail? How are you using source maps without source map comments? Are you uploading them to a bug tracker?

TylerHorth avatar Nov 01 '16 13:11 TylerHorth

yup: https://docs.sentry.io/clients/javascript/sourcemaps/

vincentwoo avatar Nov 01 '16 19:11 vincentwoo

this might be a duplicate of https://github.com/rails/sprockets/issues/413. it's definitely related.

gilesbowkett avatar Nov 03 '16 02:11 gilesbowkett

I want source map in prod too (for error tracking, same as @vincentwoo) Anything we can do to achieve that quicker?

PikachuEXE avatar Nov 28 '16 06:11 PikachuEXE

Our team is also looking into generating source maps in production/staging for error reporting (via Airbrake).

For our particular case, we need to explicitly configure our error reporter with the source map locations and do not need to include source map URL comments in the minified results.

mjc-gh avatar Jan 03 '17 20:01 mjc-gh

im looking into some architectural changes that can hopefully make generating source maps optional, or rather only do them when requesting a debug asset.

Progress is slow though. I want to change how assets are loaded, the current implementation of "pipelines" is challenging to work with.

schneems avatar Jan 04 '17 03:01 schneems

As far as source maps in dev with debug mode enabled goes, everything is working fine off of master (this is with a Rails v5.0.1 prject).

For production and staging we wanted to generate source maps without including the comment in the actual minified JavaScript files. Instead, we need to just tell our error reporter (Airbrake) about the map files.

To achieve this, we created a small Compressor which wraps the Uglifier compressor and links the source maps so they are written to disk.

class UglifyWithSourceMaps < Sprockets::UglifierCompressor
  def call(input)
    result = super(input)

    map_uri = input[:environment].build_asset_uri(input[:filename], {
      type: 'application/js-sourcemap+json'
    })

    links = Set.new(input[:metadata][:links])
    links << map_uri

    result.update links: links
  end
end

Sprockets.register_compressor 'application/javascript', :uglify_with_source_maps, UglifyWithSourceMaps

In our production and staging environments, we now use this compressor.

config.assets.js_compressor = :uglify_with_source_maps

We then added another small JavaScript file to let our error handler know about these map files:

airbrake.addFilter(function(notice) {
  notice.context.sourceMaps = {
    '<%= asset_url 'editor.js' %>': '<%= asset_url 'editor.js.map' %>'
  }
});

mjc-gh avatar Jan 04 '17 18:01 mjc-gh

Not sure here, but I think you should be able to do that directly inside of your JS file

//= link editor.js.map

I think that's how I would want to support the use case of generating a map file in staging/production, but i'm not 100% on that. What do you think?

schneems avatar Jan 04 '17 18:01 schneems

I think that's fine, but I also don't particularly care about @mikeycgto's desire to not have the linking comment be present in the minified JS.

vincentwoo avatar Jan 04 '17 19:01 vincentwoo

Thanks for the advice @schneems. Using the link directive works and is a lot cleaner than using a custom compressor.

We're indifferent to the source map comment being present in minified files. It's just for our particular use case we just need explicitly tell our error reporter about the source maps and, more importantly, have a public URL to provide to the reporter.

mjc-gh avatar Jan 04 '17 20:01 mjc-gh

Glad that worked. Now I just need to figure out how to decouple this ball of mud. One asset can generate up to four seperate files. The way we do it currently is by calling load from inside of load, via different "pipelines" and processors which is quite elegant and completely impossible to work with.

Here's the four case:

  • foo.js
    • Load/Require dependencies
    • Concatenate dependencies
  • foo.js.map
    • Load foo.js
    • Currently grabs metadata[:map] from asset to build an asset, need to move that generation somewhere else to accomplish de-coupling map generation
  • foo.debug.js
    • Load foo.js
    • Load foo.js.map
    • Add comment to end of foo.js with path to foo.js.map
  • foo.source.js
    • The raw file on disk, the map file will need to point to source files.

I'm kinda stuck at the moment, going around in circles. Everything is really heavily coupled. I would like to get to the point where no load is called from within processors, but i'm not sure if that's possible. Currently the API and the caching strategies are fighting me at every step of the way. I have a branch where i'm hacking through some refactoring, no light at the end of the tunnel yet though :(

schneems avatar Jan 04 '17 20:01 schneems

Certainly appreciate the work you've done! If I can find some time, maybe I can help pitch in somehow.

mjc-gh avatar Jan 04 '17 21:01 mjc-gh

Yeah, can we pay money to make this go faster? Serious question.

vincentwoo avatar Jan 04 '17 21:01 vincentwoo

@vincentwoo I don't know re 💰 but based on @schneems saying

Now I just need to figure out how to decouple this ball of mud.

my guess is that automated tests and written documentation would both help. 😃

gilesbowkett avatar Jan 04 '17 21:01 gilesbowkett

For the $$$ question, nothing comes to mind. These problems i'm hitting up against are larger than a contractor could solve in a few hours of work (which would be hundreds/thousands of dollars). Right now major changes require a deep and broad understanding of the codebase and how things get done. If a company really wanted to invest, I would prefer they dedicated an employee for X hours a week for Y months than money.

Tyler and Bouk have both been very helpful in pushing the project forwards.

Giles is also right, docs and tests are good. Running the betas and reporting problems is good. Triaging issues, reproducing bugs, fixing reported bugs are all helpful. Money could be good if it is spent to provide some of the above things. Money on it's own is hard because then it means I would have to spend time book-keeping and managing instead of programming.

schneems avatar Jan 05 '17 18:01 schneems

The way that source maps work is they're generated from the processor. For example the coffee script processor calls coffeescript which returns a source map.

Some of these tools have the ability to turn off source maps. What I think we need to do is introduce a new config just for source maps that is auto enabled when debug is on. When we process assets we can use the new config to tell sources not to run build source maps. Then we tell sprockets to not use the DefaultSourceMap processor.

That should work, but it's a non-trivial change.

schneems avatar Nov 17 '17 16:11 schneems