asyncLiz / minify-html-literals

Minify HTML template literal strings
MIT License
68 stars 14 forks source link

Returns null instead of `{error}` #11

Closed WebReflection closed 4 years ago

WebReflection commented 4 years ago

I'd love to use this optimizer in conjunction with other libraries, but minifyHTMLLiterals(validJS) returns null, where validJS can be the source code of any file.

I've tried to pass a filename too but basically this always returns null.

Any idea what am I doing wrong? Is this supposed to return null when the file has no html template literals in it? If that's the case, what does it return in case the file contains template literals with html in it?

Thanks.

asyncLiz commented 4 years ago

If there's nothing to minify, it'll return null. From the documentation:

@returns the minified code, or null if no minification occurred.

If it minifes the code, it'll return a Result object.

WebReflection commented 4 years ago

uhm ... may I ask how come that's the case? ergonomics are a bit awkward, i.e.

// this throws
const {error, code} = minifyHTMLLiterals(code);

// this is a bit awkward
const maybeNull = minifyHTMLLiterals(code);
saveContent(maybeNull && !maybeNull.error ? maybeNull.code : code);

Wouldn't it be simpler to just return code as is as Result object?

WebReflection commented 4 years ago

P.S. since we are here, is the filename really mandatory? it doesn't warn me, and I don't want to use a file name as it's all runtime without files access, thanks.

asyncLiz commented 4 years ago

This library was primarily built for rollup-plugin-minify-html-literals. In rollup, if a plugin returns null, that indicates that no transformations should be made, which is why this library does that.

The filename is optional. The downside is that the generated sourcemap will not point to the right file.

asyncLiz commented 4 years ago

Additionally, error is not a property of the result. If there is an error, it will be thrown.

try {
  const result = minifyHTMLLiterals(code);
  if (result) {
    // minified
    const { code, map } = result;
  } else {
    // not minified
  }
} catch (error) {
  console.log(error);
}
WebReflection commented 4 years ago

The filename is optional.

Screenshot from 2020-05-08 19-59-06

Additionally, error is not a property of the result. If there is an error, it will be thrown.

fair enough.

I guess my last question would be: is there a way to not bother with the map ? I guess not passing a filename would do, but it's not super clear in the README.

I was going to file a PR but I wouldn't know if it's Required for source map or simply optional.

asyncLiz commented 4 years ago

Looks like the readme is outdated, it's not required. If no filename is provided, the library will give an empty string as the filename to the sourcemap generated.

If you don't want the map, you can just ignore it.

const result = minifyHTMLLiterals(code);
const minified = result ? result.code : code;

Let me know if that doesn't work.

WebReflection commented 4 years ago

Thanks!

WebReflection commented 4 years ago

Sorry for the late question, does this reason well to you?

    new Promise((res, rej) => {
        try {
          const mini = minifyHTMLLiterals(content);
          const {code, error} = terser.minify(
            mini ? mini.code : content,
            terserArgs
          );
          if (error)
            throw error;
          else
            res(code);
        }
        catch (error) {
          rej(error);
        }
    });
asyncLiz commented 4 years ago

Makes sense to me