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

Handle input files that have source maps #123

Open
buschtoens opened this issue Jul 5, 2018 · 8 comments
Open

Handle input files that have source maps #123

buschtoens opened this issue Jul 5, 2018 · 8 comments

Comments

@buschtoens
Copy link

I'm not 100 % sure on the claims I make here, so please correct me, if I'm wrong.

It appears that input files that already have source maps attached to them are just passed through transparently, including the sourceMappingURL comment. This means that you will end up with a concatenated output file, that has one valid sourceMappingURL comment—the one generated by broccoli-concat—and possibly multiple invalid sourceMappingURL comments somewhere in the file, that reference source maps that are not part of the output tree anymore.

Adding a file to vendor.js via EmberApp#import() under the hood eventually calls this plugin and I actually tripped over this in ember-cli/ember-fetch#119. ember-fetch also uses broccoli-concat to merge two files and since a source map is generated by default, the resulting concatenated file has a sourceMappingURL comment, which then gets passed through to the vendor.js file, where it eventually wreaks havoc in broccoli-uglify-sourcemap/lib/process-file.js#L31-L39.

@akashdsouza
Copy link
Contributor

I haven't tested this yet. But, it looks like the comment is being removed here

@buschtoens
Copy link
Author

buschtoens commented Jul 5, 2018

Hm, yeah. Seems like this is supposed to remove that commend.

I've set up a demo repo here: https://github.com/buschtoens/ember-fetch-sourcemap-bug

When I can find the time, I'll sprinkle in some broccoli-debugs.

@xg-wang
Copy link

xg-wang commented Jul 6, 2018

Seems removeFrom in source-map-url doesn't have a global flag in the RegExp so additional comment wouldn't be removed.

@buschtoens
Copy link
Author

But addFileSource is run for each file that is being included, right? So as long as each included file only has one comment, it should work.

@xg-wang
Copy link

xg-wang commented Jul 9, 2018

So I did some debugging, seems the comment is at the end of a single line:

// ember-fetch.js
...t.exports=r(7)}])});//# sourceMappingURL=ember-fetch.map

From 0.3 to 0.4 source-map-url has fixed this case.

The dependency from ember-cli-uglify if the correct v0.4 so my yarn why source-map-url was pointing to 0.4. However, fast-sourcemap-concat is using v0.3 and causing broccoli-concat to fail this case.


Then in the tmp/source_map_concat-input_base_path-oTFtyRCF.tmp/vender we have ember-fetch.js and ember-fetch.map.
The ember-fetch.map is broken, basically it's like:

if (typeof FastBoot === 'undefined') { (function (global) {
  define('fetch', ['exports'], function(self) {
    'use strict';
    var Promise = global.Ember.RSVP.Promise;
    var window = self;
    if (global.FormData) {
      self.FormData = global.FormData;
    }
    if (global.FileReader) {
      self.FileReader = global.FileReader;
    }
    if (global.Blob) {
      self.Blob = global.Blob;
    }
    if (global.ArrayBuffer) {
      self.ArrayBuffer = global.ArrayBuffer;
    }
    if (global.Symbol) {
      self.Symbol = global.Symbol;
    }
    if (global.URLSearchParams) {
      self.URLSearchParams = global.URLSearchParams;
    }
// actual source-map
    var pending = 0;
    function decrement(result) {
      pending--;
      return result;
    }

    if (global.Ember.Test) {
      global.Ember.Test.registerWaiter(function() {
        return pending === 0;
      });

      self['default'] = function() {
        pending++;

        return self.fetch.apply(self, arguments).then(function(response){
          response.clone().blob().then(decrement, decrement);
          return response;
        }, function(reason) {
          decrement(reason);
          throw reason;
        });
      };
    } else {
      self['default'] = self.fetch;
    }
  });

  define('fetch/ajax', ['exports'], function() {
    throw new Error('You included `fetch/ajax` but it was renamed to `ember-fetch/ajax`');
  });
}(typeof window !== 'undefined' ? window : typeof global !== 'undefined' ? global : typeof self !== 'undefined' ? self : this));
 }

So the source-map is also being templated and mapped just as the actual file.


What to do:

  1. Update fast-sourcemap-concat dependency.
  2. Either merge fix(build): disable source map generation for concatenation ember-cli/ember-fetch#120 or we find a way to source-map correctly in ember-fetch

@stefanpenner
Copy link
Collaborator

stefanpenner commented Jul 10, 2018

Update fast-sourcemap-concat dependency.

released as v3.3.1

Either merge ember-cli/ember-fetch#120 or we find a way to source-map correctly in ember-fetch

released as v5.0.1

@kmoe
Copy link

kmoe commented Aug 13, 2018

fast-sourcemap-concat has now downgraded source-map-url as a dependency - it is now using the old 0.3 version. ef4/fast-sourcemap-concat#54

Versions of broccoli-concat using fast-sourcemap-concat v1.3.2 and above will suffer a regression of this issue.

@stefanpenner
Copy link
Collaborator

@kmoe thanks! I'll try to investigate.

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

No branches or pull requests

5 participants