buglabs / node-xml2json

Converts XML to JSON using node-expat

808 stars 209 forks source link

Upgrade node-expat to ensure compatibility with node 10 #166

Closed Clement134 closed 5 years ago

Clement134 commented 6 years ago

Versions of node-expat oldest than 2.3.17 are not compatible with node 10: https://github.com/astro/node-expat/pull/190

@c4milo is it possible to merge this pull request and create a new release with all the pending fixes since the latest version ?

c4milo commented 6 years ago

I will cut a new release tomorrow. Thanks!

3z3qu13l commented 6 years ago

@c4milo Would be great to have 0.11.3 ;)

maximnara commented 6 years ago

Hey! Would it be new version with upgraded node support soon?

c4milo commented 6 years ago

I will merge this PR and https://github.com/buglabs/node-xml2json/pull/159 once Travis passes. It looks like we have some memory leaks, most likely in dependencies.

Clement134 commented 6 years ago

@c4milo by memory leaks you refer to this error message: "The following leaks were detected:SharedArrayBuffer, Atomics, WebAssembly" ?

Those are not real memory leaks, but false positive due to differences between nodejs version used for tests, and the nodejs version handled by the version of lab (see https://github.com/hapijs/lab/issues/818). I think the purpose of #159 is precisely to fix that.

c4milo commented 6 years ago

Why does it keep showing up in #159 Travis ouptput if it is fixing it?

Clement134 commented 6 years ago

837).

3z3qu13l commented 6 years ago

Hello @c4milo , Can we merge this PR since the memory leaks is in a dev dependency and not in the code itself ?

Clement134 commented 5 years ago

I will merge this PR and #159 once Travis passes. It looks like we have some memory leaks, most likely in dependencies.

I have upgraded lab to the latest version, now all the tests on Travis are green. @c4milo could you merge this PR ?

c4milo commented 5 years ago

Done. Thanks @Clement134!