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

mixins and namespaces #57

Closed
jamesfoster opened this issue Jul 1, 2010 · 14 comments
Closed

mixins and namespaces #57

jamesfoster opened this issue Jul 1, 2010 · 14 comments

Comments

@jamesfoster
Copy link
Contributor

I'm trying to use a namespace to avoid naming conflicts as follows:

#ns {
  .square() {
    width: 10px;
    height: 10px;
  }
  .box() {
    .square();
  }
}

within the namespace #ns mixins should be able to reference each other.

#mybox {
  #ns > .box();
}

however, you get ".square is undefined"

@jamesfoster
Copy link
Contributor Author

Would it be possible to limit a mixin call or variable to the current namespace?

something like:

#ns {
  .square() {
    width: 10px;
    height: 10px;
  }
  .box() {
    && > .square();
  }
}

where && would refer to "the parent of the parent", in this case #ns

That way you could write a completely self contained namespace which isn't affected by naming conflicts outside that namespace.

@cloudhead
Copy link
Member

Hmm, well I guess mixins should be scoped like variables, so your first example should just work.

@jamesfoster
Copy link
Contributor Author

But you would still include mixins with the sane name outside that scope.

#ns {
 .mixin() { color: red; }
 .test() { .mixin(); }
}
.mixin() { content: "conflict"; }
#abc { #ns > .test(); }

This would output:

#abc {
  color: red;
  content: "conflict";
}

It would nice to able to make it self contained.

@cloudhead
Copy link
Member

In that case I think I would just specify the full path:

#ns {
    .mixin() { color: red; }
    .test() { #ns > .mixin(); }
}

The other solution is to have self-contained modules like in less.rb. Eventually I guess something like & > .mixin could make sense, but I don't think it's urgent, although I'm going to see, if I can get it to work in < 1h, I might do it.

@jamesfoster
Copy link
Contributor Author

yeah i thought about #ns > .mixin(); but that is ambiguous with #ns { #ns { .mixin() {} } }. granted that won't generally exist and in this case it's fine. I just thought there would be a better solution.

@jamesfoster
Copy link
Contributor Author

i just noticed that this is also an issue with variables

#ns {
  @width: 10px;
  .box() {
    width: @width;
  }
}
#mybox {
  #ns > .box();
}

it thinks @width is undefined.

@cloudhead
Copy link
Member

Right, because mixins aren't closures.

@jamesfoster
Copy link
Contributor Author

Just so I know, do you consider this a bug worth fixing?

If you like I can work on a fix and you can take it to fix less.js

I prefer to leave these fundamental decisions up to you so we don't diverge too much.

@cloudhead
Copy link
Member

I think closures are good, so yea, ideally mixins would behave that way—I just don't have the time to look into it. It might actually be really easy, ie just appending the definition's frames to the caller, when evaluating the mixin.

@jamesfoster
Copy link
Contributor Author

so does a mixin requires 2 frames to be evaluated properly? take the following example.

#ns {
  @width: 10px;
  .box() {
    width: @width;
  }
}
#mybox {
  @width: 20px;
  #ns > .box();
}

obviously, this fix would find the @width: 10px variable, but lets say @width: 10px didn't exist, would it use 20px? if they were both there which one would it prefer use?

I've got an idea for deriving the mixin.Definition frame... lets say the mixin.Call is at A, B, C, D, E and the mixin.Definition is at A, B, X, Y, Z

start with the current frame, and pop rulesets as you go up. and push rulesets as you find matches. so effectively you would pop E, D, then C and then push X, Y, and Z.

one issue with this is ruleset.find() would need to return an array of ruleset/frame pairs.

@jamesfoster
Copy link
Contributor Author

ok. here's a rudimentary fix

http://github.com/jamesfoster/dotless/blob/be24bfc1/src/dotless.Core/Parser/Tree/MixinDefinition.cs#L58

http://github.com/jamesfoster/dotless/blob/be24bfc1/src/dotless.Core/Parser/Tree/Ruleset.cs#L56

I simply build a path to the matching rulesets in Ruleset.Find and then add that onto the context for resolving the mixin. It possibly doesn't behave as you'd expect under the more complex situations but it works for this simple one.

@cloudhead
Copy link
Member

I just pushed a really simple fix which makes mixins behave like closures. The only case in which it doesn't work is when mixins are defined after mixin calls, but that's not a good idea anyway.

http://github.com/cloudhead/less.js/commit/04d2d3ab68af05c9c00e45418931236f315be416

@jamesfoster
Copy link
Contributor Author

I hate the javascript functions slice and splice, i always have to look them up. love how slice(0) just means clone().

You didn't answer my earlier questions:

lets say @width: 10px didn't exist, would it use 20px? if they were both there which one would it prefer use?

From looking at your change, I'm guessing the answers are "yes" and "10px"?

I think your change is possibly too simple. Even, as you say, for the simple case of defining the mixin after the call. Variables are late bound, so should mixins be.

Also, storing the frames on a mixin.Definition won't work for us since we allow a mixin.Definition inside a mixin.Definition:

.apples(@width: 100px) {
  .bananas() {
    width: @width;
  }
}

.apples(50px);

#test {
  .bananas();
}

... We don't know the frame of the .bananas mixin until we evaluate the call to .apples. calling .apples() effectively "unlocks" the inner mixins.

This is great for encapsulating mixins/variables for libraries. Lets assume you had a grid system with a default width of 960px, 16 columns with a margin of 10px. you could call

.grid();

and accept the defaults or call

.grid(1000px, 20, 5px);

and those would become the width, columns, and margin. you could then use the inner mixins like .span(@columns), .prefix(@columns) etc. as normal.

However, these are technically different issues and this issue should be closed?

@jamesfoster
Copy link
Contributor Author

I'll close this and open another

This issue was closed.
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

2 participants