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

Incorrectly allowing functions to be defined in control directives or mixins #1550

Closed
xzyfer opened this issue Sep 16, 2015 · 7 comments
Closed

Comments

@xzyfer
Copy link
Contributor

xzyfer commented Sep 16, 2015

Originally reported on Twitter - https://twitter.com/kaelig/status/643584415789154304

Functions may not be defined within control directives or mixins but LibSass allows it i.e.

@if (true) {
  @function foo() {
    @return 'foo';
  }
}

Should produce the following error but does not

Error: Functions may not be defined within control directives or other mixins.
@xzyfer
Copy link
Contributor Author

xzyfer commented Sep 16, 2015

Other notable cases where we should, but don't throw this error.

@mixin foo() {
  @function foo() {
    @return 'foo';
  }
}
$i: 1;
@while $i == 1 {
  @function foo() {
    @return 'foo';
  }
  $i: $i + 1;
}
@each $i in (1) {
  @function foo() {
    @return 'foo';
  }
}
@for $i from 1 through 2 {
  @function foo() {
    @return 'foo';
  }
}

@saper
Copy link
Member

saper commented Sep 16, 2015

Nice enhancement :) Closures, anyone?

@xzyfer
Copy link
Contributor Author

xzyfer commented Sep 16, 2015

I like it, but it's not Sass :(

@xzyfer
Copy link
Contributor Author

xzyfer commented Sep 16, 2015

Turns out this also affects mixins

@mixin foo() {
  @mixin bar() {
    @return 'foo';
  }
}

@saper
Copy link
Member

saper commented Sep 16, 2015

Let's open an enhancement proposal :)

@xzyfer
Copy link
Contributor Author

xzyfer commented Sep 16, 2015

@perplexes
Copy link

Just ran into this with Igosuki/compass-mixins#34 / Igosuki/compass-mixins#67. It's trying to define compact if it doesn't exist. Since compact was stripped from libsass, this is needed to put it back in, in a version independent way.

Currently (as of b2a3d98) function definition is scoped to the control block, breaking this usage (@if not(function-exists(compact)) { ....). Causing this funny behavior: https://gist.github.com/perplexes/e0e75f00b3772f2bf643. It seems to not register with the globals, so even function-exists breaks within the block.

Current work-around is to just remove the if statement (since in my codebase I know we need it). Any suggestions on what compass-mixins can do, though? Maybe a function-if-not-exists directive?

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

No branches or pull requests

4 participants