Improve ES5 detection

[cdeutsch]

Found a few NPMs that aren't detected:

• numbro
• strict-uri-encode

Not sure if there is a better way to do the detection?

Maybe running a linting tool?

My company still supports IE11, so grateful for a tool like this 👍

[obahareth]

@cdeutsch You're most welcome 🙏 , so if something is not detected it's either because of #2 (which I'm not sure how to handle yet) or because acorn didn't detect it. At the moment, all this package does is check against the entrypoint file defined in a package's package.json, if that file is ES6 and above it'll pass. I didn't want to solve #2 because there may be a chance that it's not even an issue (premature optimization), and that most likely packages that use ES6 will use it in their entrypoints.

It looks like numbro's entry point is dist/numbro.min.js and that does seem like ES5 to me though. Here's an unminified version of numbro's entrypoint to double check.

strict-uri-encode's entrypoint is simply

'use strict';
module.exports = str => encodeURIComponent(str).replace(/[!'()*]/g, x => `%${x.charCodeAt(0).toString(16).toUpperCase()}`);

and that's definitely not ES5. I'll give that one a quick look (Acorn should've detected this) and investigate numbro more.

[obahareth]

So I tried adding these dependencies locally and this is the output I got:

✅ acorn is ES5
✅ commander is ES5
✅ numbro is ES5
❌ strict-uri-encode is not ES5

It did properly detect that strict-uri-encode (version 2.0.0) is not ES5, and numbro's distribution entrypoint is actually ES5 from what I can tell.

[cdeutsch]

Does are-you-es5 only scan direct dependencies in package.json?

If so, that would explain why I don't see strict-uri-encode, since it's a dependency of some other module in my project.

Not sure, what's wrong with numbro in my code. I'll look at that some more.

I think you can close this.

[obahareth]

Yes only direct dependencies at the moment. I was also not sure if we needed to go beyond that, direct dependencies served my use-case.

If another package includes strict-uri-encode but that package is transpiled to ES5, should I still throw an error for strict-uri-encode?. That's basically the dilemma.

[cdeutsch]

Looks like query-string is the NPM in my project that requires strict-uri-encode.

query-string is also ES6, and it's installing and pulling in strict-uri-encode and breaking IE11.

So, yes, I'd throw an error.

And it looks like even if query-string was ES5, it wouldn't compile strict-uri-encode. This is an ES5 fork I used to use for query-string; and strict-uri-encode isn't in the dist (assuming I compiled it correctly)

Side note: It looks like if I just replaced the @sindresorhus modules query-string and pretty-bytes in my project I wouldn't need any of this (for now) 🤦‍♂

[obahareth]

I'll do some more testing and try and find a solution that helps users of this package get a regex that works for all cases.

I think as a start I'll produce separate output for dependencies of dependencies and add a flag to include those in the ignore regex. Do you think we should go beyond checking the entry files for dependencies of dependencies (e.g. check the whole directory), or is checking the entry file enough?

I'll probably also add some form of caching to avoid double-checking a package (e.g. if two packages require strict-uri-encode, I should only check it once).

[cdeutsch]

I would lean towards checking the whole node_modules directory instead of looking at package.json files for dependencies, but maybe other people won't like that.

[obahareth]

I thought of that but I want to avoid it as much as possible because it would make this package incredibly slow, but I'll definitely consider it as a flag.

[cdeutsch]

Follow up on numbro in case someone else does this.

It was my fault.

I was importing the language files from src instead of dist so they were ES6, not ES5 like dist. Oops!

[obahareth]

@cdeutsch I released version 1.2.0 with support for checking all node_modules. There's a tiny breaking change and how to deal with it mentioned in the README. Can you let me know if this gives you what you need? (Better regex ignoring of babel/webpack stuff is still on the way)

[cdeutsch]

When I run with:

npx are-you-es5 check ./ -r -a

I get the error:

Cannot find module '/<FULL_PATH>/node_modules/.cache/package.json'

[obahareth]

Oh that's probably because I didn't ignore .cache (didn't have that in my repos), I only ignored .bin. I guess I'll ignore any folder that starts with a . to fix that.

[obahareth]

@cdeutsch I published version 1.2.1 which ignores all node_modules folders that begin with a . character. I hope that helps you in testing against all node modules.

[cdeutsch]

Seems to work. Thanks 👍