Use whacko instead of cheerio #191
Comments
|
There would be a few issues with this based on what is on the readme:
We want to modify the input as little as possible, rather than try to make the input into valid HTML5, which may not be what the user wants at all. |
|
SVG for emails? If you're referring to inline SVG, that's an HTML5 feature which is fully supported by parse5. Regarding fragments, this is from the whacko readme:
juice will need a major version bump, though, to accommodate the necessary API change (new method or option). |
|
I think it would be more practical for users to simply run their HTML through whacko before they give it to Juice to make sure the input to Juice is valid HTML, if that is something that is a requirement for them. Someone could easily wrap that pipeline into a single module if desired, and if published to npm it could help anyone with the same concern without breaking all the modules dependent on Juice's current behavior. In my opinion, one of the benefits of Juice is that it make as few changes as possible to what it is given. |
<style>
a { color:yellow }
table a { color:red }
</style>
<table>
<a href="#">link</a>
<tr><td></td></tr>
</table>Using htmlparser2/cheerio will result in a red link. Any web browser and parse5 will result in a yellow link. |
|
I'm working to make juice work with jQuery instead of cheerio (#215), and I'll have to add a bit of indirection so to make sure that calls can be adapted to be made by jQuery. I think we could work so to be able to support https://github.com/inikulin/parse5 at the same time, as an alternative, so that juice will have a default "dom manipulation library" (Cheerio) but it will also support jQuery and Parse5. Did you already check if parse5 have special requirements or if it works by just changing the require('cheerio') call? |
|
Why do you need to support all three? parse5 can be compiled with browserify/webpack to work in the browser. jQuery will require jsdom server side which is slower than parse5. |
|
cheerio works in the browser but it increase the library size of about 400KB.. the same is for parse5. Some people (us = mosaico.io) already needs jquery for different tasks so it is good if we can let juice works with 400KB less dependencies. And we don't want to use jquery on the server side, we want to use it in the browser. I didn't make benchmarks, but I expect jquery to be faster because it leverages the browser DOM handling than cheerio on the browser that is a lot of javascript lines of code to be compiled by the javascript VM... so it could be also a matter of speed. I hope I'll be able to produce some data wrt this aspect, later. BTW, I already did what I needed to make it work with jQuery... given it doesn't make juice more complex i hope jrit will merge it. I encourage you to make your PR to support parse5, too if you want that. As jrit told you each parser has its own "quirks" so there is no "best choice" for every scenario. |
|
What use cases do we have for a browser solution? I would think that it has a much lower priority than running on the server. |
|
https://github.com/voidlabs/mosaico At this time the css-inlining in the backend is one of our bigger issues because some users implement it in PHP, others in ruby, others in java, others node.js others in python or perl.. each one uses an existing css inliner and each existing CSS inliner have its own quirks and break most of our templates... and people think mosaico is broken while their css inliner is broken. Unfortunately CSS inlining is not an hard-science.. each one interpret inlining in a different way: |
|
I also have concerns about the size of the library whether it is on the client or server, it is pretty bulky and challenging to trim very much. The client library is definitely smaller though because it excludes all the stuff to fetch remote resources. Nobody has ever referenced speed of execution as a particular problem in the issues, so baring any really bad change, I don't think that should be a big deciding factor. 10% this way or that shouldn't matter. I would like to see how the jQuery/cheerio(or similar) refactoring works out. I don't see a pull request open right now. The most important issue to me if someone wants to propose another library is that it does the minimum possible to modify the existing HTML or XML that is being inlined. There are a bunch of other libraries that try to "fix" code and end up breaking document fragments that users need to be left how they are. There should be pretty good test coverage for that currently so breakages should let you know if a library would work or not. |
|
I'm working here: There are really few changes for jquery compatibility (.attr() instead .attribs[]) ). Another change is to be able to specify a different attribute than "style" because IE10/11 will not let you change the style attribute "as you like" and they will try to "fix" your content (add -ms- to some props, remove others), so in order to inline in the client you have to use a different attribute and then fix it later. Then I'm doing some refactoring that is related to dependencies isolation. I'm not changing in any way the default behaviour that uses cheerio, just refactoring the code so it can be partially used by jquery with no "references" to cheerio (just to a $ object that share some common behaviour from jquery, cheerio and other dom libraries inspired on jquery). |
|
BTW I think I'm going offtopic given this issue is about parse5 and I'm just working so to be able to let some part of juice to work with jQuery instead of cheerio. I'm sorry I took-over this issue. |
|
Given the changes @bago made for 3.0, I'm going to close this one. |
|
cheerio is switching to parse5 anyway. |
So that incorrect HTML can be corrected, thereby resulting in correct CSS selectors. Bad HTML templates are common from designers.
The text was updated successfully, but these errors were encountered: