Async-ified Linter.lint #3336
Async-ified Linter.lint #3336
Conversation
Breaking change: Forces the public `Linter`::`lint` and `runTests` to be async too.
# Conflicts: # test/ruleTestRunner.ts
| const files: string[] = []; | ||
| for (let pattern of patterns) { | ||
| if (path.basename(pattern) !== "tslint.json") { | ||
| pattern = path.join(pattern, "tslint.json"); | ||
| } | ||
| files.push(...glob.sync(pattern)); | ||
| } | ||
| return files.map((directory: string): TestResult => runTest(path.dirname(directory), rulesDirectory)); | ||
| return await Promise.all( |
There was a problem hiding this comment.
Don't return await promises, just return the Promise. There are several cases thoughout this PR.
There's already a PR to add a rule to prevent this.
There was a problem hiding this comment.
Interesting! I remember anecdotally hearing some preference for return await for standardization of where execution context results during errors... but all I can find now to back it up is https://stackoverflow.com/questions/38708550/difference-between-return-await-promise-and-return-promise (and that doesn't). Will change.
|
Currently I'm -0 on this one. But let's continue this discussion in the issue you just created. |
PR checklist
Overview of change:
Changed
Linter.lintto beasyncand read/write files using the asynchronousfsAPIs. Necessitated also changingapplyAllFixesandapplyFixeswithinLinter,runTestsandrunTestwithintests, and the runner logic inruleTestRunner.CHANGELOG.md entry:
[api] Breaking change:
Linter.lintis nowasync(returns aPromise)