lint-staged / lint-staged

🚫💩 — Run linters on git staged files
https://www.npmjs.com/package/lint-staged
MIT License
13.05k stars 412 forks source link

Consider allowing running of programatic tasks via Node API #1398

Open pastelsky opened 4 months ago

pastelsky commented 4 months ago

Description

Currently, the signature of a task in lint staged is —

(filenames: string[]) => string | string[] | Promise<string | string[]>

where it always expects a shell command as output that it runs via execa. However, consider adding support for promise returning functions instead. E.g.

import { doFoo } from 'package-foo';

export default {
   '**/*.js': async (filenames) => {
      console.log('Start a task')
      await doFoo(filenames);
      console.log('End a task)
  }
}

Advantages —

  1. In a few cases avoid shell execution cost all together (this can be in seconds in large projects using yarn)
  2. Allow for more flexibility in what's executed inside the task fn — including logging / telemetry etc

Given lint-staged is used in steps like pre-commit that need to be lightning quick, saving on shell invocation / yarn invocations costs can be significant. I understand there is value in the declarative nature of the current config — but optimizing for latency in ms is an important goal for something that runs in the critical path of dev workflow.

If this is allowed, the signature would become —

(filenames: string[]) => string | string[] | Promise<string | string[]> | Promise<void>

Where Promise<void> can indicate a freeform task.

Is this something that sounds interesting to you? Do you see any challenges with this?

okonet commented 4 months ago

I really like it! Do you want to create a PR for that? @iiroj wdyt?

iiroj commented 4 months ago

Is the functionality essentially the same as creating a separate Node.js script file, and having that be the linter? What benefit is there for defining it directly in the config?

It sounds doable, but is a separate pipeline to the existing thing being based on spawning processes with execa.

iiroj commented 4 months ago

To answer myself: one benefit is that file names would be passed directly so there's less need for escaping etc.

iiroj commented 4 months ago

This might be a good use of node:worker_threads so that we can run them in parallel and capture console output similar to execa.

iiroj commented 3 months ago

Since we execute the current "function config" functions before running actual tasks and also validate the output to be string | string[], the only way to handle this in a backwards-compatible way would be to return more functions:

// lint-staged.config.js
export default {
  "*.js": (stagedFiles) => {
    return () => lintJsFiles(stagedFiles)
  }
}

the syntax of the returned functions could be:

type TaskFunction = (stagedMatchedFiles: string[]) => Promise<void>

but since they're returned from inside a config function, one could of course handle the matched filenames in there as well.

what do you think?