fix: use indexOf instead of includes to support ES5 browsers

[Krinkle]

I'd like to use kleur inside the isomorphic js-reporters library, as used inside the QUnit application. We still support a number of plain ES5 browsers, such as IE11, and this one function call inside kleur is currently requiring a polyfill to ship with our standard distribution.

The code is very unlikely to be needed inside a browser, but I'm not sure how else to avoid it, also since the colors are enabled by default when Node/process is undefined. This is fine for us, but it'd help us a lot to avoid these two function calls for now.

[codecov-io]

Codecov Report

Merging #45 (0de4248) into master (fc9cc25) will not change coverage.
The diff coverage is 100.00%.

@@           Coverage Diff           @@
##           master      #45   +/-   ##
=======================================
  Coverage   98.77%   98.77%           
=======================================
  Files           2        2           
  Lines         163      163           
  Branches       28       27    -1     
=======================================
  Hits          161      161           
  Misses          1        1           
  Partials        1        1           

Impacted Files	Coverage Δ
colors.mjs	98.11% <100.00%> (ø)
index.mjs	99.09% <100.00%> (ø)

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update fc9cc25...0de4248. Read the comment docs.

[lukeed]

Hey, thanks~! Can you please "allow edits from maintainers" so I can make a minor adjustment?

[Krinkle]

Hey, thanks~! Can you please "allow edits from maintainers" so I can make a minor adjustment?

Aye, I didn't turn this off intentionally and don't see it here. Thus I can't turn it on. I think this is a bug on GitHub when the PR comes from an org rather than my personal account (even though I'm owner of that org).

Feel free to modify it as part of the squashed/cherry'ed land commit, though. I know maintainers sometimes prefer to run a change by the original author before landing it (with their name on it), but I trust your judgement.

[lukeed]

Oh, I didn't notice the org.
No no, not that! Just didn't want to bother you with a nit, since by now you may have trashed the project from your local machine. (Tho I do prefer squashing for clean log.)

Can you update all changes from foo.indexOf(bar) > -1 to !!~foo.indexOf(bar) for me please?

[lukeed]

Also, you probably already knew this, but if you had something like this, kleur is a no-op in browsers:

if (typeof window !== 'undefined') {
  kleur.enabled = false;
}

And if using kleur/colors, it never would have hit the includes call.

[Krinkle]

Also, you probably already knew this, but if you had something like this, kleur is a no-op in browsers:

if (typeof window !== 'undefined') {
  kleur.enabled = false;
}

I vaguely recall trying this, but I don't remember why I abandoned it. I might give it another go. Long-term I'd prefer to leave it because in my case the code wouldn't be used solely in a browser but only when either 1) running unit test and I'd prefer to keep the tests simple and strict with exactly asserted outcomes, and 2) there is one case where the code will intentionally run in a browser, which is when it runs headless and then console output gets reported via something like Karma Runner or Puppeteer back to Node.js standard out.

I'm guessing, but I don't know, that this might be the reason kleur is enabled by, when run outside of Node?

[lukeed]

It looks like you were trying to use the main kleur module, which would have still bumped into includes() to build the chain state.

While slightly more verbose, if you used the kleur/colors submodule, it wouldn't hit includes(). This variant is also tree-shakable, which is nice since you're bundling/ inlining deps.

Kleur tries to be enabled by default, because it'd be weird for something to try and do nothing by default 😁 So it tries to do something unless the environment (or the user) says otherwise. By default, kleur is harmless in the browser. Only the plain ANSI colors will have an effect in the console (except firefox I think) . Everything else is ignored & does not affect the string, which is nice & makes it easy to reroute the output into a terminal window as you mentioned

[lukeed]

Thanks for this, will merge and publish tomorrow. Am away from my computer for the night