search

thgh / rollup-plugin-scss

Rollup and compile multiple .scss, .sass and .css imports
MIT License
136 stars 46 forks source link

Sourcemaps support is missing #7

Closed tk-o closed 3 years ago

tk-o commented 7 years ago

I was trying to use sourcemaps for the SCSS files (as node-sass provide them) but it seems that feature has been omitted during development of this plugin.

thgh commented 7 years ago

Does it work when you pass sourceMapEmbed as option to the scss plugin? https://github.com/sass/node-sass#sourcemapembed

I agree that when sourceMap=true is set in the rollup config, the plugin should create source maps. I don't know how source maps works precisely, so not sure if I will get it work properly. I hope to take a stab at it soon!

PR welcome!

tk-o commented 7 years ago

I had a first look on that. node-sass render method returns css binary and when required map binary as well. The problem is that only css value is being write to file. I believe I will manage to create the PR this week.

rojakcoder commented 5 years ago

@thgh sourceMapEmbed works but not for having the map as a separate file.

thgh commented 5 years ago

How about turning the sourceMapEmbed option on and split the output in code/map, would that work?

rojakcoder commented 5 years ago

When you say "split the output in code/map", do you mean writing code to separate the embedded source map?

If so, I don't think most, if not all, users would want to do that. The README says "Options are passed to node-sass", so users would expect that all the options node-sass accepts would just work.

I'm not sure what the issue is that is preventing this function from working. Is it because of the outFile parameter which states "enabling this option will not write the file on disk for you"?

thgh commented 5 years ago

I was thinking of map in return { code, map } which Rollup expects, but that's not relevant here, woops.

Just tried to embed the sourcemaps, but this seems less than ideal because the plugin is currently concatenating all scss before processing, so the actual source of the scss is not present.

Alternative 1: generate a virtual scss file that imports all imported scss files Alternative 2: process all scss files seperately like in #5 + something like combine-source-map

slikts commented 4 years ago

Is this how the option is supposed to be passed?

scss({
  sourceMapEmbed: true,
})
moritzebeling commented 3 years ago

sourceMapEmbed: true works, but is only a fallback solution during development. Unfortunately it easily doubles the css bundle size, so this is no option for production. Would be really great to have the sourcemap in a separate file, just like the sass docs suggest: https://sass-lang.com/documentation/js-api#source-maps Thank you for considering that!

astappiev commented 3 years ago

I solved the issue for myself and provided a PR #66. Feel free to test it and improve.

UPD: after reading this thread, it seems my solution will not work for everyone, in my case I have only single input file and source map. But I did exactly this.

thgh commented 3 years ago

Sourcemap support has been merged!

cwadrupldijjit commented 3 years ago

This should be documented. Only way to know about this right now is by going to this issue and then on to the associated PR.

I'd add it in another PR, but I'm currently busy and can't take too much time to do that.

cwadrupldijjit commented 3 years ago

Ah, I finally see what's happening here. It looks like this is in version 3, but that hasn't been released yet. Looking at the PR, I might temporarily throw those changes into the version I have so that I can have it. When is v3 going to be released?

thgh commented 3 years ago

Soon, I hoped to get more feedback, but let's just release it and see what comes of it.