-
-
Notifications
You must be signed in to change notification settings - Fork 87
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Feature/migrate html minifier terser #378
Feature/migrate html minifier terser #378
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm terrified of mixing callbacks and promises in this way. In my experience, it is the recipe for:
- performance issues
- memory leaks
- race conditions
I'm ok with moving the whole codebase to an async/await or just adding a .then()
to the calls to the minifier. However the best approach would be to drop the callbacks and use async/await everywhere.
In my small experience I prefer only await and async instead callback. But there are some case where we cannot wait a "line" but only start a "parallel" method. If we agree to replace all callback with await async, do you you think I can put changes in that pr ? |
It's better to do the two things separately |
Ok. I will try to use callback for this or. Thanks |
@mcollina when you say to migrate all codebase to async await you are considering also for example readFile() or only calls "then()" for promises? |
All of it, yes. |
ba6d662
to
9bc223e
Compare
I resume this PR since async flow was introduced |
CLosing for no activity |
OK. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lgtm
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lgtm
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lgtm
Checklist
npm run test
andnpm run benchmark
and the Code of conduct
Goal
I saw an issue of an user that ask support for html-minifier-terser in place of html-minifier.
Only thing that change is that minify method is a promise.
Behavior using old html-minifier
Since in js we can do
Even point-of-view will add the terser version which is Promise based, configuring options like this:
can be done also using html-minifier instead html-minifier-terser