Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Cleanup of the Netlify CLI #156

Open
talentlessguy opened this issue Jan 28, 2025 · 8 comments
Open

Cleanup of the Netlify CLI #156

talentlessguy opened this issue Jan 28, 2025 · 8 comments
Labels
umbrella issue This issue contains a list of dependents of a package and tracks the progress in each

Comments

@talentlessguy
Copy link

talentlessguy commented Jan 28, 2025

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:

@pralkarz
Copy link

pralkarz commented Jan 28, 2025

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
Copy link
Collaborator

43081j commented Jan 28, 2025

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
Copy link
Author

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
Copy link
Author

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

netlify/cli#7008

@benmccann
Copy link

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
Copy link

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?

@Fuzzyma Fuzzyma added the umbrella issue This issue contains a list of dependents of a package and tracks the progress in each label Jan 29, 2025
@talentlessguy
Copy link
Author

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
Copy link

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
umbrella issue This issue contains a list of dependents of a package and tracks the progress in each
Projects
None yet
Development

No branches or pull requests

6 participants