fastify / point-of-view

Template rendering plugin for Fastify
MIT License
338 stars 86 forks source link

Feature/migrate html minifier terser #378

Closed multivoltage closed 6 months ago

multivoltage commented 1 year ago

Checklist

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

function syncMethod(){
   return "hello";
}
const output = await syncMethod()

Even point-of-view will add the terser version which is Promise based, configuring options like this:

const minifier = require('html-minifier')
// optionally defined the html-minifier options
const minifierOpts = {
  removeComments: true,
}
  options: {
    useHtmlMinifier: minifier,
    htmlMinifierOptions: minifierOpts
  }

can be done also using html-minifier instead html-minifier-terser

multivoltage commented 1 year ago

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. I will do an analysis next day.

If we agree to replace all callback with await async, do you you think I can put changes in that pr ?

mcollina commented 1 year ago

It's better to do the two things separately

multivoltage commented 1 year ago

Ok. I will try to use callback for this or. Thanks

multivoltage commented 1 year ago

@mcollina when you say to migrate all codebase to async await you are considering also for example readFile() or only calls "then()" for promises?

mcollina commented 1 year ago

All of it, yes.

multivoltage commented 6 months ago

I resume this PR since async flow was introduced

mcollina commented 6 months ago

CLosing for no activity

multivoltage commented 6 months ago

OK. I updated PR (like I said in the previous comment). Did you expect other changes?