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

Feature request: new option for import directive: "optional" #2145

Closed
AoDev opened this issue Aug 13, 2014 · 11 comments
Closed

Feature request: new option for import directive: "optional" #2145

AoDev opened this issue Aug 13, 2014 · 11 comments

Comments

@AoDev
Copy link

AoDev commented Aug 13, 2014

Hello,

This is a fairly simple option which I'd like to see in less.js.

It would look like this: @import (optional) "filename";
or @import-optional (reference) "filename"; (this way it can be combined to other import options)

If less does not find the file marked as optional, it won't fail and just continue the compiling process.

The use case is the following:
Less.js is part of a build process. (grunt based)

During this process, there are specific cases when we want to override the default styles. There are already variables than can be overridden for example.

@import "variables.less";
@import "mixins.less";
@import "buttons.less";
...

When we want to override the variables, we need

  • to copy the file containing the previous code
  • modify the code to add a new import
  • copy the file with the overrides in the same dir
@import "variables.less";
@import "overridden-variables.less"; 
@import "mixins.less";
@import "buttons.less";
...

If we don't do that, less will fail when overridden-variables.lessis not there.
It would be much easier to mark some files as optional that won't cause a problem if they are not present and this simplifies the build process.

What do you think?

Thanks for reading

@SomMeri
Copy link
Member

SomMeri commented Aug 13, 2014

Cant you just create an empty overridden-variables.less file if you do not need to override anything?

@AoDev
Copy link
Author

AoDev commented Aug 13, 2014

It would work, yes, but it was a simplified example. There are more files concerned and the build is more complex so I didn't think of creating an empty less file for each of them. You are right but it would be nice for less to not fail the whole compiling process just because one file/import is not always needed.

@lukeapage
Copy link
Member

Do you realise that variables can be over-ridden by definitions after they are used.. e.g. you could have one import right at the end.. does that simplify things?

@AoDev
Copy link
Author

AoDev commented Aug 14, 2014

I know the current possibilities so I'll describe what I am doing with more details. We are using less to build skins which can be children of one another for a given product. A child can override its parent variables, mixins or styles directly, then a second level of customisation during the build is offered, when again, it's possible to override the skins variables, etc to adapt the product to the website where it will be. This, is also based on Twitter Bootstrap from which we import some of the less files. It is actually a nice way to rapidly create and deploy new variations and it's easy to fix something in the parent skin for the benefit of all the children. But, we don't need to import everything all the time. So it would be easier to just indicate an import as optional in our case. As SomMeri said, we could put empty less files everywhere but it would be a lot of useless files.

@lukeapage
Copy link
Member

I've added ready for implementation because it seems like it would be quite a simple addition to add and would have some benefit. If any of the core team disagree please feel free to substitute with NeedsDecision and add a comment.
@AoDev would you be prepared to contribute this feature?

@AoDev
Copy link
Author

AoDev commented Sep 15, 2014

Hi,
I am really happy you consider this feature :)
@lukeapage Sure, I will help. (I am not familiar with less.js source code or parsers yet).
Is there some steps I should follow to start working on it?

I'd like to discuss what would be the best syntax for this.

  • @import-optional [(reference | ...)]
  • @import [(optional)] [(reference | ...)]
  • ...

@seven-phases-max
Copy link
Member

I'd like to discuss what would be the best syntax for this.

There's no choice in this case. All other import options are used like this:
@import (less, reference, multiple) "file";
So optional goes the same way (in fact you don't need to add any special code for the @import syntax, just add the new keyword somewhere around here and the code to handle it ... hmm, probably around here)

@AoDev
Copy link
Author

AoDev commented Sep 15, 2014

@seven-phases-max Thanks I'll check it. (although I see that your links points to 2.0.0 branch)

@seven-phases-max
Copy link
Member

links points to 2.0.0 branch

Yep, because I suspect that (since v2 is coming and it would not be a good idea to come up with new-feature-v1.x-release) if you make a PR for this to the 1.x branch the efforts needed to move it to v2 later will overweight the efforts needed to code the feature itself (though @lukeapage will correct me if I'm wrong :)

@vtamm
Copy link

vtamm commented Dec 15, 2014

What's the current status on this feature? Is someone working on it? I've been looking for a way to import a .less file that will ignore the import if the specified file doesn't exist, but haven't been able to find a solution that doesn't include using conditional variables (which eliminates the purpose since I have to set the variable anyway and might as well just set the @import).

@import (optional) would be awesome to have. I just checked the version history for the latest releases but couldn't find any mention of this.

@AoDev
Copy link
Author

AoDev commented Dec 15, 2014

@Villeman I said I would help but haven't done anything yet. I'm a bit overloaded with my own stuff. I don't think someone else is working on this.

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

5 participants