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

Possible to output invalid CSS with a @function #2569

Closed
chriscoyier opened this issue Feb 28, 2018 · 3 comments
Closed

Possible to output invalid CSS with a @function #2569

chriscoyier opened this issue Feb 28, 2018 · 3 comments

Comments

@chriscoyier
Copy link

chriscoyier commented Feb 28, 2018

input.scss

@function test() {
  @if (false) {
    @return 0;
  } @else {
    opacity: 1;
  }
}

.my-module {
  opacity: test();
}

Actual results

Libsass 3.5

.my-module {
  opacity:   opacity: 1; }

Expected result

Throw an error like

Functions can only contain variable declarations and control directives.

http://libsass.ocbnet.ch/srcmap/#QGZ1bmN0aW9uIHRlc3QoKSB7CiAgQGlmIChmYWxzZSkgewogICAgQHJldHVybiAwOwogIH0gQGVsc2UgewogICAgb3BhY2l0eTogMTsKICB9Cn0KCi5teS1tb2R1bGUgewogIG9wYWNpdHk6IHRlc3QoKTsKfQo=

Spec sass/sass-spec#1218

@xzyfer xzyfer self-assigned this Mar 5, 2018
xzyfer added a commit to xzyfer/sass-spec that referenced this issue Mar 5, 2018
xzyfer added a commit to xzyfer/libsass that referenced this issue Mar 5, 2018
We treat `@else` as a block which breaks the check nesting algorithm
ported over from Ruby Sass. With change the intermediate `Block`
to container `@else` children is invisible. This better emulates the
the Ruby Sass algorithm.

Fixes sass#2569
xzyfer added a commit to xzyfer/sass-spec that referenced this issue Mar 5, 2018
@xzyfer
Copy link
Contributor

xzyfer commented Mar 5, 2018

Thanks @chriscoyier. This is an issue with how we validate @else blocks.

xzyfer added a commit to xzyfer/sass-spec that referenced this issue Mar 5, 2018
xzyfer added a commit to xzyfer/sass-spec that referenced this issue Mar 5, 2018
xzyfer added a commit to sass/sass-spec that referenced this issue Mar 5, 2018
xzyfer added a commit that referenced this issue Mar 5, 2018
We treat `@else` as a block which breaks the check nesting algorithm
ported over from Ruby Sass. With change the intermediate `Block`
to container `@else` children is invisible. This better emulates the
the Ruby Sass algorithm.

Fixes #2569
@chriscoyier
Copy link
Author

This probably isn't the perfect place to ask this, so feel free to shoo me there.

I'm curious how the rollouts work. This eventually goes into a point release for libsass, right? Then implementations of it need to update to that version, so if we're using Node, we wait for the next point release of node-sass?

@xzyfer
Copy link
Contributor

xzyfer commented Mar 6, 2018

Yep that's pretty much how it goes. Luckily I'm the maintainer of node-sass also so the turn around is pretty fast once we make a LibSass release.

mgreter pushed a commit that referenced this issue Mar 9, 2018
We treat `@else` as a block which breaks the check nesting algorithm
ported over from Ruby Sass. With change the intermediate `Block`
to container `@else` children is invisible. This better emulates the
the Ruby Sass algorithm.

Fixes #2569
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

3 participants