@import with variables

[gustavohenke]

Hey, finally is here, done without much API changes as it was in #426.
This was discussed a lot in #410 and other issues that reference it.

This was only tested in Node. No browser, no Rhino for now.
As soon I got time to do this (currently it's 3h45 here in Brazil :/), I'll do it 👍

Thanks in advance

[lukeapage]

Hi,

that is an interesting way of doing it - one I hadn't considered.

I guess that this will only support variables on the root, e.g.

.getVariable() {
     @myvar: 'a';
}
.getVariable();
@import "@{myvar}.less";

and that if the variable is in an import, it also won't work?

@import "variables.less";
@import "@{myvar}.less";

My intent to doing this was going to be

1. make the eval phase asyncronous
2. provide a wrapper such that if eval doesn't need to be asyncronous, the old way of calling less will work. if it is asyncronous and the wrapper is used, throw an exception that features are being used not supported by the current interface.
3. in an import, if it contains variables, put off requesting it in the parse phase
4. in an import, if it hits the eval phase, eval the string, request the import and then call back on the eval

and this would fit into 2 refactorings I would like to do in the future, but that wouldn't have to be part of this feature

1. make env into a prototyped variable that can handle cloning
2. make env responsible for building the css
3. introduce a visitor pattern and do multiple passes of the ast

If someone provides something that will be useful to people but is a smaller change I have no problem committing it as a temporary measure, as I know its taking me a while to get this done.

[gustavohenke]

Yeah, you're right, this will only work with variables defined in the root of a less file.
Do you think that looping thru each mixin called in the root would be very painful, or not? This way we can resolve one of the problems.

Another point to note here: this wouldn't be able to import "yet undefined vars". For example, when I do this, it'll work:

body { color: @color }
@import "fileWithTheColorVariable.less";

However, I think that imports doesn't need to allow this, at least for now; heavy refactorings would be needed, don't you agree?

[lukeapage]

"Do you think that looping thru each mixin called in the root would be very painful, or not? This way we can resolve one of the problems."

Then it really starts to become a hack around doing it properly and it could have unintended side effects.

"However, I think that imports doesn't need to allow this, at least for now; "

If the variables are known in the file then you don't really need to use the variables in the import - this might help out some people, but I'm not sure how many.

"heavy refactorings would be needed, don't you agree?"

I describe the refactorings above and yes, its not trivial

I would leave this here.. if I don't get variables in imports working how I'd like it done (or get another pull request with a more complete solution) then I will take another look at pulling this as an interim solution.

[gustavohenke]

So, I'm leaving this up to you. Anything else you want me to do with this pull request...?

[lukeapage]

not at the moment. I'll definitely take a couple of the clean up commits.

[gustavohenke]

Okay then :)
I'll be picking other features/bugs here. It's great to help with the LESS development.

[lukeapage]

great, thats wonderful.

I listen to all activity so just put a comment on an issue or feature with a proposed way of fixing if you are not sure or it is a large piece of work, then we can make sure you don't do anything un-necessarily.

The current branch is 1_4_0 ;- unless we get anything serious in 1.3.3 in the next week or so I will move that to master, so work on top of that.

Some feature requests we've worked out what to do, some it is still under debate. I think that everything under the milestone 1.4.0 we know what to do, but it may or may not be documented in that particular issue (many have related issues or we've spotted a pattern and have found a way to make a feature that satisfies multiple use-cases and therefore feature requests.

[gustavohenke]

Right. Can I contact you privately about less? My e-mail is in my profile. Thanks.

[gustavohenke]

@agatronic, testing this in my code the last day it seemed unstable.
By using syncImport: true, the things are much more stable until now.

[lukeapage]

Closing this as my import-interpolation branch is now equal in functionality.

[jonschlinkert]

👍