search

beaugunderson / ip-address

πŸ’» a library for parsing and manipulating IPv4 and IPv6 addresses in JavaScript
http://ip-address.js.org/
MIT License
525 stars 71 forks source link

Add hybrid ESM support #138

Closed beaugunderson closed 3 years ago

beaugunderson commented 3 years ago

combines the best parts of the PRs by @pcafstockf and @napei (thank you both!) to enable hybrid CJS/ESM builds

closes #131

codecov[bot] commented 3 years ago

Codecov Report

Merging #138 (2d6ddc4) into master (d37f763) will not change coverage. The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##           master     #138   +/-   ##
=======================================
  Coverage   98.04%   98.04%           
=======================================
  Files           9        9           
  Lines         564      564           
  Branches       82       82           
=======================================
  Hits          553      553           
  Misses          3        3           
  Partials        8        8

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 c6670c6...2d6ddc4. Read the comment docs.

beaugunderson commented 3 years ago

@pcafstockf can you verify this works as you expect it to?

pcafstockf commented 3 years ago

@pcafstockf can you verify this works as you expect it to?

Looks good.
I did not notice that I had used mjs instead of esm (have to go back and fix my own GitHub projects) πŸ˜„

My only comment would be that using esnext instead of es2015 means you will be tracking a moving target.

FWIW: I cloned the bg-simple-modules branch, built and ran all the tests. Then diffed with my PR to review, and npm linked. Angular seems to use the esm as expected, and all my tests that use ip-address look good.

πŸ‘

beaugunderson commented 3 years ago

@pcafstockf oh great call re: esnext vs. es2015--I will change to es2015 (es2018 seems like an option as well, but unsure of the compatibility implications)

edit: ah, this is for the module property specifically, so basically anything post-es5 and pre-es2020 is the same, and es2020 adds support for dynamic import and import.meta; es2015 still makes sense to me as the value πŸ‘