search

w0rm / gulp-svgstore

Combine svg files into one with symbol elements
https://www.npmjs.com/package/gulp-svgstore
645 stars 33 forks source link

Reconsider xml library #9

Closed w0rm closed 9 years ago

w0rm commented 9 years ago

I've chosen libxmljs but just found this fork by @lemming that replaces it with cheerio.

Unfortunately changes by @lemming cannot be merged because they contain code style changes. Does anybody know what are benefits of this? Is it because of better performance or because its easier to install cheerio than libxmljs?

lemming commented 9 years ago

The main and the only reason for fork was that we had issues with building libxml under Windows in our project. There is no difference in performance between libxmljs and cheerio in our case (200-250ms for 41 small svg icon files), but I suspect other cases would show it.

w0rm commented 9 years ago

@lemming thanks for the feedback! My past coworker @fetis was able to install it on Windows, but he had some troubles. I'll do some benchmarking and then decide if there is a need to migrate to cheerio or some other framework.

w0rm commented 9 years ago

@lemming I tried to benchmark libxmljs vs cheerio by running them over a bunch svgs from Open Iconic collection.

It turns out that libxmljs is roughly twice as fast as cheerio (162ms vs 293ms). So I'm not sure that I should replace current library with cheerio.

w0rm commented 9 years ago

On the other hand I'd like it to be pure node solution that is easy to install.

I probably have to consider some streaming parser like this one: https://www.npmjs.org/package/sax

davidtheclark commented 9 years ago

I'm unable to use the plugin right now because of the Python dependency from libxmljs --- so I'd be all for another solution.

w0rm commented 9 years ago

@davidtheclark I see. So at the moment you can use @lemming fork and I will try to do it with streaming parser that should be really fast.

w0rm commented 9 years ago

@lemming, @davidtheclark I have decided to use cheerio, published new version on npm.

davidtheclark commented 9 years ago

Great. I will give it a try soon.