Cleanup of the Netlify CLI

[talentlessguy]

Currently it has 107 direct dependencies, and many more non-direct. It's worth cleaning them up.

Modules that can be replaced with other modules:

• chalk -> picocolors (mainly a size win)
• chokidar v3 -> v4 upgrade, -11 deps (if I remember correctly)
• find-up -> empathic, -7 deps
• dot-prop -> dlv + dset, might be that only one is needed
• execa -> tinyexec

Modules that can be removed in favor of 1-3 lines of code:

• is-stream
• p-filter:

const filteredItems = await Promise.all(
  items.map(async (item) => {
    const shouldKeep = await someAsyncCondition(item); // Replace with your async condition
    return shouldKeep ? item : null;
  })
).then((results) => results.filter((item) => item !== null));

• p-map, probably could be replaced with Promise.all
• p-wait-for:

const waitFor = async (condition, interval = 100) => {
  while (!(await condition())) {
    await new Promise((resolve) => setTimeout(resolve, interval));
  }
};

• through2-filter:

const { Transform } = require('node:stream');

const filterStream = new Transform({
  decodeStrings: false,
  transform(chunk, encoding, callback) {
    if (chunk.includes('foo')) { // Example filter condition
      this.push(chunk);
    }
    callback();
  },
});

process.stdin
  .pipe(filterStream)
  .pipe(process.stdout);

• from2-array:

const { Readable } = require('node:stream');

const arrayToStream = (array) => {
  return new Readable({
    objectMode: true,
    read() {
      array.forEach((item) => this.push(item));
      this.push(null);
    },
  });
};

• tempy -> os.tmpdir() + fs.createFile / fs.mkdir
• hasha -> crypto.createHash('sha512').digest()

PRs:

• remove hasha: chore(deps): remove hasha netlify/cli#7017

[pralkarz]

Another suggestion: ora -> nanospinner/picospinner.

Some dev dependencies could be dropped too:

• is-ci simply exposes a function from ci-info, so it's redundant given that the latter is a regular dependency
• fs-extra -> node:fs
• strip-ansi -> node:util (stripVTControlCharacters)

[43081j]

good issue 🙌

cc @sarahetter

another interesting one to look at, but much bigger job, would be moving from express to something like polka (or another fast/light alternative)

there's a few modern, lightweight node web servers floating around these days. using one could improve the dev server performance and overall CLI size i imagine

[talentlessguy]

good issue 🙌

cc @sarahetter

another interesting one to look at, but much bigger job, would be moving from express to something like polka (or another fast/light alternative)

there's a few modern, lightweight node web servers floating around these days. using one could improve the dev server performance and overall CLI size i imagine

shameless plug of tinyhttp, in case you need a smoother transition from express, rather than going hardcore with polka

[talentlessguy]

Seems like this PR is also related, it bumps a lot of deps and shrinks the lockfile heavily

netlify/cli#7008

[benmccann]

Moving to polka is quite easy. It's API compatible with express. You just have to be sure to use the version tagged with next (currently 1.0.0-next.28). The latest version, which is pre-1.0 has a different API. We've been using the next version in SvelteKit and have a good experience with it.

[benmccann]

Also wanted to mention since this issue doesn't yet that the package we're talking about here is netlify-cli: https://npmgraph.js.org/?q=netlify-cli

I wonder if it's necessary for it to depend on both fastify and express?

[talentlessguy]

Moving to polka is quite easy. It's API compatible with express. You just have to be sure to use the version tagged with next (currently 1.0.0-next.28). The latest version, which is pre-1.0 has a different API. We've been using the next version in SvelteKit and have a good experience with it.

depends on how they use it. If they use a lot of different request / response extensions, it's not that easy. If it's just res.send and res.json, then yes I agree it's better to move to Polka

[sarahetter]

Hey all! Thanks for all your investigation here. I'm excited about the potential work & cleanup! ✨

For the immediate future hold off on submitting PRs as we're actively scoping out a larger project in the netlify-cli and don't want to have any throw-away work.

I'll keep this issue updated when we have more information.

[serhalp]

👋🏼 Thanks so much for looking into this, folks! I've been working on the Netlify CLI lately and this has been helpful. Let me provide a quick update (and get this collated for my own sake):

• chalk -> picocolors
• chokidar v3 -> v4 upgrade — ah I hadn't seen https://github.com/paulmillr/chokidar?tab=readme-ov-file#upgrading, I'll take a look 👀
• find-up -> empathic
• dot-prop -> dlv + dset
• execa -> tinyexec — we probably won't do this one. We make very extensive use of execa and so does our dependency @netlify/build (so even if we removed it from netlify-cli we wouldn't save anything). Don't think the juice is worth the squeeze here.
• remove is-stream
• remove p-filter
• remove p-map
• remove through2-filter
• remove from2-array
• tempy -> os.tmpdir() + fs.createFile / fs.mkdir
• hasha -> crypto.createHash('sha512').digest() — chore(deps): remove hasha netlify/cli#7017
• ora -> nanospinner/picospinner — WIP fix(deps): replace ora and log-symbols with tiny dependency yocto-spinner netlify/cli#7026
• is-ci -> reuse dep on ci-info
• fs-extra -> node:fs chore(deps): remove unnecessary fs-extra netlify/cli#7025
• strip-ansi -> node:util (stripVTControlCharacters) — was this in reference to strip-ansi-control-characters? it's more than just strip-ansi
• express -> polka/tinyhttp/fastify — feel free to take a stab at this one? I don't think the ROI makes sense for us, but maybe it's straightforward for someone who knows these intricacies already
• (indirect) glob + fast-glob -> tinyglobby — WIP fix(deps): replace noisy deprecated glob@8 with tinyglobby netlify/build#6094

The ones with no comments I'll be knocking out soon I think. But if anyone here feels like grabbing any of them, by all means please go ahead and just give me a heads up! 🙌🏼

btw if you happen to have any tips about any of these (they're untyped, so we'd love to get rid of them anyway) please let me know 😄:

• maxstache
• maxstache-stream
• strip-ansi-control-characters
• tomlify-j0.4: I'm thinking of replacing with https://github.com/squirrelchat/smol-toml (goes from TOML spec 0.4 to 1.0 but that should be fine for how it's being used)

[talentlessguy]

I don't think it's worth moving from express to fastify tbh, it's a standalone framework and isn't meant to be consumed by libraries/tools directly. I'd use either polka or tinyhttp. I could send a PR with migrating to tinyhttp if you'd like

[Fuzzyma]

If you need glob support for chokidar, I can release my chokidar-glob package. You would be the first consumer tho. But all tests pass

[43081j]

thanks a lot for getting the list together @serhalp 🙏

there's a lot of low hanging fruit on there, easy wins. we should churn those out first and can figure out the larger stuff like express later (though i think you'll have got through most yourself soon)

depending on the usage of express inside the CLI, i would agree polka or tinyhttp will probably do nicely

If you need glob support for chokidar, I can release my chokidar-glob package. You would be the first consumer tho. But all tests pass

i think we should hold off on this until it has been proven out elsewhere. but you raise a point, we should be doing that very soon as we need to unblock the various projects in need of it

[benmccann]

I noticed a couple of dependencies that could be upgraded to cleanup the graph a bit. E.g. upgrading @vercel/nft would help a fair amount as I've been working on cleaning that up for the past several months

You can see a list of everything outdated here: https://npmgraph.js.org/?q=netlify-cli#color=outdated

[serhalp]

I don't think it's worth moving from express to fastify tbh, it's a standalone framework and isn't meant to be consumed by libraries/tools directly. I'd use either polka or tinyhttp. I could send a PR with migrating to tinyhttp if you'd like

@talentlessguy Sounds great, I won't decline that offer! 😄 Even better if it happens to remove the untyped express-logging dep, since I need to tackle that soon too.

[talentlessguy]

I don't think it's worth moving from express to fastify tbh, it's a standalone framework and isn't meant to be consumed by libraries/tools directly. I'd use either polka or tinyhttp. I could send a PR with migrating to tinyhttp if you'd like

@talentlessguy Sounds great, I won't decline that offer! 😄 Even better if it happens to remove the untyped express-logging dep, since I need to tackle that soon too.

what logger would be preferred?

[serhalp]

@talentlessguy no opinion! It doesn't need to be backwards compatible... It's just for local convenient pretty request logging. I think we can just pass a 1-2 line middleware instead of using a third-party dep here...? I guess we'll need something like https://www.npmjs.com/package/on-headers. Is that still the way to go these days?

[talentlessguy]

there's no need for on-headers, it's basically the same to what res.header and req.header do

[43081j]

can you explain that a bit more?

on-headers seems to be so you can write a bunch of headers just before writeHead happens

are you saying instead you'd just have regular middleware and set the headers there? so later middleware could overwrite/remove them is the only difference

[talentlessguy]

@43081j I think it just calls res.writeHead with headers. Could be replaced with res.writeHead directly as well. i might be wrong because last time I looked at on-headers source was 4-5 years ago

[43081j]

It seems to override writeHead such that it calls your callback whenever a request calls writeHead. So you could possibly achieve similar with middleware

It isn't a wrapper around calling writeHead though. It is a wrapper so when it is called, it calls your logic.

Worth having another read of the source to be sure though

[serhalp]

I might be a few years behind in Express patterns, but how else would you log something whenever a request completes?

[serhalp]

I noticed a couple of dependencies that could be upgraded to cleanup the graph a bit. E.g. upgrading @vercel/nft would help a fair amount as I've been working on cleaning that up for the past several months

@benmccann 😞 Unfortunately most of these (including @vercel/nft) are coming via our dependency on @netlify/build (and its friends), which are stuck with supporting node 14 and 16 (still supported by the Netlify build platform), which is blocking a lot of its potential dependency updates. We're working on coming up with an EOL timeline 🤞🏼...

[43081j]

if a request is considered "complete" when the response's head has been written, this pattern is probably still of some use unless you can do it through middleware 👍

@talentlessguy maybe you could take a look at the on-headers source and have a think of how you could achieve the same with polka/tinyhttp. its basically a way to run middleware just before the response is written (like a middleware you force to the end of the stack i suppose, but i could be wrong there)

to avoid derailing this thread, let's pick up on discord until we know the answer for sure

[talentlessguy]

I might be a few years behind in Express patterns, but how else would you log something whenever a request completes?

by listening to a request's "finish" event. See tinyhttp's logger as an example: https://github.com/tinyhttp/logger/blob/bdff8eb1522f1efb4e5faa5022eda9eb012754c1/src/index.ts#L71