Pass options to minifiers

[tdewolff]

Opinions needed, please tell me what you think

Passing options to the minifier would serve more users that have certain limitations or requirements. Options for HTML that come to mind:

• keep default attribute values
• keep whitespace (but collapse to one space)
• pass current scheme (http or https) so URLs can be shortened better
• pass the template delimiters to support HTML templates, i.e. '{{' and '}}'

There would be several ways to implement this. Would some or all of these settings be set on the encompassing Minifier level (all minifiers), would the settings be set on a minifier function level, or would the settings be set on a mimetype level?

Some settings are on the mimetype level: encoding of the mimetype (i.e. base64) and whether the CSS is inline or a stylesheet. These differ per source and are passed to the minifier function but not passed down to embedded resources. These are embedded in the mediatype now (i.e. text/css;inline which is used for <p style="color:#ff0;"> which is different from regular CSS stylesheets).

The options mentioned before would apply to a minifier function. One would pass options to AddFunc or derivatives. As shown below this happens in analogy to the Go stdlib HTTP handlers. You can pass the function or the struct (context, in this case options) containing the function. But this comes at a price: the API will be changed for most exported functions, invalidating currently working code.

Example of new code:

m := minify.New()

// add with options
m.Add("text/html", &html.Minifier{DefaultAttrVal: true})

// or add function directly (default options)
m.AddFunc("text/html", html.Minify)

// use minifier based on mimetype
if err := m.Minify(w, r, "text/html", map[string]string{"charset": "UTF-8"}); err != nil {
    panic(err)
}
// or use minifier directly
if err := html.Minify(m, w, r, map[string]string{"charset": "UTF-8"}); err != nil {
    panic(err)
}

Note that w and r are first parameters now, to adhere the style used in the Go stdlib (output first, input second, arguments next). I got rid of the mediatype in html.Minify and others, because if you call that function directly the mimetype is obvious/uninteresting.

All Minify functions have different signatures now! And AddRegexp has been renamed to AddPattern.

See PR #56

[tdewolff]

@omeid can you take a look since you proposed #39?
@tillberg this would solve your issue #41
@Knorkebrot this would pave the way to #35 by allowing configurable template delimiters.

[stroborobo]

Hey Taco,

I love that this is happening :) Thoughts from the top of my head:

• Multiple ignore delimiters would be nice. I'm thinking about templates with copyright comments (Usually like /*! ... */ in JS)
• Why is params a map, not a struct? And it's per type, html.Minifier{} with options exists.
  • Should params["scheme"] be in html.Minifier{}?
• A NewWriter() kinda API would be amazing like in the gzip package

[tdewolff]

Hey Knorkebrot,

Good points! I could build an option that preserves JS bang-comments, that would be trivial when options can be passed.

So you have options and parameters. Options are specified for a certain minify function (mimetype agnostic in principle, but probably linked in practice). The parameters would be specified for each minification separately. I guessed the scheme depends on which file you minify. You might first minify a page that lives on http, then next minify one that lives on https.

The reason that it is a map is because all minify functions must adhere the interface, so it can't be strongly typed because it differs for each minify function. I'm not fully satisfied with the map type, do you see a better alternative? Preferably strongly typed.

Could you give an example of how the gzip API would be applied to the minify package specifically? I'm interested to hear more about it, why would that method be beneficial?

Thanks for the feedback!

[tdewolff]

I don't think using the gzip's way of writer-to-writer is convenient. The reader interface is required for the underlying buffer, which can't work the way it does when written to by the user. I can however make a helper function that wraps a writer, because that is not really trivial code and I've seen people struggle with that before; mostly with ResponseWriter in a minifier middleware.

[tdewolff]

Merged.

[omeid]

Sorry, I am late to the party.

This looks sweet but is there a reason you don't take an options structure instead of assigning the configurations directly to the minfier object? Having an options structure allows to expose the config from a consumer package to others in a consistent way.

[tdewolff]

Yes, because each minifier has it's own options struct type, so when passed through a parameter (instead of through a receiver) it should be part of the minify function interface. But because each options struct is another type it should be either passed as an empty interface or by a minimal get/set interface or as a map[string]string. All those alternatives involve the loss of the type system.

This seemed the easiest and most straightforward way to implement it. How does this not expose the config in a consistent way? Do you have an example?

[omeid]

If a consumer wants to expose the minifier options, it will end-up looking like this:

type Options struct {
    KeepDefaultAttrVals bool
    KeepWhitespace      bool
}

//and then consume it as...
&html.Minifier{
    KeepDefaultAttrVals: opt.KeepDefaultAttrVals,
    KeepWhitespace:      opt.KeepWhitespace,
}

Instead of simply

type Options html.Options 

//and then consume it as...
&html.Minifier{html.Options{opt}}