coderaiser / minify

Minifier of js, css, html and img
https://coderaiser.github.io/minify
MIT License
228 stars 29 forks source link

feature(minify) add ability to pass options to HTML minifier via JS api #52

Closed aminimalanimal closed 4 years ago

aminimalanimal commented 4 years ago

I am using this package to minify code that will be copy/pasted into a custom template in a CMS. This CMS has stipulations for what it considers valid, and I therefore needed to turn off certain options so that the resulting file passes the CMS's tests. In particular, it was necessary to keep attribute quotes and keep the closing body and html tags intact.

This PR allows options to be passed in via the JavaScript API. The user's options are then combined with the default options before being passed to the HTML minifier.

This also resolves https://github.com/coderaiser/minify/issues/37, which was already closed, but was looking for the same sort of feature. It might resolve https://github.com/coderaiser/minify/issues/24 as well.

coderaiser commented 4 years ago

This is a very good idea, but would be great to have ability to configure all minifiers, not only html :). The simplest way would be to split options into 4 categories:

So example would look like this:

const html = {
    removeAttributeQuotes: false,
    removeOptionalTags: false
};

const options = {
    html,
};

const result = await minify('./client.js', options)
console.log(result);

What do you think about such API :)?

aminimalanimal commented 4 years ago

Sounds reasonable enough. I think I'll have time to do this tomorrow.

aminimalanimal commented 4 years ago

Updates made. I was able to locally validate that HTML, CSS, and JS options worked. My particular use-case doesn't have any images its passing through, but it works the same, so it should work.

coderaiser commented 4 years ago

That's amazing :), could you please add a couple tests for this?

aminimalanimal commented 4 years ago

I considered that, but hesitated. Given that minify's only job in this regard is to pass the options to other tools, and that the APIs are defined entirely by the other tools, it makes sense that minify would not want to fail any particular test due to one of its dependencies changing their API. So the only thing minify would want to test is whether another one of the tools successfully received the options, right? And I'm not exactly sure how to test that.

Maybe I'm overthinking it. Could use some brainstorming.

coderaiser commented 4 years ago

The simplest way to test things, would be to pass options into minify and call real API with the same options, and check that output is the same :). One test for each html, js, css and if it's not hard for img into existing test/minify.js would be enough :).

aminimalanimal commented 4 years ago

I've added unit tests following this direction, and also added a notEqual check (comparing versions with options to versions without options) to ensure the options are causing changes. I didn't add a test for img.

aminimalanimal commented 4 years ago

I also noticed that some of the existing tests were passing minify's output into terser/css-clean (effectively re-minifying items) instead of testing the output of the original input into both, so I fixed that. Hopefully I've correctly understood the intention.

coderaiser commented 4 years ago

That's amazing, thank you :)!

coderaiser commented 4 years ago

Landed in v5.1.0 :tada: