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] Namespaced values #3242

Merged
merged 25 commits into from
Jun 30, 2018

Conversation

matthew-dean
Copy link
Member

@matthew-dean matthew-dean commented Jun 28, 2018

This is an implementation of less/less-meta#12

I finally did it! After a bit of work, the namespaced feature is, I think, ready to roll. See https://github.com/less/less.js/compare/master...matthew-dean:feature/namespace-value?expand=1#diff-4dc2efdc41b7211e1b32b5afca199bf1 for test examples.

In short, you can access properties and variables from mixins and detached-rulesets, to use them as map-like objects, or just for convenience in retrieving a value.

Summary example:

@theme: {
  @width: 400px;
  @colors: {
    toolbar-background: red;
    toolbar-foreground: white;
  }
};

#ns {
  .mixin() {
    @height: 200px;
  }
}

.toolbar {
  width: @theme[@width];
  height: #ns.mixin[@height];
  background: @theme[@colors][toolbar-background];
  color: @theme[@colors][toolbar-foreground];
}

Will output:

.toolbar {
  width: 400px;
  height: 200px;
  background: red;
  color: white;
}

Please check out the branch and try it out!

@matthew-dean
Copy link
Member Author

Note, this PR doesn't introduce breaking changes discussed in less/less-meta#12 - namely, it doesn't stop mixins from dumping vars into the caller scope.

My thinking is we could introduce this feature, and document the deprecation of mixins leaking variables into scope. Then whenever that feature is removed (4.0?), there will hopefully be enough lead time and documentation to make the switch easy.

@matthew-dean
Copy link
Member Author

matthew-dean commented Jun 28, 2018

Note, this also does not include the part of that discussion around assigning mixins to vars, so that you could do.

I updated this PR to include setting mixins to variable calls / variable call lookups, see the added tests and error cases.

.foo {
  @theme: #ns.theme(dark);
  color: @theme[color];
}

@matthew-dean
Copy link
Member Author

matthew-dean commented Jun 29, 2018

So just now (after I should already be in bed), I realized you still COULD do some pretty amazing aliasing because of the nature and current behavior of mixins. I added two aliasing examples to tests to demonstrate this.

Prepare to be like...

tenor 1

So, with this PR, say you had this library file we'll call better-bootstrap.less.

// better-bootstrap.less
#library {
  .mixin(@a) when (@a = 1) {
    @a: 20px;
  }

  .core() {
    .colors() {
      primary: blue;
      foreground: white;
    }
  }

  .buttons() {
    .colors() {} // etc
  }
}

Okay, now we're gonna override some values in our overrides file.

// overrides.less
#library {
  .core() {
    .colors() {
      primary: rebeccapurple;
    }
  }
}

Now let's import both and see some maaaaaaagic 🌈

// main.less
@import "better-bootstrap";
@import "overrides";

// Let's alias a call to #library.mixin, because why the f not
.alias() {
  #library.mixin(1);
}

.foo {
  .colors() { #library.core.colors; } // alias
  width: .alias[@a];
  background: .colors[primary];
  color: .colors[foreground];
}

So NOW, you would get this output:

.foo {
  width: 20px;
  background: rebeccapurple;
  color: white;
}

In other words, you could deeply namespace your values, and then if you needed something more concise, you could just make a mixin call in another defined mixin.

2d3m32

Copy link
Member

@calvinjuarez calvinjuarez left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This changes the game. I'm so excited for this.

@TigersWay
Copy link

@calvinjuarez Not the only one excited :-) But someone should soon warn "gulp-less"!

Copy link
Member

@calvinjuarez calvinjuarez left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I just noticed that .vscode files were removed in an earlier commit. This one adds them back. I wonder if we just add .vscode/ to the .gitignore?

@matthew-dean
Copy link
Member Author

@TigersWay What's your concern about gulp-less? Would something in this PR break it?
@calvinjuarez I'm somewhat inclined to leave in the .vscode file (launch.json), because I got it working to very easily debug the Less lib using the integrated debugger, which makes development waaaaay easier. I know not everyone uses VSCode, but I thought it could be useful for others?

@matthew-dean matthew-dean merged commit 6237e13 into less:master Jun 30, 2018
@TigersWay
Copy link

@matthew-dean Did some testing with the command line as I quite love that PR. And no I did not find any break :-)
But I'll need to wait for to wait for a non-beta, than for gulp-less which will be waiting for accord to get any chance to really use it.
Just ranting, no worry!

@calvinjuarez
Copy link
Member

calvinjuarez commented Jul 1, 2018

@matthew-dean

but I thought it could be useful for others

Word. I love it. I just noted it 'cause a saw a previous commit remove some .vscode/* files, so I wanted to double-check it's intentional. I'm all about making it easy to debug and move fast. So my approval stands. 👍

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

Successfully merging this pull request may close these issues.

3 participants