Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with HTTPS or Subversion.

Download ZIP

Loading…

Lift the @include loop recursion restriction #943

Closed
hedron-dnd opened this Issue · 7 comments

5 participants

@hedron-dnd

I'm proposing the @include loop recursion restriction be lifted.

I'm unaware of the intent of this restriction, whether it has technical merit or not, however as far as I can see without further information, it only exists to prevent end-users from unwittingly causing a "stack overflow".

An alternative to lifting the restriction entirely, could instead be a recursion depth counter; only when hitting <arbitrary number> will a warning/error be issued. PHP behaves this way when running with XDebug, and will fatal if the stack count reaches 100.

Patching this per-project is all well and good, but it would make sense to remove or change this restriction if it only exists to insulate users from themselves.

For reference: http://stackoverflow.com/questions/15531004/sass-mixin-recursion-include-loop

As a side note, I'm happy to say I've learned enough Ruby at this point to patch this behavior and others, such as some of the CheckNesting constraints. Looking forward to contributing in the future.

@nex3
Owner

This restriction exists because it's relatively easy to accidentally end up with an infinitely recursive mixin and the default error message for such a case is extremely user-unfriendly. I haven't seen a use-case for recursive mixins that's compelling enough to override this concern.

@eoneill

We use recursive mixins at LinkedIn in Archetype. Our use case is, we have a list of key/value pairs of styles, and we want to iterate through that list and output them as css values.

e.g.

@include to-styles((
  background: red,
  color: blue
));

this gets complicated as this list can contain nested lists (of states) e.g.
e.g.

@include to-styles((
  background: red,
  color: blue,
  (states : (
    hover : (
      color: green
    )
  )
));

here's an example of the recursive mixin:
https://github.com/linkedin/archetype/blob/master/stylesheets/archetype/util/_styles.scss#L170

to get around this limitation, we used to have numerous mixins that did the exact same thing (to-styles, to-styles-2, to-styles-3, etc). We now monkey patch handle_include_loop! to allow a subset of mixins to be called recursively:
https://github.com/linkedin/archetype/blob/master/lib/archetype/sass_extensions/monkey_patches/handle_include_loop.rb

it'd be nice if this was a flag we could turn off, for the power uses that accept they power they've been given

@nex3
Owner

Writing map-processing mixins is a sufficiently compelling use case. I'm convinced; we should add this.

@nex3
Owner

Marking this as milestone 3.3 since it's so closely tied to writing mixins that can process maps.

@hedron-dnd

Glad to see that this has been planned :+1: My original intent actually predated the current use-case of recursive map-processing. I was trying to emulate what the parent selector interpolation now allows; something like:

$name-stack: ();

@mixin scope($name)
{
    push-onto-name-stack($name);
    @content;
    pop-from-name-stack();
}

@function get-name()
{
    @return concatenate-list($name-stack);
}

@include scope(foo) 
{
    .#{get-name()} // .foo
    {  
        @include scope(bar)
        {
            .#{get-name()} { } // .foo-bar
        }
        @include scope(qux)
        {
            .#{get-name()} { } // .foo-qux
        }
    }
}
@nex3 nex3 referenced this issue from a commit
@nex3 nex3 Allow mixin recursion.
Closes #943
054b486
@nex3 nex3 closed this
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Something went wrong with that request. Please try again.