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

On the Less API #15

Closed
matthew-dean opened this issue Mar 26, 2016 · 12 comments
Closed

On the Less API #15

matthew-dean opened this issue Mar 26, 2016 · 12 comments

Comments

@matthew-dean
Copy link
Member

This is a continuation of the 3.0 Roadmap Proposal Discussion here: #10


One of my hopes was that after custom root functions were working well, which they are, I could start digging in and try all sorts of fun things with the API.

But the more I try to do something useful, the more the API keeps kicking me in the shins. One of the major issues that the Less API has - it's completely wrong. I don't mean wrongly constructed; I mean things aren't called what they are. Just as some examples:

  • At-rules (or @-rules) are called Directives (?)
  • Declarations are called rules (which is wrong - "rules" is another name for "rulesets")
  • Rulesets contain collections of nodes, but nodes are called Rules, even though they might not be rules

To be fair, none of these things were named for an "API"; those are just the internal names originally assigned. Less didn't have plugins at that time, so it didn't matter. Moving on:

Node complexity

As @seven-phases-max noted in #10 (comment), Less, out of necessity, has a number of unique node types. That's fine, but what would be challenging for an average developer is that it's an unknown which node type they actually want to create, and a typical CSS value, for example, might be a collection of a number of different nested Less nodes.

For example: an Element node is a uniquely Less concept (I don't mean the concept of an element, I just mean the concept that a selector has sub-nodes and those sub-nodes have combinators), and it's not necessarily intuitive how one would construct a collection of Element nodes to represent a single selector. Or if one is creating a CSS value, which nodes might be created to exist as part of that value.

So, even if you could document the nodes, I'm not sure it would make any sense at first glance.

Proposal

I think there are 2 main things which could simply 99% of use cases.

  1. First, name things what they are. Change node types to be consistent to how they're documented in the CSS spec. Obviously, a concern is that this could be a breaking change to any plugins that add node visitors. But I don't think Less's plugin ecosystem has been that significant thus far partly for many reasons already listed. But someone could speak to that.
  2. I think it would be prudent to at least support parsing (in the API) of selectors and CSS values. That way you could write very simple statements like:
var decl = new less.Declaration("border": "1px solid black");
var rule = new less.Ruleset(".box", [ decl ]);

Right now something like that would take about 10x the code, and requires passing in information about index and fileinfo (the latter of which should never be needed). But I think it wouldn't take too much work to hook back in some of the parsing logic to make things a lot simpler.

@rjgotten
Copy link

@matthew-dean
name things what they are.

https://www.w3.org/TR/css-syntax-3/

That may assist, incase you hadn't found it yet. (The official nomenclature actually changes with CSS3 and it's far more common to stumble across the old and deprecated CSS2 nomenclature.)

Also; you are completely right. Using the correct names should help navigate around the compiler quite a bit. (It would also be really nice if there was a way to access a correctly labeled AST during debugging..)

@seven-phases-max
Copy link
Member

Just a remark. I guess certain things has to be taken with special care. For example:

var rule = new less.Ruleset(".box", [ decl ]);

It's not a Ruleset actually, it's Style Rule or Qualified Rule in general.
The spec. Ruleset is really nothing but a list of declarations (aka an array of rules, counting in Less every Style Rule itself may contain almost anything, not just "a list of declarations" like it's in CSS).

I.e. I mean certain drift in naming is expected due to practical implementation reasons (for instance the spec. does not say anything specific (it just has not to) about rules (as whatever array of rule) vs. {rules} (as a "block"), but in the code we often have to refer to both and strictly distinguish between them. Then if one tries to follow the spec. naming strictly... it goes doh...).

@rjgotten
Copy link

@seven-phases-max
the spec. does not say anything specific (it just has not to) about rules (as whatever array of rule) vs. {rules} (as a "block")

Actually the spec literally states that for a Style rule, the general Component value tokens inside its {} block token are to be parsed as a Declaration list.

It's kind of spread all throughout the spec, but it's there. ;-)

@seven-phases-max
Copy link
Member

Actually the spec literally states that for a Style rule , the general Component value tokens inside its {} block token are to be parsed as a Declaration list .

Maybe I was blurrish. What I really meant is that the CSS simply has no name for {rules} entity on its own (while in Less it's a thing to name specifically because it's (ideally has to be) (re)used with at-rules, style rules, mixins, DRs and plays with scope, and thus it's different from just a raw list/array of rules/declarations/whatever... And that's where Ruleset would fit perfectly (not meaning it's used like that in the current implementation of course)).
But never mind, I guess the "use CSS naming with caution since it may be be too CSS specific" message is delivered :)

@matthew-dean
Copy link
Member Author

@seven-phases-max @rjgotten MDN gives these definitions more clearly than the spec, which can be hard to parse.

I don't necessarily think it should be called ruleset; the only reason I wrote it that way is because it's somewhat correct. A CSS stylesheet is really a collection of rules, which include Style Rules and other rules, and those rule collections are technically called a RuleList.

So, as to this:

What I really meant is that the CSS simply has no name for {rules} entity on its own

If you mean a collection of, say, Style Rules, as you might have as children of a Media Rule, that's a RuleList in the CSSOM. So RuleList / Ruleset... okay, they're pretty close. Probably fine to keep Ruleset.

But that's really splitting hairs. As far as what collections are named, "rules" is probably fine to mean a "node collection". That's not my issue. My issue is that the Rule prototype does not define a rule. It defines a declaration. Yes, we don't need to follow CSS naming exactly, sure. But I don't think you can argue that we should directly contradict CSS terms, by calling a Less node type something that does exist as a term in CSS, like having Less Rule =/= CSS Rule. And the things in Less that are clearly still just the things in CSS (like AtRules), it doesn't serve us to try to build a public API where we don't call it those things. One can argue that we don't have to use the same names, but to me that's just shooting ourselves in the foot. The more we name things in a way that's intuitive and familiar, the easier an API is to use.

Another thing to keep in mind: when Less was first written, there wasn't much in the way of documented CSS ASTs. Now there are, so people may also be familiar with those AST conventions from other libraries. Some examples are here: http://astexplorer.net/

So the node types that are debatable or proprietary, those names I think we can leave alone. The other node types that are objectively incorrect I think should probably change.

@seven-phases-max
Copy link
Member

If you mean a collection of, say, Style Rules,

No, yet again {[rules]} is not the same thing in Less that [rules] is.

@matthew-dean
Copy link
Member Author

I'm not sure what that means. Or, it's confusing to follow because now we're talking about several things across two languages that are called "rules". Which is part of the point. It's hard to have the discussion and even understand what we're talking about, except by specifying which type of "rule" we're referring to.

@matthew-dean
Copy link
Member Author

matthew-dean commented Mar 28, 2016

@seven-phases-max I think you were just ultimately saying that Ruleset and its "rules" property is fine? That's ok with me. Maybe I should get more specific with proposals. I think we can limit changes to something like this.

  • Rename "Rule" prototype to "Declaration" prototype.
  • Rename "Directive" prototype to "AtRule".
  • Introduce some type checking and parsing in selector and value prototypes to allow passing in a string.
  • Move API statements to "less" object rather than less.tree. And as a forward-thinking idea, whitelist properties on "less" object to just what we plan on documenting. With the nature of JS objects, it's fairly trivial to just add an object/function onto the less object at the moment we want to expose / document.
  • Don't require index or fileInfo details when creating nodes. When ommitted, they should append the original function call index for sourcemap generation. Currently, the code only adds that info for the returned node. (Unless that's sufficient?)

That's not a huge heap of changes, and, in reality, needn't break any plugins in 3.0 with proper aliasing of prototypes. So, we can internally change the names and document them that way, but allow plugins to continue to call "new Rule" etc as we deprecate it (for 4.0?).

Is that an easier list to work with?

@matthew-dean
Copy link
Member Author

The one thing I'm not sure about: @lukeapage started an ES6 refactor but I don't know if that should be folded in or what the state is. Hopefully you'll see this Luke?

@matthew-dean
Copy link
Member Author

A lot of this work has been done, but the public API could always use more love (and needs to be documented).

@matthew-dean
Copy link
Member Author

matthew-dean commented Feb 11, 2018

Introduce some type checking and parsing in selector and value prototypes to allow passing in a string.

I know I added more casting of values to simplify the API as I went along, but am not sure which ones without going back and reviewing nodes.

@matthew-dean
Copy link
Member Author

The 3.0 goals were implemented for the most part, but I think a pivot is needed moving forward, so closing this in favor of a new discussion here: #31

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

3 participants