Lessons from Less 3.0

[matthew-dean]

I thought this might be helpful to others - here are some of the lessons I learned and insights from working with the Less codebase:

1. Code Documentation - It is fairly poor. A lot of figuring things out requires a good deal of forensics. How/why things are the way they are is often not clear. I'm probably party to that - you tend to adopt the pattern given in a codebase. When you're working on a section with no documentation, putting in a large JSDoc can feel odd. But that should probably not be the case.

2. High abstraction - There are a lot of abstracted layers that allow for a high level of theoretical extensibility, but being undocumented, a lot of that theoretical extensibility will never be utilized. For instance, it looks like Less can have X number of file managers and even URL managers per system per directory per filename. And Less can have Y number of environments that encapsulate those? Less creates instances of classes that are only used once, but, in some cases, because of the way the prototype objects are exposed, creates more than one instance when only one is needed. In that sense, it almost resembles a Java API vs. a compiler whose job 99.99% of the time will be to run once and be done.

3. Strict program flow - Despite a lot of environment extensibility, the way nodes are parsed and evaluated is fairly rigid. This prevents things like incremental parsing / evaluation (not that that's a huge deal, but has been brought up in the past).

4. Nodes are both smart and dumb - Nodes hold data, and are visited with a visitor pattern (so are almost like a plain AST), but they contain lots of methods to self-evaluate their own nodes, but ALSO they are unable to fully do that without being passed in the context in which they reside. All of this makes for code that's hard to reason about, but it also makes a simple API for those nodes impossible. You can't call toCSS() on a node. You have to pass in context, even though that node is an instance which has a fixed context. So there's a lot of pushing / pulling of data flow in the process of node evaluation. Sometimes things act on nodes and modify them; sometimes nodes act on themselves and push data out. From a JS performance standpoint, it also means that many node instances are modified from their original prototype, so a JIT engine is unlikely going to be able to optimize them.

5. Some inconsistency in patterns - This is probably to be expected with open source, but in some cases, Less uses a callback pattern, and in other cases, Promises. Probably the above evolution of nodes is likely related.

6. Ghost code - There are probably irrelevant or unused options or sections of code that is just legacy stuff. Like I know chunking was removed as the default at one point?

Despite that, it doesn't take terribly long to do things, but there were lots of cases where I wish I'd known a pattern / interface already existed in the codebase, or times when I just never could fully grok how a section of code was intended to be used, no matter how much I looked at it.

I don't have any recommendations, but perhaps some recommendations could come out of this? It's worth opening for discussion, IMHO.

[seven-phases-max]

I'm afraid this is how any opensource project eventually looks like unless:

1. it has quite strict "used patterns" review on PRs.
2. (and) it's continuously refactored (by people having the same plan).

This is specifically dramatic for JavaScript with its lack of strict encapsulation thus where everything may touch anything anywhere.

[matthew-dean]

I'm afraid this is how any opensource project eventually looks like...

Absolutely! It's nobody's fault! That's why I don't even know if I can recommend anything can be changed, as its really dependent on people's time and energy, and often takes a skillset of setting into place those patterns and creating a bunch of the supporting documentation. So, I just wanted to make sure this was documented as a known problem.

[rjgotten]

I'm afraid this is how any opensource project eventually looks like

True. Much like the concept of technical debt; this is - at its root - organizational debt. And as you've both realized; this trickles down into technical debt.

Unless you are capable of burning maintainance hours at a set rate per month, to handle PR reviews; open refactoring work; etc. this is very hard to solve. You can't really hope to combat the accumulating debt itself while your resources are too limited. The best you can do in that case, is to first combat and get a handle on the interest rate; i.e. how quickly the debt rises.

For that, you should put into place design documentation, coding guidelines, etc.
Sure: it's a big one-time effort, but over the longer term, it will pay off.

Better documentation would also lower the barrier of entry and would hopefully get more people involved in maintaining the project, giving each of them more net-spendable time for refactoring and review work.

So it would work on both sides of the problem: ensuring better quality on incoming code, and creating more opportunity to improve on quality of existing code or to review incoming code.

[matthew-dean]

For that, you should put into place design documentation, coding guidelines, etc.

Who is "you"? 😉

[rjgotten]

Who is "you"? 😉

"you" in the abstract sense. I.e. "one should put into place"
But I get what you mean; it's a team effort.