Updates

[brettz9]

Let me know if and what of these you need in separate PRs (not too many line changes though).

• refactor: use fully ESM source
• testing: check where right of BinaryExpression is itself a BinaryExpression (brings to 100% coverage)
• chore: add source maps
• chore: update devDeps (babel/preset-env, ava, coveralls, eslint); check Node 12 and 14 in Travis
• chore: switch from --ignore-pattern to ignore file, allowing IDEs to avoid showing errors for ignored files

[coveralls]

Coverage remained the same at 100.0% when pulling 5eb233b on brettz9:updates into e90d3d3 on dustinspecker:master.

[dustinspecker]

This all looks great! I want to take a closer look at the export default later today after work. I thought there was a reason to prefer module.exports = over export default back in the day. I don't know if that's still relevant. But either way, this looks like a nice change to eventually remove the need for babel.

[brettz9]

Re: export default, ah yes, I forgot it needed a transform to create the module.exports which ESLint expects and which I've now added. I would expect a future ESLint supporting ESM would allow for consuming default ESM exports, so given that native ESM doesn't understand module.exports, it should be more future compatible to use the transform now rather than module.exports (and in any case, I think more appealing to be consistent).

Btw, I also added a target Node version for @babel/preset-env based on the minimum current engines. As you may be aware, the higher the target we can use there, the shorter and more native the code.

Re: eventually removing Babel, while it is nice one can more easily go that route these days, there still seem to be a good steady stream of very appealing features, like optional chaining or the null coalescing operator, so I'm personally hesitant to drop it in my own projects as a result. :-)

[dustinspecker]

Thank you! Sorry for the delay. This all looks good to me. I've added a comment about the source maps. After that gets answered I'm 100% cool with this change.

I've also created a commit on master that would have caught the breaking change to the default export.

Thank you! This all looks awesome.

[dustinspecker]

What's the value of of creating the source maps? Does this help consumers of this plugin? Or is it to help with local development?

[brettz9]

If there is an error, the user (if a technical enough user) will know the exact line in source to find the problem, and if not a technical user, if they report the trace to the project, it will make it faster for us to find the problem.

It also helps with testing coverage so long as your tests are pointing to rules rather than src--ensuring that if there are any problems (now with 100% coverage, that would only be for regressions) the proper line numbers would be shown. (While you could point to src in the test file, an advantage of keeping it as is is that it can fail if there are build problems.)

[dustinspecker]

WHOA! I didn't know Node added support to use source maps in stack traces. That is amazing. Thank you for sharing! I've been out of the loop on Node stuff. :)

[brettz9]

I've also rebased, and it is indeed nice to see full coverage including index.js now (as a result of your commit).

[dustinspecker]

Thank you for all of the enhancements!

[brettz9]

Thank you very much for the reviews!