consolidate global and require() usages

[alexlamsl]

Hopefully this will keep the source code clean from multiple module import paths, and makes it more readable plus happy with things like webpack etc.

[alexlamsl]

It's commit like this where I wish Github provides a diff mode that ignore whitespace changes... 😅

[alexlamsl]

So this will hopefully solve #568 and more - @oliviertassinari if you can verify this that would be helpful!

And we can actually review the diff with whitespace changes ignored 👏

[XhmikosR]

This is a browser file and strict is fine in the function.

[alexlamsl]

strict rule from eslint complaints - do you want me to add /* eslint-disable */ instead?

[alexlamsl]

@XhmikosR both comments addressed, PR rebased

[XhmikosR]

Have you just tried setting node to false without turning off strict?

[alexlamsl]

No dice 😕

{
  "env": {
    "browser": true,
    "node": false
  }
}

gives

  114:2   error  Use the function form of 'use strict'  strict

[XhmikosR]

Weird, I'd expect it to handle the browser case. How about strict: [2, "function"]? Can you test if it works OK for both cases? Otherwise there is the safe option too.

[alexlamsl]

I think it's more to do with the Google Analytics code at the bottom of the script?

[XhmikosR]

If that's the case please just add eslint ignore around the snippet.

[alexlamsl]

"node": false would mean I need to add /* global: require */ as well - but okay, I'll get them 😅

[XhmikosR]

Then keep node...

[alexlamsl]

Alright:

• keep node: true
• change to strict: [2, "function"]
• add /* eslint-disable */ before GA

Sounds about right?

[XhmikosR]

👍

[alexlamsl]

@XhmikosR amended last commit to adjust eslint rules

[alexlamsl]

Editted commit message to close relevant issue.

[XhmikosR]

Ship it.
On Mar 24, 2016 16:51, "Alex Lam S.L." notifications@github.com wrote:

Editted commit message to close relevant issue.

—
You are receiving this because you were mentioned.
Reply to this email directly or view it on GitHub
#569 (comment)

[genintho]

@alexlamsl

It's commit like this where I wish Github provides a diff mode that ignore whitespace changes... 😅

https://github.com/kangax/html-minifier/pull/569/files?w=1

Sadly you can not comment in this mode.

[kangax]

Long time coming and is much needed by now! :)

[kangax]

Next step — ES6 via babel :)

[XhmikosR]

Will you make the switch? :)

BTW I'm thinking we might need to find another parser.
On Mar 24, 2016 23:58, "Juriy Zaytsev" notifications@github.com wrote:

Next step — ES6 via babel :)

—
You are receiving this because you were mentioned.
Reply to this email directly or view it on GitHub
#569 (comment)

[alexlamsl]

@genintho yeah, thanks for the tip 😉

@kangax is that the same babel that broke and went onto the news a few days back?

@XhmikosR thanks for the review! To switch HTML parser we'll need to decouple the logical dependencies first, but it's something to think about nonetheless.

[kangax]

@kangax is that the same babel that broke and went onto the news a few days back?

Nope, nothing to do with Babel. Babel broke because the author of one of the (npm) dependencies pulled them out.