Skip to content
This repository has been archived by the owner on Mar 26, 2018. It is now read-only.

After rename a sourcemap, sourceMappingURL should be updated with the new map file #62

Closed
facundocabrera opened this issue Oct 28, 2014 · 22 comments · Fixed by #79 · May be fixed by #69
Closed

After rename a sourcemap, sourceMappingURL should be updated with the new map file #62

facundocabrera opened this issue Oct 28, 2014 · 22 comments · Fixed by #79 · May be fixed by #69

Comments

@facundocabrera
Copy link

Hey all,

After run filerev with sourceMaps for JS and CSS, I found with there is none update of the sourceMappingURL pointing to the new .map file.

Current behavior:
1- a.js with sourceMappingURL=a.js.map
2- run filerev
3- a.12345678.js with sourceMappingURL=a.js.map

Expected behavior:
1- a.js with sourceMappingURL=a.js.map
2- run filerev
3- a.12345678.js with sourceMappingURL=a.12345678.js.map

Is there something complicated behind this issue?

Thanks!

@sgnl
Copy link

sgnl commented Nov 12, 2014

I am encountering this issue as well.

@johnjcorcoran
Copy link

Me too.

@nadeesha
Copy link

I actually fixed this in #67. Waiting for a merge.

@sgnl
Copy link

sgnl commented Nov 19, 2014

@ncthis nice work. Lets get this merged! @sindresorhus (not picking on you just, you're the last person who made a commit to this repo 💃)

@nuclearsuitcase
Copy link

Any suggestions on the best work around for this? I pulled down @ncthis code, and it works. Unfortunately if someone else on my team wants to build our project, their npm will pull down, the version that doesn't rename the map.

@facundocabrera
Copy link
Author

@stroebele My suggestion in this case (temporal fix easy to disable):

module.exports = function (grunt) {

  grunt.registerMultiTask('afterFilerev', 'Fix sourcemaps paths after filerev', function () {
    var files = grunt.file.expand({
      filter: 'isFile'
    }, this.data.src);

    // replace sourcemaps with revv name
    eachAsync(files, function (el, i, next) {
      var content = grunt.file.read(el);

      grunt.file.write(
        el,
        content.replace(
          /sourceMappingURL=([^\s]+)/g,
          'sourceMappingURL=' + path.basename(el) + '.map'
        )
      );

      next();
    }, this.async());
  });

};

And I just configured this task to be run after filerev.

@david-gang
Copy link

This is really annoying. I think it is best to switch back to 2.0.0

@duhseekoh
Copy link

+1

@eddiemonge
Copy link
Member

Does sourceMappingURL point to the actual map file or the source file? Are the sourcemaps gettings revisioned as well?

@sgnl
Copy link

sgnl commented Feb 13, 2015

sourceMappingURL points to the map file.

The Source-map file are rev'd with the minified .js file but the //sourceMappingURL= comment in the minified files points to the non-rev'd file name.

@eddiemonge
Copy link
Member

I kind of feel like sourcemaps should not be revisioned ... because they should not be in production code. Debugging should happen on dev code. The system that converts from dev to production should not be introducing new errors (if it does then that needs to be patched). Having source maps in production just sends unneeded extra data

@sgnl
Copy link

sgnl commented Feb 13, 2015

There are cases when you might need maps to catch unforeseen production bugs. A project I was working on needed it so services like bugsnag could work.

@eddiemonge
Copy link
Member

waiting for code updates before #67 can be merged.

@stevenleija
Copy link

Has this been merged yet?

@travi
Copy link

travi commented Mar 12, 2015

Please get this merged in soon.

Unfortunately, you can't even exclude sourcemaps from getting rev'ed because of the "feature" to ensure they are rev'ed in sync with the source files. Sourcemaps are valuable in production and are only downloaded when dev-tools are open, so there should be no reason for them not to be included.

With the current state of this plugin, sourcemaps leave dev-tools in a broken state.

@nadeesha
Copy link

Ok, let me try to get around updating #67 and see.

Edit: It just so seems that there's an #68, which improves what I've done tremendously and has tests - which can be merged. I don't know why this is not being merged.

@esperancaJS
Copy link

@david-gang going back to 2.0.0 doesn't seem to change the bahaviour

@david-gang
Copy link

@PedroEsperanca . It worked for me. I traced back when the change of the source map was made. I think it was made around 2.1, so i got back to 2

@nelsonpecora
Copy link
Contributor

Updated PR (#79) that can be merged.

@kevva kevva closed this as completed in #79 Mar 22, 2015
@travi
Copy link

travi commented Mar 23, 2015

It appears that this is not quite fully fixed.

I've pulled down the updated version by pointing npm directly at yeoman/grunt-filerev#e7ddab5849ce38e4cd5fb78db2fe8b9d623ad3c4. After installation, an npm list grunt-filerev shows [email protected] (git+https://github.com/yeoman/grunt-filerev#e7ddab5849ce38e4cd5fb78db2fe8b9d623ad3c4).

Running the task, I get the following failure message: Warning: ENOENT, no such file or directory 'c:\<path to file...>\app.css.map'. Looking in the directory, it looks like the filenames for app.css and app.css.map got rev'ed appropriately, but the sourceMapUrl in app.css failed to update. Also note that this is the first file, alphabetically, in my list of files provided to the grunt task. All other files were left untouched after the failure.

Please let me know if there is something that I missed or if there is additional information that I could provide to dig deeper into this.

@nelsonpecora
Copy link
Contributor

Ah, I think the fix was only added for js files. Let me take a look...

@nelsonpecora
Copy link
Contributor

@travi PR #80 should fix that for you, and it makes dealing with the sourceMap comments more stable overall. ✨

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