Pm2: cli-table is unmaintained and don't handle null values

Created on 27 Jun 2017  Â·  12Comments  Â·  Source: Unitech/pm2

What's going wrong?

Setting a value to null in package.json "config" crash the install while rendering the cli-table, due to a two year old bug in cli-table.

How could we reproduce this issue?

Create a module, set config: {"foo": null} in package.json and try to install it.

Supporting information

PM2 version: `pm2 -v` 2.5.0
Node version: `node -v` v8.1.0
Windows? Mac? Linux? Description:    Debian GNU/Linux 8.7 (jessie)
Pending Release CLI Bug

All 12 comments

Indeed, we may need to fork it (or use a maintained fork) to fix the problem.

Just to make it clear, does having null as some values in the config of package.json looks a right choice to say "this setting does exists but is not set, you can set it"? If so I'll wait for a patch to use it, for the moment I use temporary meaningless values.

I forked it once (https://github.com/soyuka/cli-table/commits/master) to remove the pull bug (array check on the repo is bad, though there is a fix https://github.com/Automattic/cli-table/pull/83). There is also cli-table2 but doesn't look maintained either.

cli-table2 make PM2 CLI commands much slower (+- 30%)

Just spotted cli-table is from https://github.com/Automattic/ which … is active (they're WordPress). Not like it's hosted by some vanished developper on an abandonned github account. There's probably a way to get them merge those pull requests ^^

https://github.com/Automattic/cli-table/issues/91 whoops.

ping @Automattic @rauchg feel free to add me to the cli-table contributors and I'll take care of it (also on npm).

My proposal if they don't answer:

  • we fork cli-table
  • we merge all those pending PRs actually resolving blocking bugs
  • we publish the fork and use it on pm2

cli-table has a lot more open issues (35 more) than just the null value bug you identified. I built tty-table (which is backwards compatible with cli-table) and which fixes a good bit of those issues.

Two of the possibly helpful fixes tty-table has that cli-table doesn't (in addition to your null values fix):

  • automatic text wrapping
  • wide character (i.e. East Asian) support

```
var Table = require('tty-table')('automattic-cli-table');
//now runs with same syntax as Automattic/cli-table
...

Hello @tecfu

There are more then a couple issues but yeah they're often related to each other. There are 5 PRs fixing East Asian language support, though none of those seems satisfying enough to be merged (adding dependencies etc.).
We don't want to add more features (because we don't need them) and we want to reuse the original code as much as we can. Indeed, it currently fits PM2's needs very well and has been approved to be well-working for a couple of years now.

automatic text wrapping

Looks like this works well on the fork.

@tecfu I already tested your tty-table module, the only problem is that we can't just drag-and-drop the dependency into PM2, see the big difference (first being tty-table and the second is cli-table) :

image

We will be happy to use your module but we don't want to refactor your whole UX code to have the same experience, it's actually better for us (less work) to just fork and do our fixes.

Thanks for taking the time to test my module. I have the default width of the cells set to be a fraction percentage of the viewport, which makes it responsive when the viewport is too narrow to fit the minimum unwrapped column widths.

I'll change that and see to it that the views match cli-table if you wish.

I also plan to maintain my project for some time, and I'd pledge to put a high priority on any issues related to its performance in in pm2. But its up to you guys. If you just want to stick with the fork that's fine too.

After reading some of your code, there are three problems for me :

  • Your project is under LGPL v3 and we (keymetrics) sell enterprise version of PM2 so legally we can't use your dependency with any custom license.
  • It isn't a drag-and-drop replacement (actually there is a shift in the column, there is a version column that appear out of nowhere)
  • Your project install 88 modules and since we try to keep our dependencies at the bare minimum, it's a little problem for me.

In conclusion i think it's just better to just fork it, but if in the future all point are good, just make a PR and we will be happy to use your library :)

Was this page helpful?
0 / 5 - 0 ratings

Related issues

rangercyh picture rangercyh  Â·  4Comments

liujb picture liujb  Â·  3Comments

alexpts picture alexpts  Â·  3Comments

psparago picture psparago  Â·  3Comments

chaos-git picture chaos-git  Â·  3Comments