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

Sourcemaps with multiple src files: #89

Closed
phadej opened this Issue Nov 2, 2013 · 29 comments

Comments

Projects
None yet
@phadej

phadej commented Nov 2, 2013

The example is a code at https://github.com/phadej/relaxed-json/tree/c3667434eb211b0f745872793723dcffe6e304d4

When running grunt less output is:

Running "less:web" (less) task
File web.min.css.map created.
File web.min.css.map created.
File web.min.css created.
Original: 7798 bytes.
Minified: 5564 bytes.

Looks like less itself doesn't support multiple input files for the source maps, but I guess it good to have this feature bug at least documented.

@rapidrapids

This comment has been minimized.

rapidrapids commented Nov 9, 2013

+1

@lukeapage

This comment has been minimized.

Member

lukeapage commented Nov 17, 2013

If you don't specify the sourcemap output filename, does grunt-contrib-less default to the input filename + ".map" ? if it did then it would get output as two different files?

@kellygrape

This comment has been minimized.

kellygrape commented Dec 6, 2013

+1

@lukeapage - if no sourcemap filename is specified, the sourcemap is not generated.

@umaar

This comment has been minimized.

umaar commented Dec 10, 2013

+1, for large projects, this would be useful!

@zamber

This comment has been minimized.

zamber commented Dec 20, 2013

The source map is generated and appended to the .css file if you don't specify a filename.

Just tested this. The linked repo is not using grunt-contrib-less but uglify.

IMO a more prominent issue is that having a index.less containing @import's defeats the pros of source maps :(.

@dan-santana

This comment has been minimized.

dan-santana commented Dec 26, 2013

+1

@tkellen

This comment has been minimized.

Member

tkellen commented Dec 26, 2013

PRs welcome everyone--I can say with pretty much 100% certainty that I won't be implementing this any time soon.

@phadej

This comment has been minimized.

phadej commented Dec 26, 2013

@zamber, the linked commit uses uglify for javascript, but grunt-contrib-less for generating css. Also with grunt I could have input files specified with a glob pattern, can't do that with index.less approach. Of course I could generate index.less from template, but that starts to be unnecessarily complicated.

@tkellen would it be enough for now to fail the task if there is sourceMapFilename option and more then one input?

@tkellen

This comment has been minimized.

Member

tkellen commented Dec 28, 2013

Sure, that seems reasonable to me.

@blittle

This comment has been minimized.

Contributor

blittle commented Mar 6, 2014

WIsh I had time to try a PR here :) This pretty much makes LESS sourcemaps useless on larger projects.

@jamesplease

This comment has been minimized.

Member

jamesplease commented Mar 6, 2014

So, wait, if I'm understanding correctly this is an issue with less and not this task?

@leolanese

This comment has been minimized.

leolanese commented Mar 17, 2014

Yep, you are right.

@GuidoJansen

This comment has been minimized.

GuidoJansen commented Jun 5, 2014

You could make different tasks for that

less: {
    dev_one: {
        options: {
            // ..
            sourceMapFilename: "css/one.css.map",
            // ..
        }
        files: {
            "css/one.css": "less/one.less",
        }
    },
    dev_two: {
        options: {
            // ..
            sourceMapFilename: "css/two.css.map",
            // ..
        }
        files: {
            "css/two.css": "less/two.less",
        }
    },
}
@dougludlow

This comment has been minimized.

dougludlow commented Jun 15, 2014

What about files that are found via dynamic expansion (http://gruntjs.com/configuring-tasks#building-the-files-object-dynamically)? For example:

module.exports = function(grunt) {
  grunt.initConfig({
    less: {
      development: {
        options: {
          compress: true,
          cleancss: true,
          optimization: 2,
          sourceMap: true
        },
        files: [{
          expand: true,
          cwd: 'client/styles/',
          src: ['**/*.less'],
          dest: 'client/styles/',
          ext: '.css'
        }]
      }
    },
    watch: {
      styles: {
        files: ['client/styles/**/*.less'], // which files to watch
        tasks: ['less'],
        options: {
          nospawn: true
        }
      }
    }
  });

  grunt.loadNpmTasks('grunt-contrib-less');
  grunt.loadNpmTasks('grunt-contrib-watch');

  grunt.registerTask('default', ['watch']);
};
@HisRoyalRedness

This comment has been minimized.

HisRoyalRedness commented Aug 6, 2014

Ditto! I too have a number of less files (20 in total), that I'd like source maps generated for. I'd like each source map to remain seperate, and carry the same name as the less file, but with a .css.map suffix (i.e. source_file_1.less => source_file_1.css.map)

Uglify handles this beautifully for compressing .js files. It would be nice if grunt-contrib-less did something similar

@rmunson

This comment has been minimized.

rmunson commented Aug 11, 2014

I may have a possible fix for this with pull request #204. But, it is certainly in need of some review as it requires a change to how the src files are processed per dest.

@metaColin

This comment has been minimized.

metaColin commented Sep 28, 2014

+1
It seems only logical that CSS rules should lead back to LESS rules form which they originate, even if those rules live in separate files. I don't edit them once concatenated so why would I want the map to lead me to their concatenated form.

@bassjobsen

This comment has been minimized.

Contributor

bassjobsen commented Nov 5, 2014

You can use grunt.file to get a list of your less files and than automatically generate your task per file, see http://stackoverflow.com/a/26758062/1596547

@kxbrand

This comment has been minimized.

kxbrand commented Nov 6, 2014

Thank you very much!‍

@gelus

This comment has been minimized.

gelus commented Jan 22, 2015

@rmunson was your fix ever merged in? What remains on this issue?

It doesn't look like your fix generates an accurate source map.
I pulled your tasks/less.js file and added it to my grunt-contrib-less module. ( and adjusted the package.json to match your branch )

given the below setup, inspecting the element in chrome will point everything to one less file or the other rather than the actual source:

  • Gruntfile.js
  • index.html
  • css/
    • compiled css file
    • compiled map file
  • less/
    • vars.less
    • demo.less

vars.less contians:

    @color: green;
    .mydiv{
       font-size: 20px; 
     }

demo.less contains:

    div.mydiv{
      background-color: darken(@color, 50%);
    }
    .mydiv{
      padding: 20px;
      border-radius: 10px;
      background-color: @color;
      color: contrast(@color);
      border: solid 3px spin(@color, 175); 
    }

and the Grunt file ( less part ):

   less: {
      dev: {
        options: {
          sourceMap: true,
          sourceMapFilename: "css/demo.css.map",
          sourceMapURL: "file:///home/bmiller/lessTest/css/demo.css.map"
        },
        files: {
          "css/demo.css":["less/**/*.less"]
        }
      }
    }
@gelus

This comment has been minimized.

gelus commented Jan 22, 2015

Okay, below is my current work around and what I'll be implementing on the project for the intermittent period.

The theory is to auto generate a "main" less file, which is simply a file containing a bunch of @import statements to all the other less files and then compile that. After compiling I clear out the "main" file.
This generates the source maps correctly and keeps me from needing to maintain a manual list of files.

it is a work around but for now it will do. If I get some time over the weekend I'll see if I can put together a PR that runs off the same theory.

My complete demo Gruntfile:

module.exports = function(grunt) {
  grunt.initConfig({
    clean: {
      removeMainLess: ["main.less"]
    },
    less: {
      dev: {
        options: {
          sourceMap: true,
          sourceMapFilename: "css/demo.css.map",
          sourceMapURL: "file:///home/bmiller/lessTest/css/demo.css.map"
        },
        files: {
          "css/demo.css": "main.less"
        }
      }
    },
    "file-creator": {
      createMainLess: {
        "main.less": function(fs, fd, done) {
          grunt.file.glob("less/**/*.less", function(err, files) {
            files.forEach(function(file, i) {
                fs.writeSync(fd, '@import "'+file+'";\n');
            });
            done();
          });
        }
      }
    }
  });

  grunt.loadNpmTasks('grunt-contrib-less');
  grunt.loadNpmTasks('grunt-contrib-clean');
  grunt.loadNpmTasks('grunt-file-creator');

  grunt.registerTask('default', ["file-creator:createMainLess", 'less:dev', "clean:removeMainLess"]);
};
@zamber

This comment has been minimized.

zamber commented Jan 22, 2015

@gelus, looks good but the trade-offs are:

  • No control over .less order (can be solved by file naming).
  • Still does not map to .less directly but to the css generated in main.css.
@gelus

This comment has been minimized.

gelus commented Jan 22, 2015

@zamber
file order was one of my thoughts too. For the work around, it is as simple as adding a sort to the grunt glob. ( or whatever other manual array manipulation you care to do )

"file-creator": {
      createMainLess: {
        "main.less": function(fs, fd, done) {
          grunt.file.glob("less/**/*.less", function(err, files) {
            files.sort(function(a,b){
              //.... what ever sort of sorting you want to implement.
            });
            files.forEach(function(file, i) {
                fs.writeSync(fd, '@import "'+file+'";\n');
            });
            done();
          });
        }
      }
    }

or better yet, since glob is deprecated, don't use it and go with expand instead. ( which, now that I look at it, I think I will do. )

    "file-creator": {
      createMainLess: {
        "main.less": function(fs, fd, done) {
          var files = grunt.file.expand(["less/vars.less","less/**/*.less"]);
          files.forEach(function(file, i) {
              fs.writeSync(fd, '@import "'+file+'";\n');
          });
          done();
        }
      }
    }

I'll bet something similar is possible with the actual implementation.

It does map to the less directly. The main file ( the one auto generated by file-creator ) should be a less file, not a css file. You then compile that main.less file and less will create a source map that will point to the original file.

Here is a screen shot of the console using the result of the above set up:

image

Thanks for the review!

@rmunson

This comment has been minimized.

rmunson commented Jan 23, 2015

@gelus, @zamber - I think this is a better solution than what I had done previously. It's the same approach I take in my current workarounds as well... either create a master less file for the project, or have the grunttfile generate one.

It is documented a bit better in issue #203.

My pull request should probably be closed at this point, as it hasn't had traction in so long. I'm glad to implement this option if neither of you want to take it on.

@gelus

This comment has been minimized.

gelus commented Jan 23, 2015

@rmunson awesome! I'm glad you like it.
I expect to have some spare time today this weekend and don't have another project right now. So I'm looking forward to implementing it.

Looking over your PR it seems this functionality needs to be place behind an option. Did you experience any other hang-ups I should watch out for?

@zamber

This comment has been minimized.

zamber commented Jan 23, 2015

@gelus, @rmunson I'm convinced. Thanks 👍.

@gelus

This comment has been minimized.

gelus commented Jan 25, 2015

Alright! PR is in. #243
it works under the same theory as the work around and will be hidden under the option combineSourceFiles. Which will allow one to share variables and mixins across files ( per destination ) and cause sourceMaps to point to the correct location.

@sindresorhus

This comment has been minimized.

Member

sindresorhus commented Feb 20, 2015

@gelus

This comment has been minimized.

gelus commented Feb 23, 2015

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