nicoespeon / sass-graph-viz

Draw a visual graph of Sass dependencies
https://medium.com/@nicoespeon/i-built-a-tool-to-work-with-complicated-sass-codebases-b5c909e269fb
MIT License
25 stars 2 forks source link

[Feature Request] `sass-graph` supports `.less`, can you? #10

Closed dosentmatter closed 5 years ago

dosentmatter commented 5 years ago

sass-graph had a PR a while back to support passing --extensions in the CLI and { extensions: [] } in the options object to support other file types such as less.

Examples:

sassgraph . --json --extensions css less
var graph = sassGraph.parseDir('/static/css', {
    extensions: ['css', 'scss', 'sass']
});

Can we get an --extensions option like sass-graph has? If not, can you at least pass all extensions to your call to parseDir()?

  return sassGraphGraphToGraph(sassGraph.parseDir(rootFolder, {
    extensions: ['less', 'scss', 'sass', 'css']
  }));

I tried it on my own project less and it seems to work fine: image

It still works fine on bootstrap scss: sassgraphviz node_modules/bootstrap/scss/bootstrap.scss image

I tried on antd but it goes crazy. I don't know if it's because it has a lot of dependencies. Not sure why bootstrap has green but antd is all gray. What does green stand for? Is it for sass partials? sassgraphviz node_modules/antd/dist/antd.less antd_sassgraphviz

If I run it on just their main theme file (not including their components), it works better: sassgraphviz node_modules/antd/lib/style/index.less image

I read your suggestions. The -s option is too large and is hard to grasp. I tried playing around with the physics, but that didn't help. I did physics: { enabled: false } but the graph would get stuck on the loading screen. When I set it back to true, it loaded. I also tried tweaking other physics settings like maxVelocity but it didn't help. I don't understand all the options.

Also, when a file argument is passed instead of a directory, perhaps you can use sassGraph.parseFile instead of parseDir. If you look at their code, they both call graph.addFile(). parseFile() calls it for one file. parseDir calls it for the list of globbed files by file extension. https://github.com/xzyfer/sass-graph/blob/master/sass-graph.js#L152-L161

Maybe a --depth option would help.

Thanks for the tool, it looks and works great!

nicoespeon commented 5 years ago

Hey @dosentmatter 👋

First, thanks for suggesting handling LESS & CSS files. It was easy to implement for sure. I think the best would be for the lib to just handle them, so I made the change and published a new minor version.

Not sure why bootstrap has green but antd is all gray. What does green stand for? Is it for sass partials?

That's a very good question, and I realized that I didn't put that explanation in the README. I just did.

Colors tries to add semantics to nodes:

I tried playing around with the physics, but that didn't help.

Thanks for letting me know. Indeed the physics doesn't help much when there are a lot of nodes. I'm open to anyone that would propose another rendering lib though. But I can't promise I'll do that for now.

Also, when a file argument is passed instead of a directory, perhaps you can use sassGraph.parseFile instead of parseDir.

Interesting. But I'm not sure I understand what this would change from a user perspective? Why should we use parseFile in that case?

Thanks for the tool, it looks and works great!

Thank you for your great and detailed issue 👍

That being said, I'm closing the issue as the original request (handling LESS files) has been solved with v2.3.0. But we can continue talking about the parseFile thing—or you can also create another issue for this particular suggestion, as you wish.

dosentmatter commented 5 years ago

Hey @nicoespeon, I'm busy right now so I'll respond to all your comments later. I just want to comment on one thing before I forget. Thanks for implementing my feature request! 😊

Interesting. But I'm not sure I understand what this would change from a user perspective? Why should we use parseFile in that case?

Sorry for not explaining, I missed it by accident. I had to remind myself of what I was thinking.

So parseFile documented in their API. The reason I suggest it is because the graph can be smaller if your sass/scss/less/css in your directory would have created a bipartite graph, or more generalized multipartite.

parseDir globs the whole directory for files with matching extensions, so you can get two sets of graphs. By adding less and css support, if you have a directory with both less and sass, you can get both a less and sass graph. Now, this might not be something you are worried about since less is not this project's focus. But sass can be affected too. For example, if you have files which aren't imported. I know you said:

orphan partials which are not imported are suspicious, so they are red

But, that can still work if you pass in a directory. If a file is passed in, the user is saying they only care about dependencies linked to that specified file. So passing in a file can guarantee a 1-partite graph, if that's the correct terminology.

sassgraph's CLI only uses parseDir but that is fine because they are only outputting text. Since you are drawing a graph, this is a useful optimization to provide the user. If I get the time, maybe I can look into creating a PR.