-
Notifications
You must be signed in to change notification settings - Fork 68
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
incompatible with node 10 #68
Comments
Seeing the same issue on node 10.06 on OSX:
|
Having the same issue with node 10.6.0 on Windows 10... Any updates as to when this may be resolved? I had to start using Saxon-HE to get going but I'd much rather use this. |
Still occurring on node 10.7 on OS/X as well. Currently has me stymied. Are there any workarounds? |
@RaviDesai this will require an actual code fix - my current "workaround" is to keep my project at a lower version of node :( |
@eorsi Thanks. That's my "workaround" as well I guess. :( |
For node 10 try this fork
|
Thanks @alexdee2007. Your fix seems to work for me on node 10.9.0 (and also on node 8.11.2) on Mac OS. 🎉 Maybe @albanm would like a PR? |
Hello @MartijnR, you are welcome. Unfortunately there is also dependent repo libxmljs-mt, which is also need to be fixed. So I hope @gagern and @albanm will do it asap. |
HI, Node v8.12.0, NPM v6.4.1
|
@belfour, that looks like a completely different issue. Would be good to move it to its own issue, I think. |
The node libxslt package is currently not compaltible with node 10 (see albanm/node-libxslt#68).
@alexdee2007 Trying to use your fork with electron. electron-rebuild works, but I get runtime error "The specified procedure could not be found.". |
Hi @albanm - I know you're not very invested in this project any more, but I'd like to ask you about it, since I'm blocked from upgrading Node.js in my project (by way of libxmljs-mt, which is now abandoned). Do you think that it is possible for me to rewrite parts of it to work with libxmljs, or is it too fixed to the libxmljs-mt interface? I'm not a C or C++ developer, although I understand some of the code. Your library is good, so I'd prefer to work with this than go back to passing files to xsltproc :-) |
@gagern was the one putting some work into this after some report of hard bugs caused by a lack of concurrency management in libxmljs. He wrote libxmljs-mt and included it into this project to solve this. node-libxlst worked fine before that, except for these specific bugs. I don't think there were that many changes done here to switch from libxmljs to libxmljs-mt. Maybe only the references to libxmljs::WorkerSentinel, cf https://github.com/albanm/node-libxslt/blob/master/src/node_libxslt.cc#L70 For minimal work I suppose this could be done:
The resulting fork would have a simpler code. But it would fail in one major way: if the processed stylesheets include other stylesheets the resulting IO operations will be performed in a blocking synchronous function. Maybe forking libxmljs-mt, and updating it from libxmljs to support latest node would be a better solution. I have no idea of the complexity of the task. |
Thanks for such a quick response, @albanm - yes, I was looking at this and I understand why you have done it this way and why you used libxmljs-mt - it makes sense. The libxmljs-mt fork is unfortunately idle, there are some pull requests to solve this and try to make a working build for Node.js 10 (this is the real problem) but it seems that there's no movement. I have asked about the changes to libxmljs-mt, there is some interest but little has happened. Your libraries are two of the three dependent on it, so there's not much traction for changes... Thanks again! |
Right, I'm back: I reread the thread on libxmljs-mt, and followed the instructions here, to get a Node.js 10.x compatible build of libxmljs-mt. That works! Which is great. However, getting a build of node-libxslt seems to be a little harder. I've repointed the dependency for libxmljs-mt to my local build, but I got the error below:
So this relates to the C bridge and nan, using some calls that are deprecated with Node.js 8.x and unsupported by Node.js 10.x. So I'm looking at that - and learning a lot! |
@albanm The issue is in stylesheet.cc: With this, it will build and work with Node.js 10.x. There are other deprecations, so I will need to look at those. If I fork and fix, would you like me to submit the changes back to you? |
Yes, pull requests are always welcome. |
Except if you wish to maintain the fork from now on.. In this case I will gladly transfer ownership of the npm name :) |
With great power comes great responsibility! Ok, we can look at this, because you have done wonderful work with this module and it will be your 'calling card' for years to come. But if you are tired of it, maybe it can help too. Thanks again for responding! |
details at: albanm/node-libxslt#68 now using a fork from: https://github.com/alexdee2007/node-libxslt
I don't want to come off as prick, but is this being worked on? Is it possible to help out in some way? |
I am having the same issue: |
Pending PR: #73 |
issue can be closed, I think |
As noted here: gagern/libxmljs#12
I too cant get past node 9 in :
libxmljs-mt seems stale.
The text was updated successfully, but these errors were encountered: