gruntjs / grunt-contrib-stylus

Compile Stylus files to CSS.
http://gruntjs.com/
MIT License
174 stars 70 forks source link

added sourcemap support #121

Closed pixeldrew closed 8 years ago

vladikoff commented 9 years ago

image

With this config:

sourceMap: {
        files: {
          'tmp/sourcemap.css': 'test/fixtures/sourcemap/sourcemap.styl'
        },
        options: {
          banner: '/* \n\n\n\n\n\n\n\n\n */\n',
          paths: ['test/fixtures/'],
          sourcemap: {
            /* from: http://learnboost.github.io/stylus/docs/sourcemaps.html
             `comment`     Adds a comment with the `sourceMappingURL` to the generated CSS (default: `true`)
             `inline`      Inlines the sourcemap with full source text in base64 format (default: `false`)
             `sourceRoot`  "sourceRoot" property of the generated sourcemap
             `basePath`    Base path from which sourcemap and all sources are relative (default: `.`)
             */
            comment: 'true'
          }
        }
      }

With this test html file

<!doctype html>
<html class="no-js" lang="">
<head>
  <meta charset="utf-8">
  <meta http-equiv="X-UA-Compatible" content="IE=edge">
  <link rel="stylesheet" href="/tmp/sourcemap.css">
</head>
<body>
  <div>DIV</div>
  <h2>H2</h2>
</body>
</html>
pixeldrew commented 9 years ago

the sourceRoot needs to be appropriately set in config to point to a non relative url where the source styl files are hosted.

pixeldrew commented 9 years ago

This option looks like it is entirely project specific. The sourceUrl is a non-relative url that would map the output to the source files.

I don't see this as easily testable and should be noted in the Readme.MD

vladikoff commented 9 years ago

We have a manual sourcemap tester here https://github.com/gruntjs/grunt-contrib-concat/tree/master/test
We should one to this project as well

flying-sheep commented 9 years ago

thanks, looks fine!

i think this could be pulled and then we’d add support for more than one file later, using the same method as this tool: it is able to load and merge multiple source maps.

or we could simply depend on and use that project directly.

pixeldrew commented 9 years ago

I believe that grunt-concat-sourcemap uses the source-map npm module which I also believe only works with javascript.

flying-sheep commented 9 years ago

likely. hmm, so pull this, add CSS sourcemap support to the source-map project and then add support for merging sourcemaps?

rudolf commented 9 years ago

@pixeldrew @vladikoff I think the sourceUrl is easy enough to test. If the output css contains the expected sourceUrl then the compiler worked. That doesn't mean that the sourceUrl isn't still very project related, but if a test assumes a certain server root then we can easily test whether the compiler created the correct output for this particular project setup. Having a manually testable html file is handy, but I'd suggest we also test the css file. See https://github.com/gruntjs/grunt-contrib-stylus/pull/117/files#diff-c14fd80e059d5fa0eadb7254a8b413f0R161

In my opinion, rewriting the sourceMappingURL inside the css file is over stretching the responsibilities of what a grunt task should do. If the underlying library isn't able to produce the correct results the issue should be taken up there instead of fixing it in the grunt task itself. In my PR I solved the sourceMappingURL problem by setting the dest option, though I can't say with certainty that this would work for all project setups. https://github.com/LearnBoost/stylus/issues/1669 is further evidence that aside from grunt, the stylus source map configuration options are still finicky to get right.

sapegin commented 9 years ago

Any news here?

kylechadha commented 9 years ago

Also interested in an update here. Thanks!

argyleink commented 9 years ago

that PR looks like what I would have attempted. what can we do to help this move forward?

sindresorhus commented 9 years ago

@argyleink Submit a new PR that fixes the current issues.

Enideo commented 9 years ago

I got sourcemaps working without a pull request, see my comment at https://github.com/gruntjs/grunt-contrib-stylus/issues/118#issuecomment-98126180

pixeldrew commented 9 years ago

Sourcemaps were never a problem with inline, it has always worked. When using a path reference it doesn't work due to the way this plugin builds the stylus files.

vladikoff commented 8 years ago

Feel free to use this fork, if anyone else needs this PR please reopen a new one. Thanks!