refactor: use host for watcher #2200
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
| export function findConfigFileName(host: LinterHost) { | ||
| const children = new Set( | ||
| host.readDirectory(host.getCurrentDirectory()).map((file) => file.name), | ||
| ); |
There was a problem hiding this comment.
I think we need the host to be async-first, blocking second. @auvred, is there a reason why the host is synchronous-first beyond that typescript is synchronous? Can we rename readDirectory to like readDirectorySync for ts and make readDirectory the promised version?
Also is there a reason getCurrentDirectory is a function? It's always constant. I really couldn't care less, if it's a style choice that's fine by me, but I'm mildly curious. Wouldn't it be more efficient (on a nano-optimization level ofc) to make it property?
| const cwd = process.cwd(); | ||
| const configFileName = await findConfigFileName(cwd); | ||
| const host = createDiskBackedLinterHost(process.cwd()); | ||
| const configFileName = findConfigFileName(host); |
There was a problem hiding this comment.
Should we still pass in the rootDir here? I feel like we don't need to if it's just an internal utility, but I could also see the benefit to being explicit and acting like all of our utilities are micro-libraries we happen to bundle.
| export interface WatchDirectoryOptions extends WatchOptions { | ||
| recursive: boolean; | ||
| } | ||
|
|
||
| export interface WatchOptions extends Abortable { | ||
| pollingInterval?: number; | ||
| } |
There was a problem hiding this comment.
I'm a Deno guy, I want my options bags.
Moreover, when I'm a Pythonista I use Ruff... where boolean-default-value-positional-argument reminds me not to fall into the boolean trap (we should totally port that rule!). Also I know we don't like the magicNumbers rule, for good reason. Moving numbers to options bags helps there too... I guess that's the "numbers trap"?
I'm happy to move these back, but putting signal as yet-another-argument felt silly and it's pretty ecosystem-standard to put signal in an object. And at that point the rest of them wanted to get moved too.
|
We really need a better way to do stacked PRs, this is stacked on #2199 so I have to keep this in draft but that stops it from showing up the review queue when this is equally ready for review... 🤔 |
lishaduck
force-pushed
the
hosted-cli2
branch
from
098c55e to
7bafdf6
Compare
lishaduck
force-pushed
the
hosted-cli2
branch
from
7bafdf6 to
75d8f23
Compare
lishaduck
force-pushed
the
hosted-cli2
branch
from
75d8f23 to
44e0308
Compare
PR Checklist
status: accepting prsOverview
❤️🔥
I don't remember if this works, I whipped this up on a 6-hour car ride to Oklahoma yesterday.