mafintosh / dns-packet

An abstract-encoding compliant module for encoding / decoding DNS packets
MIT License
201 stars 70 forks source link

Replacing ip with @leichtgewicht/ip-codec #67

Closed martinheidegger closed 3 years ago

martinheidegger commented 3 years ago

The ip npm package references both buffer and os which makes it kind of stressful to use in a browser setting. This PR uses the extracted encoding functionality of node-ip through @leichtgewicht/ip-codec. Reducing a bit of performance overhead in the process.

silverwind commented 3 years ago

That package still depends on a Buffer polyfill to be available from what I gather. Wouldn't it maybe be better to use ArrayBuffer instead, which is always available in all environments?

martinheidegger commented 3 years ago

@silverwind thank you for the attention. I assume with "that package" you mean @leichtgewicht/ip-codec? It uses Buffer only in the test not in general code.

silverwind commented 3 years ago

Ah, I misread your code. I see it's using Uint8Array in the code which indeed is browser-compatible, so this should be fine.

mafintosh commented 3 years ago

Awesome @martinheidegger - Is your package using the same regexes etc as the previous one?

martinheidegger commented 3 years ago

Yes. Same Regexes, same algorithm. Same Tests. A bit of cleanup here and there. Full coverage: https://github.com/martinheidegger/ip-codec/runs/2960741507?check_suite_focus=true#step:5:64