Separate CLI and API

[atjn]

This PR splits the CLI package into two packages; an API, and a CLI that is build on the API. (Closes #934)

The goal

The goal of this initial separation is not to create a super sleek, well-thought-out API. That would require making some pretty big changes to the underlying code that I, as an outsider to the project, didn't want to impose on you.
My goal with this PR is just to take the code that is already there, and make it accessible to external programs. That does mean that the API has some funky design decisions and a long list of minor gotcha's, but I believe that can be iterated on as you move to a more stable design.

The general design

The API exports an ImagePool, which is mostly just a shell for a WorkerPool. You can add new images to the pool by calling imagePool.ingestImage(file). This returns an object representing the decoded image. On this object, you can then call things like image.encode(options) and image.manipulate(options), and then you can access an encoded image with image.encodedAs.jpg as an example.
If you want all the nitty gritty details, check out the readme. It's not perfect, but I think it should be good enough that most developers can figure the rest out themselves.

Other minor stuff

I have made some tweaks to the way npm packages are included. By letting npm manage them, instead of hardcoding them into the project, we gain a lot of cool things, like proper security alerts when a package finds a vulnerability, up to a ~20MB smaller install size (and even smaller publish size), and much better debugging.

I removed the Rollup step from the CLI. I don't think it's necessary, but I can easily add it back if you want me to.

For now, I have added JSDoc comments to enable intellisense and other goodies for developers using the API. This can easily be replaced with Typescript stuff in the future.

If you want to test the CLI on the API before you release the API to npm, you'll have to temporarily remove the API dependency from /cli/package.json, and you'll also have to follow the instructions on line 10 in /cli/src/index.js.

Happy reviewing :)

[surma]

This is amazing. Thanks @atjn! I’ll take a closer look later this week!

[surma]

This is great work. Thank you so much.

I have couple minor remarks that I left as comments.

One bigger remark is the removal of rollup build for the CLI. That means taht we are shipping ES module code which I am not sure is wide supported in older versions of Node. Even right now, if I link the api locally, it doesn’t actually run:

$ cd api
$ npm run build
$ cd ../cli
$ npm link ../api
$ node ./src/index.js
file:///Users/surma/src/github.com/GoogleChromeLabs/squoosh/cli/src/index.js:3
import { program } from 'commander';
         ^^^^^^^
SyntaxError: The requested module 'commander' is expected to be of type CommonJS, which does not support named exports. CommonJS modules can be imported by importing the default export.
For example:
import pkg from 'commander';
const { program } = pkg;
    at ModuleJob._instantiate (internal/modules/esm/module_job.js:98:21)
    at async ModuleJob.run (internal/modules/esm/module_job.js:137:5)
    at async Loader.import (internal/modules/esm/loader.js:165:24)
    at async Object.loadESM (internal/process/esm_loader.js:68:5)

I think we should rollup the CLI as well, but happy to hear your thoughts on this!

[atjn]

One bigger remark is the removal of rollup build for the CLI. That means that we are shipping ES module code which I am not sure is wide supported in older versions of Node. Even right now, if I link the api locally, it doesn’t actually run [...]

As far as I can tell, you "only" support all maintained versions of node (which is currently 12, 14, 16), and they can all run ES modules perfectly fine, so I don't see any problems there :)

As for the error you're encountering, I simply cannot reproduce it. I have tried running it with node and npx on different versions of node, and it worked in all cases. However, I can see that commander has improved their ESM support quite a bit in v7, so I'll add a quick patch to upgrade it, and hope that that works for you.

[atjn]

Commander v7 has some changes that means you might be able to use it more optimally if you rethink your implementation.
I just implemented it with as few changes as possible :)

[surma]

Thanks for addressing all of this, @atjn!

I am now seeing a problem that is isolated to Node 14 for some reason (Node 12, 15 and 16 work fine!). Any idea what to do here?

$ nvm use 14                
Now using node v14.9.0 (npm v6.14.11)
$ node ./src/index.js --mozjpeg '{}' ~/Downloads/02-01-implementations-1.png
file:///Users/surma/src/github.com/GoogleChromeLabs/squoosh/cli/src/index.js:11
import { ImagePool, preprocessors, encoders } from '@squoosh/api';
                                   ^^^^^^^^
SyntaxError: The requested module '@squoosh/api' is expected to be of type CommonJS, which does not support named exports. CommonJS modules can be imported by importing the default export.
For example:
import pkg from '@squoosh/api';
const { ImagePool, preprocessors, encoders } = pkg;

[atjn]

Oh, that makes perfect sense now! Node 14.9.0 did not support named imports yet, it was added in 14.12.0 I think. You should seriously consider upgrading to 14.17.0, you are missing a loot of security updates ;)

[surma]

Alright cool. If we can reasonably expect people to be on the most recent Node 14, then I’m happy with this (just confirmed it works with 14.17 :D)

[atjn]

I can implement a two-line fix if you want me to, but it will exclusively be adding support for Node versions that are an active security risk to have running :)
If it was up to me, I would ship this as is 👍

[pi0]

This is an awesome initiative @atjn <3 Just in time when was investigating to use squoosh for unjs/ipx > nuxt/image.

I was wondering is there any interest to make squoosh API compatible with the rest of the JavaScript runtimes? (Browser, Web workers, Cloud workers, and Deno). This could be possible by designing core API to not depend on worker_threads and other NodeJS APIs like fs and creating target APIs like @squoosh/node that wrap with environment powerups.

[atjn]

Thank you @pi0! I agree it would be awesome to see support for more runtimes in the future.

Another way to solve this, could be to make the API run exclusively in the browser, and then create glue packages for Deno and Node that simply runs the API with puppeteer. I'm thinking that would reduce complexity and make the project easier to maintain. It would also solve a bunch of current issues where the decoders in the API aren't as robust as the ones that run in the browser. The downside would be that you need to run a complete browser separately from the rest of your project.

[pi0]

Targetting for browsers and wrappers makes sense. I'm just curious isn't it possible to use wasm build directly in NodeJS/Deno runtime? (i guess there are edge cases that Chromium/Puppeteer handles them best)

[surma]

Thanks @atjn for seeing this through. Stellar work ⭐

I’ll probably push a commit or two to change this from @squoosh/api to @squoosh/lib, because “API” will be used for our embedding plans.

[atjn]

Awesome! Thank you too @surma for seeing this through, and thanks for the kind words along the way, they're very much appreciated ❤️

[surma]

🎉 Thanks again, @atjn!