Skip to content
This repository has been archived by the owner on Jul 24, 2024. It is now read-only.

Consider splitting out the CLI as a separate module #1156

Open
callumlocke opened this issue Sep 17, 2015 · 5 comments
Open

Consider splitting out the CLI as a separate module #1156

callumlocke opened this issue Sep 17, 2015 · 5 comments

Comments

@callumlocke
Copy link

After reading sindresorhus's reasons for splitting out the CLIs from his modules, and seeing that Babel does a variation of this too (providing a CLI-free 'core' module), I've come to think it's a better approach.

I suggest making a new module, node-sass-cli, which would be a simple CLI wrapper for node-sass. (And remove all the CLI-related deps and code from node-sass itself.)

The benefits are:

  1. Quicker and less error-prone to install node-sass as a dep. Currently, if I want to depend on node-sass, I am forced to download and install a redundant file watcher too, which itself has a bunch of dependencies plus a postinstall script that takes minutes and can even error out sometimes.
  2. Separation of concerns. The -cli module can just focus on being a good CLI.
  3. One is a global application, the other is a library for importing into your code. Separating them would allow for shorter and more focused readmes.

Examples:

@saper
Copy link
Member

saper commented Sep 17, 2015

We still find some bugs in the "lib" code with cli.js, I'd love to have the parity between api.js tests and cli.js.

lib/render.js seems to live kind of on its own (used only by the CLI), good question where it should land, should we split.

And I think that bin/node-sass has a lot of redundant code and can be actually made much smaller.

I think in close future we will change the interface between the C++ bindings and the Javascript code and simplify JavaScript side a lot. Since I have introduced test/lowlevel.js to see how a "pure" C++ API feels like. Lots of similarities with what the CLI is doing actually...

But to me a priority would be to de-orbit sass_types and write them in JavaScript (#1088). For example, we could add some of the existing color modules as a dependency and just ask bindings to create /accept instances of it. And finally use normal null and JavaScript strings. We could have sass-map, sass-list modules if needed. (Unfortunately we cannot rely on tuples here).

@sindresorhus
Copy link
Contributor

👍

@thusoy
Copy link

thusoy commented Dec 7, 2016

@saper From what I can tell, this would cut the install size for the core with 14MB (on disk, from 26MB to 12MB) by getting rid of request, meow, gaze, npmlog, async-foreach, cross-spawn, get-stdin, in-publish (should be devdependency?) and sass-graph, which are only used for the cli. Not to mention that the shrinkwrap is cut from the current 824 lines down to 191, and the corresponding less requests done to npm to fetch all of these things.

@nschonni nschonni modified the milestone: 4.0 Jul 14, 2017
@saper
Copy link
Member

saper commented Nov 12, 2017

Looking at some open CLI issues I am getting more and more convinced this is the way to go. I'd love to fix the C binding API to make it usable without excessive wrapping as we do currently plus moving watching and other stuff into a separate possibly advanced command-line module.

@xzyfer
Copy link
Contributor

xzyfer commented Nov 12, 2017 via email

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

6 participants