aurelia / bundler

A library for bundling JavaScript, HTML and CSS for use with SystemJS.
MIT License
37 stars 25 forks source link

fix(minify): fix default htmlmin options #94

Closed gheoan closed 8 years ago

gheoan commented 8 years ago

Right now the bundler minifies html like so: Input:

<template>
</template>

Output (note the space):

<template> </template>

Output with this PR (no space):

<template></template>
EisenbergEffect commented 8 years ago

Is that safe?

gheoan commented 8 years ago

I think it is safe. However @ahmedshuhel chose these options with a reason so it might not be safe.

EisenbergEffect commented 8 years ago

@ahmedshuhel Any thoughts on this? I know we discussed the options. I can't remember the details of the conversation though ;)

CasiOo commented 8 years ago

What if the layout depends on at least one whitespace? Those would be then be removed.

It is good practice not to depend on spaces in your html to fix your layout. Instead of spaces, css should be used to position the elements. Those are the guidelines set by Google and others. I would say the default shouldn't be to have conservativeCollapse set to true.

EisenbergEffect commented 8 years ago

@ahmedshuhel Isn't there a way to override the settings?

EisenbergEffect commented 8 years ago

@Gheoan We may want to be conservative with our default options. However, I remember discussing with @ahmedshuhel that there should be a way to override them in the bundle config. @ahmedshuhel Can you clarify please?

gheoan commented 8 years ago

@EisenbergEffect The htmlmin options can be oberriden per bundle.

EisenbergEffect commented 8 years ago

I'm going to close this for now. @ahmedshuhel please submit a PR to the bundling docs to update it with information about minification of html. Thanks!