Closed brettz9 closed 4 years ago
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.
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. :-)
I've also rebased, and it is indeed nice to see full coverage including index.js
now (as a result of your commit).
Thank you for all of the enhancements!
Thank you very much for the reviews!
Let me know if and what of these you need in separate PRs (not too many line changes though).
right
ofBinaryExpression
is itself aBinaryExpression
(brings to 100% coverage)--ignore-pattern
to ignore file, allowing IDEs to avoid showing errors for ignored files