Implement raw css imports

[mgreter]

Also refactors the lookup code for better readability!

This is an attempt to fix one of our longer standing issues -> #318
I tried to read all related issues on ruby sass to make this compatible!
This PR allows to import a file.css with the following syntax:

@import "file";

This will try to resolve the partial as normal, but will also look for .css files.
Previously this would produces an error if no file could be found.

The old behaviour is still active:

@import "file.css";

Will result in:

@import url(file.css);

So this should IMO be pretty much backward compatible!
If this is not acceptable by default I suggest to make this configurable!

@hcatlin what do you think?

[HamptonMakes]

Yeah, this is one of those awkward ones where I've vocally disagreed with Ruby Sass about it, but... we have to follow their lead.

See @nex3 and @chriseppstein, we're being good! All those +1's be damned. ;)

[mgreter]

Hmm, sorry, but not sure if you mean it's ok or not!?

@chriseppstein wrote: "libsass can handle this like Sass does, right now. Additionally, there's really no issue with either Ruby sass or libsass resolving @import "foo/bar" to a css file. The only issue is when the explicit css extension is specified. We would break backwards compat."

That's how I have implemented it in this PR!?

[coveralls]

Coverage increased (+0.04%) when pulling 2635d43 on mgreter:feature/import-css into ccfcfd2 on sass:master.

[HamptonMakes]

I mean, yes, it's okay.

[mgreter]

👍

[jacobsvante]

Please please please merge this

[xzyfer]

For reference:

@import takes a filename to import. By default, it looks for a Sass file to import directly, but there are a few circumstances under which it will compile to a CSS @import rule:

If the file’s extension is .css.
If the filename begins with http://.
If the filename is a url().
If the @import has any media queries.
If none of the above conditions are met and the extension is .scss or .sass, then the named Sass or SCSS file will be imported. If there is no extension, Sass will try to find a file with that name and the .scss or .sass extension and import it.

---

Not related. Ignore.

[xzyfer]

Where is the previous discussion on this you've mentioned? I'm struggling to find documentation on how Ruby sass implements this functionality.

It's been officially roadmapped for Sass 3.4

Was stated in #318 (comment) but I see nothing related in the 3.4 changelog. Does Ruby sass have this feature without https://github.com/chriseppstein/sass-css-importer ?

[mgreter]

@xzyfer I meant this one sass/sass#556

[xzyfer]

Does that means it's in Sass at the moment? If so does this go against our road map for 3.4 compat, especially since the Ruby sass team has illuded that a new module system is on the cards for the next version. https://github.com/sass/sass/labels/%40import%20v2

My concern is that if we add this feat in this fashion it's going to become extremely difficult to deprecate, and become non-standard.

[mgreter]

It's not in sass currently IMO, but @hcatlin and IMO @chriseppstein haven spoken for adding it in this form. I'm also for it, since it does not really change the syntax significantly. It's just that we also check for .css extensions, when we also check for .scss and .sass. Which by itself is "altering" the syntax of css, btw. As I said, we can also make this optional and it's not hard to maintain (It's basically just one line).
It seems that sass will implement all these new import features with a @use statement ...

[xzyfer]

I guess my concern is not how easy it is to maintain or the size of the change, as much as it is diverging from Ruby sass. More so since Ruby sass may soon solve this in a different way. We are then stuck with supporting to two ways of doing similar things which will invariably be subtly different.

This sounds like a highly demanded feature which will mean people will depend on it. Once people are dependant on a feature removing it is a massive pain.

Generally speaking I try to have the mentality that any feature we add is essentially there forever.

My preference would be to start the discussion on with @chriseppstein and @nex3 on what imports v2 look be like and get a head start on implementing that. The demand for better imports is greater than say 3.4 query selectors (and probably less work) so it makes sense to prioritise IMHO.

Thoughts?

[mgreter]

I more or less agree, but I don't see why we would have to remove it. I agree that this has to stay in, once it is in. But it's merely having another extension to query (which we will have to support forever anyway).
This seems already pretty easy in ruby sass: sass/sass#556 (comment)

[xzyfer]

I think switching between Libsass and Ruby sass should be trivial for developers.

We should set the expectation that anything that works in Libsass works in Sass (and vice verse). This is why we're removing compact() (#585) and image-url() (#489).

Generally speaking if this were a feature Ruby sass was adamant to not implement and we wanted to it would be a different discussion. It's purely because that Ruby sass is seriously considering a solution to this that I'd encourage us to do it the one true way.

[mgreter]

I can only repeat, that this seems already very easy to do in ruby sass. I guess that's the reason why it's not enabled by default.

class CSSImporter < Sass::Importers::Filesystem
  def extensions
    super.merge('css' => :scss)
  end
end
Sass::Plugin.options[:filesystem_importer] = CSSImporter

But it's pretty hard to do in libsass when we only would need to add another extension to the lookup. But I'm OK with whatever the decision is here! IMO we could make it optional or even configurable to add additional extensions to the lookup!? //CC @hcatlin

[xzyfer]

A compromise could be outputting a depreciation warning to the console when the CSS loading branch is reached, rather than transparently passing through.

It's another discussion, but we should figure out where the line in the sand is for nonstandard (read: not in sass) features.

[HamptonMakes]

I have long voted for this behaviour and fully support it.

Just to be clear, the behaviour is that if the .css extension is not specified, but a .css file is discovered then its embedded into the Sass source tree?

[mgreter]

Yes, only @import "file" will import a file.css (or _file.css).
Something like @import "file.css" still outputs a @import statement.

[xzyfer]

🚀

[xzyfer]

This has been niggling at me. The big reason being it introduces unexpected behaviour for people moving from Ruby sass to Libsass. I think we should definitely make this behaviour optional.

I know this is not the behaviour I would want in my sass work, and I would need to opt out. I also stumbled across #469 which indicates people are @importing .css files and not expecting inlining

In addition, It seems like in compressed mode, instead of the import statement I get the code of the source file. Is this the wanted behaviour?

[mgreter]

Actually the behaviour of #469 would not change since

/*!
Theme URI: http://mywebsite.com
Description: my theme description
Author: whatwedo GmbH
Template: Meola
Version: 0.1
*/

// Parent Theme
@import url("../meola/style.css");

Will still become:

@import url("../meola/style.css");
/*!
Theme URI: http://mywebsite.com
Description: my theme description
Author: whatwedo GmbH
Template: Meola
Version: 0.1
*/

So I'm not sure where exactly your concerns are!?

[xzyfer]

My mistake I didn't notice the import was a url().

Ok I think I'm misunderstand this. I'm going show myself out now :)

[mgreter]

It's not only the url, but also the explicit .css file ending!
It only works if the .css extension is NOT specified!

@import "file"; // load partial file.(s[ac]ss|css)
@import "file.css"; // create @import on top
@import url("file"); // create @import on top
@import url("file.css"); // create @import on top

[micahblu]

@drewwells That is understood however as I stated is not the intent that the @import "path/style" will look for a partial or matching .css file and inline it into the output file?

• Note the missing .css

[xzyfer]

@manuelro you'll a recent version of Libsass or node-sass@beta installed to use this feature.

[chriseppstein]

configurable to add additional extensions to the lookup

Please, make the default behavior for libsass match the default behavior for ruby sass. As you point out, it's trivial to add this capability to sass, but it creates an inconsistency in that the behavior of having an extension is different from when it is omitted. We don't like that kind of unpredictable behavior. If people opt-in, they will understand why the behavior is what it is. Because docs.

[chriseppstein]

If you want to expose configuration that enables this, like sass does, I think that's ok.

[ccraze-matt]

@chriseppstein Does rubygem sass expose a configuration option to allow the inline importing of css files?

[electronicdreamer]

I can see this was merged but it's not really working on my side.

I'm using node-sass 4.5.3 which uses libsass 3.5.0.beta.2 and

@import "~vue-wysiwyg/dist/vueWysiwyg.css"; works fine but
@import "~vue-wysiwyg/dist/vueWysiwyg"; does not, saying

Error: File to import not found or unreadable: ~vue-wysiwyg/dist/vueWysiwyg.
Parent style sheet: E:/project/assets/scss/vendor/_vendor.scss
on line 3 of assets/scss/vendor/_vendor.scss
@import "~vue-wysiwyg/dist/vueWysiwyg";

Am I doing something wrong?

[xzyfer]

Seeing ~ suggests your using an importer or webpack loader. I suspect that's what is producing the error. I recommend strongly against rely on this behaviour. It will be removed in future versions.

…

On 19 Aug. 2017 11:33 pm, "Rex" ***@***.***> wrote: I can see this was merged but it's not really working on my side. I'm using *node-sass 4.5.3* which uses *libsass 3.5.0.beta.2* and @import "~vue-wysiwyg/dist/vueWysiwyg.css"; works fine but @import "~vue-wysiwyg/dist/vueWysiwyg"; does not, saying Error: File to import not found or unreadable: ~vue-wysiwyg/dist/vueWysiwyg. Parent style sheet: E:/project/assets/scss/vendor/_vendor.scss on line 3 of assets/scss/vendor/_vendor.scss @import <https://github.com/import> "~vue-wysiwyg/dist/vueWysiwyg"; Am I doing something wrong? — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#754 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AAjZWNy-rs7z8Tpx3KvSNlQvCp2AGm8hks5sZuQygaJpZM4DLXct> .

[electronicdreamer]

Not sure what you mean by importer but I'm just using grunt-sass -> node-sass -> libsass to compile the file.

I've tried with @import "../node_modules/vue-wysiwyg/dist/vueWysiwyg"; and I have the same problem.