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

@import in @if not allowed #1004

Closed
lllopo opened this issue May 12, 2020 · 8 comments
Closed

@import in @if not allowed #1004

lllopo opened this issue May 12, 2020 · 8 comments

Comments

@lllopo
Copy link

lllopo commented May 12, 2020

Dart sass does not allow the following construct:

@if $enable-light-style {
  .light-style {
    @import "include";
  }
}

saying Error: This at-rule is not allowed here. , while node-sass has no problems with it. Is this by design or a bug ?

@Awjin
Copy link
Contributor

Awjin commented May 13, 2020

Duplicate of sass/sass#451

@Awjin Awjin marked this as a duplicate of sass/sass#451 May 13, 2020
@Awjin Awjin closed this as completed May 13, 2020
@nex3
Copy link
Contributor

nex3 commented May 13, 2020

Note that recent versions of LibSass and node-sass also (correctly) forbid this.

@lllopo
Copy link
Author

lllopo commented May 13, 2020

Note that recent versions of LibSass and node-sass also (correctly) forbid this.

I’m using the latest node-sass and it doesn’t forbid it. I tried to switch to dart-sass, because node-sass doesn’t support @use and stumbled upon this problem. The main issue for me is, that I’m actually using a template, so it is out of my hands to correct this, but to be honest - I also can’t see a reason why this is not allowed.

@nex3
Copy link
Contributor

nex3 commented May 13, 2020

I’m using the latest node-sass and it doesn’t forbid it.

$ cat test.scss
@if true {@import 'foo'}
$ node-sass --version
node-sass       4.14.1  (Wrapper)       [JavaScript]
libsass         3.5.5   (Sass Compiler) [C/C++]
$ node-sass test.scss
{
  "status": 1,
  "file": "/home/nweiz/goog/sass/dart/test.scss",
  "line": 1,
  "column": 11,
  "message": "Import directives may not be used within control directives or mixins.",
  "formatted": "Error: Import directives may not be used within control directives or mixins.\n        on line 1 of test.scss\n>> @if 
true {@import 'foo'}\n\n   ----------^\n"                                                                                            
}

I also can’t see a reason why this is not allowed.

It's not allowed because it makes the set of files that are loaded, and thus the set of variables, mixins, and functions that are available, potentially different from compilation to compilation based on the values of different variables. This makes it difficult for computers to statically analyze your files, and more importantly it makes it difficult for humans to reason about what might be defined when.

In the new module system, you can use meta.load-css() to dynamically load a module's CSS, although it has a number of caveats and should only be used with care.

@lllopo
Copy link
Author

lllopo commented May 14, 2020

I’m using the latest node-sass and it doesn’t forbid it.

$ cat test.scss
@if true {@import 'foo'}
$ node-sass --version
node-sass       4.14.1  (Wrapper)       [JavaScript]
libsass         3.5.5   (Sass Compiler) [C/C++]
$ node-sass test.scss
{
  "status": 1,
  "file": "/home/nweiz/goog/sass/dart/test.scss",
  "line": 1,
  "column": 11,
  "message": "Import directives may not be used within control directives or mixins.",
  "formatted": "Error: Import directives may not be used within control directives or mixins.\n        on line 1 of test.scss\n>> @if 
true {@import 'foo'}\n\n   ----------^\n"                                                                                            
}

I also can’t see a reason why this is not allowed.

It's not allowed because it makes the set of files that are loaded, and thus the set of variables, mixins, and functions that are available, potentially different from compilation to compilation based on the values of different variables. This makes it difficult for computers to statically analyze your files, and more importantly it makes it difficult for humans to reason about what might be defined when.

In the new module system, you can use meta.load-css() to dynamically load a module's CSS, although it has a number of caveats and should only be used with care.

@nex3 You are right, but this is not my example. node-sass doesn't allow @import statements that placed directly to the control directive, but I if you put it inside a definition like my example - it does.

@nex3
Copy link
Contributor

nex3 commented May 14, 2020

I see. That's certainly a bug, and not something you should rely on. I've filed sass/libsass#3097 to fix it.

@vrushank
Copy link

@lllopo, I'm facing the same issue. Have you found any solution for this?

@lllopo
Copy link
Author

lllopo commented Feb 15, 2021

@vrushank Nope, unfortunately not.

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

4 participants