open-cli-tools / concurrently

Run commands concurrently. Like `npm run watch-js & npm run watch-less` but better.
https://www.npmjs.com/package/concurrently
MIT License
7k stars 228 forks source link

Installing package from npm takes a long time #436

Closed Gikkman closed 1 week ago

Gikkman commented 1 year ago

I noticed earlier today when I cloned an older project I have that running npm install took quite a long time. I drilled down in to why, and it turned out that installing concurrently was the main culprit. Or rather, the date-fns library it has as a dependency. I ran the installation instruction again, and supplied the --timing flag, to get some data:

$ npm i -D --timing concurrently

--- Some logs cut for longevity npm timing idealTree:buildDeps Completed in 1226ms npm timing idealTree:fixDepFlags Completed in 1ms npm timing idealTree Completed in 1305ms npm timing reify:loadTrees Completed in 1306ms npm timing reify:diffTrees Completed in 5ms npm timing reify:retireShallow Completed in 1ms npm timing reify:createSparse Completed in 346ms npm timing reify:loadBundles Completed in 0ms npm timing reifyNode:node_modules/wrap-ansi Completed in 413ms npm timing reifyNode:node_modules/is-fullwidth-code-point Completed in 417ms npm timing reifyNode:node_modules/string-width Completed in 417ms npm timing reifyNode:node_modules/concurrently/node_modules/supports-color Completed in 421ms npm timing reifyNode:node_modules/concurrently/node_modules/has-flag Completed in 421ms npm timing reifyNode:node_modules/get-caller-file Completed in 422ms npm timing reifyNode:node_modules/tree-kill Completed in 422ms npm timing reifyNode:node_modules/require-directory Completed in 427ms npm timing reifyNode:node_modules/regenerator-runtime Completed in 429ms npm timing reifyNode:node_modules/escalade Completed in 430ms npm timing reifyNode:node_modules/emoji-regex Completed in 434ms npm timing reifyNode:node_modules/cliui Completed in 434ms npm timing reifyNode:node_modules/y18n Completed in 436ms npm timing reifyNode:node_modules/spawn-command Completed in 435ms npm timing reifyNode:node_modules/yargs-parser Completed in 453ms npm timing reifyNode:node_modules/rxjs/node_modules/tslib Completed in 463ms npm timing reifyNode:node_modules/shell-quote Completed in 463ms npm timing reifyNode:node_modules/concurrently Completed in 524ms npm timing reifyNode:node_modules/yargs Completed in 537ms npm timing reifyNode:node_modules/@babel/runtime Completed in 605ms npm http fetch POST 200 https://registry.npmjs.org/-/npm/v1/security/advisories/bulk 939ms npm timing auditReport:getReport Completed in 944ms npm timing auditReport:init Completed in 0ms npm timing reify:audit Completed in 944ms npm timing reifyNode:node_modules/lodash Completed in 939ms npm timing reifyNode:node_modules/rxjs Completed in 1504ms npm timing reifyNode:node_modules/date-fns Completed in 35361ms npm timing reify:unpack Completed in 35363ms npm timing reify:unretire Completed in 0ms npm timing build:queue Completed in 2ms npm timing build:link:node_modules/tree-kill Completed in 21ms npm timing build:link:node_modules/concurrently Completed in 23ms npm timing build:link Completed in 23ms npm timing build:deps Completed in 25ms npm timing build Completed in 25ms npm timing reify:build Completed in 25ms npm timing reify:trash Completed in 0ms npm timing reify:save Completed in 36ms npm timing reify Completed in 37106ms

added 23 packages, and audited 309 packages in 37s

Notice how the step to reifyNode:node_modules/date-fns (which I think is unpacking the downloaded library) took a little over 35s, of the total 37s. I've re-ran the same command several times, and the results are pretty consistent.

I am not sure what is causing it to take such a long time, but I found an few old issue here talking about date-fns (#329) and it being imported in its fullness. Maybe that could be the problem here too, since the npm installation cannot tree shake the dependency.

Gikkman commented 1 year ago

I've researched for alternative libraries, but I haven't found any perfect replacements.

I could probably fix up a PR for switching to another formatting library that is smaller, if it would be interesting. But I'd like some input on what concessions would be acceptable (if any).

gustavohenke commented 1 year ago

Interesting find. TIL about --timing flag.

I see this problem (or similar?) has actually been reported to date-fns before: https://github.com/date-fns/date-fns/issues/3067 https://github.com/date-fns/date-fns/issues/2479

...but no satisfying solution yet (I think?).

My preference among the options you listed would be date-format, given it's rather popular, small in size and API surface (therefore easy to type ourselves). There might be more things to consider before committing to it though (bugs/security/maintenance quality).

@paescuj do you have opinions?

jpolo commented 9 months ago

There are other causes of heavy weight : rxjs. Imho, concurrently would benefit a great performance improvement if it just distribute a tree shaken optimized with no npm dependency cli bundle.

I don't see a major interest in distributing the library, but if you need to, it could be done in another package "core" which can hold the references to the heavy dependencies.

k2snowman69 commented 7 months ago

Alternative (and a bit hacky) but could open to removing the dependency all together is abusing Intl.DateTimeFormat:

const date = new Date("2024-01-02T00:04:05.006Z");

console.log(
  new Intl.DateTimeFormat('bo', {
    year: 'numeric',
    month: '2-digit',
    day: '2-digit',
    minute: '2-digit',
    hour: '2-digit',
    second: '2-digit',
    fractionalSecondDigits: 3,
    timeZone: 'UTC',
    hourCycle: "h23"

  }).format(date),
);
// Expected output: "2024-01-02 00:04:05.006"

Works on node 14.20 and above (tested using runkit).

The main cons with this option is it's:

  1. A breaking change to --timestamp-format
  2. Intl.DateTimeFormat is very limiting compared to date-fns (so you'll get some heat)
  3. Not really a good way to enable/disable Intl.DateTimeFormat options via the command line if someone wanted to customize the date

Low probability this idea would be taken seriously but in case someone else stumbles on this issue wondering if Intl.DateTimeFormat can be swapped in, here ya go!

gustavohenke commented 1 week ago

Hey! This is now fixed in v9.0.0. https://github.com/open-cli-tools/concurrently/releases/tag/v9.0.0

Gikkman commented 6 days ago

Awesome. Thank you for your hard work <3