Skip to content
This repository has been archived by the owner on Mar 25, 2021. It is now read-only.

Async-ified writing the tslint.json file in --init #3337

Closed
wants to merge 2 commits into from
Closed

Async-ified writing the tslint.json file in --init #3337

wants to merge 2 commits into from

Conversation

JoshuaKGoldberg
Copy link
Contributor

The runWorker method is already async. Added a general purpose async wrapper in utils around fs.writeFile.

PR checklist

Overview of change:

Uses fs.readFile within an async wrapper under utils instead of fs.readFileSync.

Josh Goldberg added 2 commits October 16, 2017 21:47
The `runWorker` method is already `async`. Added a general purpose `async` wrapper in utils around `fs.writeFile`.
Copy link
Contributor

@ajafff ajafff left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm -1 on this.
Awaiting the promise immediately only adds more work instead of saving time. You have the overhead of the threadpool, yielding to the event loop and all the costs that come with Promises and async/await

@@ -213,3 +215,15 @@ export function detectBufferEncoding(buffer: Buffer, length = buffer.length): En
export function denormalizeWinPath(path: string): string {
return path.replace(/\\/g, "/");
}

export async function writeFileAsync(fileName: string, data: string): Promise<void> {
await new Promise<void>((resolve, reject) => {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

you can simply return the Promise without awaiting

export async function writeFileAsync(fileName: string, data: string): Promise<void> {
await new Promise<void>((resolve, reject) => {
fs.writeFile(fileName, data, (error?: Error) => {
if (error !== null) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the mixed types here don't make sense. The optional paramter indicates that error may be undefined, but you compare it to null.
I guess consistently using null here is the right thing to do.

@JoshuaKGoldberg
Copy link
Contributor Author

I'm -1 on this.

Works for me.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants