-
Notifications
You must be signed in to change notification settings - Fork 0
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Fix build on Node 12+ #9
Comments
Hey @mattcollier, thanks for awesome work. Any plans to fix this issue? Do you need any help with this? |
@dmfilipenko-affinidi Hello, this module was created at a time when this native implementation was more performant than JS. This module has fallen into disuse since the JS/V8 implementation caught up performance wise. |
Maybe a WASM build would fill whatever gap is left? |
@danbri If someone is interesting in tackling this, I'd suggest first building the native code on an older Node.js, and benchmarking that against the pure js version on a modern Node.js. I have no idea what the result will be. I'd put my money on about even or a js edge. ;-) Someone with more experience with the modern native API could probably update this code fairly quickly too. I'd also suggest revisiting benchmarks and profiling on the pure js code before going to far down the native code route. There might be algorithmic gains there for less effort. (There's at least one outstanding patch for a small optimization.) |
This module is no longer building on node 12+
See: https://travis-ci.org/github/digitalbazaar/rdf-canonize/builds/731728025
The text was updated successfully, but these errors were encountered: