More stable sourcemap filerevving

[nelsonpecora]

• Adds support for multi-line sourcemap comments (as in CSS, addresses travi's issue here)
• Ignores inline base64-encoded sourcemaps
• Includes tests to prove both of the above

[kevva]

Remove this empty line.

[kevva]

Fix my inline comments and squash your commits and this should be good to go.

[travi]

Thanks for looking into this so quickly. It is now updating the sourceMappingURL in the css file, but I'm still getting the Warning: ENOENT, no such file or directory 'c:\<path to file...>\app.css.map' error, which prevents it from continuing to the other files. Any idea what might be causing that part?

[nelsonpecora]

Fixed spaces and squashed commits.

[nelsonpecora]

@travi that might be happening if the original sourcemap comment is pointing to a .map file with forward-slashes, but you're using backslashes (on windows)? I thought grunt.file.read abstracted away that, though...

EDIT: It might be an assumption in the regex used, or it might be a bug with grunt.file.read (or one of its dependencies). I don't really have a windows environment to test this on, but what happens if you pull down my PR and run npm test? Does it pass?

[travi]

I just tried running the same build on OSX and got the same error.

• The file globbing pattern that is used in the grunt config is simply public/**/*.*
• The source map is in the same directory as the css file, so the comment is simply the filename (no relative pathing necessary)
• the relative path in the source map to point to the scss file is ../../assets/sass/app.scss

EDIT: And yes, npm test passes on your fork in both windows and OSX.

[nelsonpecora]

Can you paste the results of npm test? I'm not quite sure what's going on. This doesn't (currently) support sourcemaps for anything besides .js and .css files.

[travi]

Is this the output you mean?

Running "jshint:all" (jshint) task
>> 6 files lint free.

Running "jscs:all" (jscs) task
>> 6 files without code style errors.

Running "clean:test" (clean) task
>> 0 paths cleaned.

Running "copy:test" (copy) task
Copied 3 files

Running "filerev:compile" (filerev) task
Revved 1 file

Running "filerev:withConfig" (filerev) task
Revved 1 file

Running "filerev:withDest" (filerev) task
Revved 1 file

Running "filerev:withExpand" (filerev) task
Revved 1 file
Revved 1 file
Revved 1 file
Revved 1 file
Revved 1 file
Revved 1 file
Revved 1 file
Revved 1 file
Revved 1 file
Revved 1 file
Revved 1 file

Running "filerev:withFilenameProcessing" (filerev) task
Revved 1 file

Running "filerev:withSummaryAttributeName" (filerev) task
Revved 2 files

Running "filerev:withSourceMaps" (filerev) task
Revved 1 file
Revved 1 file
Revved 1 file
Revved 1 file
Revved 1 file

Running "checkSummary" task

Running "simplemocha:test" (simplemocha) task


  ✓ should revision files based on content 
  ✓ should accept options 
  ✓ should allow a dest directory option 
  ✓ should allow sources defined with expand 
  ✓ should use same revision as .js source for the .map 
  ✓ should point the .js sourceMappingURL to the revisioned .map 
  ✓ should revision .js file ok without any .map 
  ✓ should use same revision as .css source for the .map 
  ✓ should point the .css sourceMappingURL to the revisioned .map 
  ✓ should revision .css file ok without any .map 
  ✓ should ignore inline base64-encoded sourcemaps 
  ✓ should allow a filename processing function 

  12 passing (6ms)


Running "clean:test" (clean) task
>> 1 path cleaned.

[nelsonpecora]

Oh! Sorry, I didn't see your edit. Hmm. Are you trying to use this on scss files, or just css files?

[travi]

I'm rev-ing the css files that are built from scss. The Sass compile step happens before the rev, and the sourcemap mentioned in the css file points to the scss file that is the source of the css file in question. Plus, everything works fine if I don't have the Sass compilation generate source maps (and, alternatively, the sourcemaps work properly in dev-tools if I skip the rev step).

[nelsonpecora]

Ahh, hmm. That's probably it. This doesn't currently have a good way to handle sourcemaps that are pointing at a different filetype. Let me think about how to handle this.

[travi]

I'm not sure I follow. The sourcemap mentioned in app.css is app.css.map. These are the only two files that get rev-ed. Only app.css.map references anything to do with scss files. As long as the comment in app.<rev>.css points to app.<rev>.css.map, it shouldn't matter beyond that, correct?

[travi]

I haven't confirmed, but I'll bet its the fact that our file pattern does not specify an extension (public/**/*.*). We did this because, before trying to ad sourcemaps, all files under public needed to be rev-ed. Since the .css.map files are being added to the list, I'm betting that the rev task is trying to rev them after they've already been rev-ed.

I could add an exclusion in the Gruntfile, but it may be cleaner for the task to handle this automatically. Thoughts?

[nelsonpecora]

Hmm. Are you revving in place?

[travi]

We are revving in place. I came into this project late and that setup was already in place, so I'm not saying its the best setup, but I do think its valid.

I've confirmed that adding a !**/*.map rule to the file match pattern fixes my issue. Therefore, I would say it is not related directly to what this pull request intends to fix. I'm also able to move forward with the addition of this exclusion rule, so I'd love to see this get merged into a release soon as is.

EDIT: I've added #81 to track the double processing of sourcemaps separately as I think it would be valuable to handle, but don't want it to block getting this fix merged.

[nelsonpecora]

Cool, thanks! Yeah, I'm inclined to agree that grunt-filerev should handle this situation, but I'm not sure what the best approach would be.

[nelsonpecora]

@kevva is this PR good to merge?

[kevva]

Super nice, thanks! :)