Skip to content
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

Allow @import within @if #451

Closed
AriaMinaei opened this issue Jul 18, 2012 · 14 comments
Closed

Allow @import within @if #451

AriaMinaei opened this issue Jul 18, 2012 · 14 comments

Comments

@AriaMinaei
Copy link

In one of the recent versions, the ability to have @import within @if has been deprecated. I looked through the changelog. Couldn't find anything mentioning that change,

Anyway, @import within @if gives the dev a lot of flexibility.

Example:

// File: latin.scss
$lang: 'latin';
@imoprt "./master";

// File: persian.scss
#lang: 'persian';
@import "./master";

// File: _master.scss
@if $lang == 'persian' {
   @impot "fonts/arabic";
}

Explanation:
Simply, it imports some arabic fonts only in persian.css.
I actually use this feature in almost all of my designs, since they are all multi-language, multi-directional designs. Deprecating it simply breaks all of those styles.

I will be downgrading to an earlier version for now.

@AriaMinaei
Copy link
Author

3.1.7 works fine, change came in 3.1.8. It's not documented in the changelog.

@chriseppstein
Copy link

Given this file:

@if true {
  @import "foo";
}

When I compile it using Sass 3.1.7 I get the following error:

$ sass _3.1.7_ --scss nested_import.scss
Syntax error: Import directives may not be used within control directives or mixins.
        on line 2 of nested_import.scss

There was a bug where an @import that was nested conditionally in a file that was itself imported didn't get checked properly for this condition. I believe that bug was fixed in 3.1.8.

It was never intended that @import would work in a conditional context, this makes it impossible for us to build a dependency tree for recompilation without fully executing the file -- which would be simply terrible for performance.

@AriaMinaei
Copy link
Author

Maybe have a flag in the config file to enable/disable this restriction?
It's too big of a feature to not have.

On Wednesday, July 18, 2012, Chris Eppstein wrote:

Given this file:

@if true {
  @import "foo";
}

When I compile it using Sass 3.1.7 I get the following error:

$ sass _3.1.7_ --scss nested_import.scss
Syntax error: Import directives may not be used within control directives
or mixins.
        on line 2 of nested_import.scss

There was a bug where an @import that was nested conditionally in a file
that was itself imported didn't get checked properly for this condition. I
believe that bug was fixed in 3.1.8.

It was never intended that @import would work in a conditional context,
this makes it impossible for us to build a dependency tree for
recompilation without fully executing the file -- which would be simply
terrible for performance.


Reply to this email directly or view it on GitHub:
#451 (comment)

@chriseppstein
Copy link

It's not a feature that ever existed. Users who do this would encounter many strange bugs. If you like, please open a new issue and we can discuss adding this feature in a future release.

@AriaMinaei
Copy link
Author

Well I was using this for at least 9 months and didn't encounter anything
strange.

Thanks, I will open a ticket.

On Wednesday, July 18, 2012, Chris Eppstein wrote:

It's not a feature that ever existed. Users who do this would encounter
many strange bugs. If you like, please open a new issue and we can discuss
adding this feature in a future release.


Reply to this email directly or view it on GitHub:
#451 (comment)

@AriaMinaei
Copy link
Author

I have to add:

Just checked the compiled css files. Turns out all the fonts had been included for all languages. Such a huge bug in my code :))

alicebartlett added a commit to Financial-Times/o-header that referenced this issue Apr 21, 2016
node-sass just fixed this: sass/sass#451
meaning @import can no longer go inside @if.

This commit moves the @import outside of the @if.
alicebartlett added a commit to Financial-Times/o-header that referenced this issue Apr 21, 2016
node-sass just fixed this: sass/sass#451
meaning @import can no longer go inside @if.

This commit moves the @import outside of the @if and adds
@if $o-header-is-silent == false to each of the noisy imports
@kalaomer
Copy link

kalaomer commented May 1, 2016

Same error is still resuming. So what is the best way for resolve this?

@tamtakoe
Copy link

It already doesn't work in last version of libsass 3.3.6 but it works in 3.3.2

@robinpoort
Copy link

I know this issue is closed and a proper solution is being worked on by the Sass team, but this issue is the first result when Googling for @if @import.

I've been looking for a solution for quite a while now and I found somewhat of a temporary (little bit nasty) solution.

Warning: it's not the most elegant solution and it does create extra styling. This technique will only let you conditionally load files, so it won't allow variables inside the @import location (theming).

$file: true; // set to false if you don't want to load the file

%loadFile {
  @import "file/you/want/to/conditionally/load";
}

html {
  @if $file == true {
    @extend %loadFile;
  }
}

The html (or :root if you prefer that, or even a class like .a that's been added to the <html> element) is required and will thus add the extra CSS in front of all the selectors inside your file that's being conditionally loaded. Make sure you're aware of the specificity this creates.

The reason I need this is because I'm including Select2 in a library I'm working on and I really want to be able to conditionally load files depending on variables. So if I'm using this in a project I can decide per project if I need select2 (and all other molecules) or not. Since the select2 SCSS files have @imports inside @imports, a simple copy the file and find and replace wasn't possible. And of course, that would also mean that I couldn't use NPM anymore to load the files in the first place. So that was a big no-no. For the last year this was the only file I couldn't load. So I'm really happy I found a solution for this. Hope this can help someone else as well!

Again, not the most elegant solution and only useful if you want to conditionally load files. If you need theming I'm afraid you'll have to wait.

@ArmorDarks
Copy link

@robinpoort You can try mixins instead, as described in #739 (comment), #739 (comment) and #739 (comment)

@robinpoort
Copy link

@ArmorDarks yep that works for theming and other files that are related to your own project. Not the thing I was trying to solve though. As I stated I want to be able to conditionally load 3rd party libraries using only variables.

As you can see in the the select2 repo they're using @import inside their SCSS files: https://github.com/select2/select2/blob/master/src/scss/core.scss

There's even @imports inside these @imports. If I want to be able to import files from my node_modules folder I simply can't put all the code inside a @mixin because it's not my code.

So for the people who need theming (or only have to deal with their own files) definitely use @mixins to do this! If however you need to conditionally load 3rd party files using only variables, I still think the (still a little bit nasty) solution I came up with above could work for you.

Cheers!

@ArmorDarks
Copy link

ArmorDarks commented Jan 23, 2018

@robinpoort I see. Another solution would be to use node-sass importer override (as described here #2455 (comment)), to enhance it and wrap imports of 3rd-party code into mixins.

Then you could do something like this:

// `mixin@` says, that this import should be imported as a mixin
@import 'mixin@path/to/3rdparty/some-module';

@if $something {
  // Now, that module available to use as a mixin with module's filename
  @include some-module();
}

Since it matters only to your own project, using custom importer is quite okay. But it isn't for frameworks, though.

@robinpoort
Copy link

@ArmorDarks Thanks!

I agree my method isn't a very good solution for large frameworks with the extra CSS. Works for me now, might go and play around with your solution once it's out of the experimental phase!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

7 participants