Run ESLint and Prettier formatting in process #539
Conversation
This took an eternity because I went down a rabbit hole of parallelising Prettier and had little to show for it based on very rudimentary benchmarking of skuba itself; it seemed the overhead of spinning up multiple worker processes was significant. I may give this another shot with worker threads in future. While this is undeniably more code that using the ESLint and Prettier CLIs out of the box, it presents a few opportunities: - Customising logging output, particularly for `--debug`ging. - Taking the above point further: supporting Buildkite annotations, particularly when `lint`ing. - A marginally faster execution and smaller resource footprint. - Likelier compatibility with alternative module resolution approaches like Yarn PnP, which can make it harder to reliably locate other `.bin` tools when `exec`ing.
🦋 Changeset detectedLatest commit: fde6bbc The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
While this doesn't perform global linkage like `npm link`, its setup is almost instantaneous compared to an arduous install process with the npm command.
`fs-extra` is back, but only in `devDependencies`.
We'd rather get feedback on a snapshot diff first.
src/cli/adapter/prettier.ts
Outdated
| ) => { | ||
| logger.debug(filepath); | ||
|
|
||
| const [config, data, fileInfo] = await Promise.all([ |
There was a problem hiding this comment.
I’m guessing this is using lower level helpers instead of prettier.format to give better user feedback? Is it worth leaving a comment to that effect?
There was a problem hiding this comment.
prettier.format is fairly low level itself actually and is synchronous. I've tried to clarify this a little in the file.
src/cli/adapter/prettier.ts
Outdated
|
|
||
| const start = process.hrtime.bigint(); | ||
|
|
||
| for (const filepath of filepaths) { |
There was a problem hiding this comment.
What’s the performance difference of using Promise.all here? We could presumably overlap some I/O with the actual Prettier computation.
There was a problem hiding this comment.
I've gone with a staged read->process->write approach in b13e729 so the debug logs remain in order.
Co-authored-by: Ryan Cumming <[email protected]>
This gracefully backs off upon file descriptor exhaustion.
This isn't used anywhere as `--debug` already logs each file. Co-authored-by: Ryan Cumming <[email protected]>
Co-authored-by: Ryan Cumming <[email protected]>
This took an eternity because I went down a rabbit hole of parallelising Prettier and had little to show for it based on very rudimentary benchmarking of skuba itself; it seemed the overhead of spinning up multiple worker processes was significant. I may give this another shot with worker threads in future.
While this is undeniably more code that using the ESLint and Prettier CLIs out of the box, it presents a few opportunities:
--debugging.linting..bintools whenexecing.