-
-
Notifications
You must be signed in to change notification settings - Fork 114
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
feat: add HTML & XML Inspectors API using Nokogiri #546
Conversation
Your Render PR Server URL is https://bridgetown-api-pr-546.onrender.com. Follow its progress at https://dashboard.render.com/static/srv-c9omngr0tnugec4jg9s0. |
Your Render PR Server URL is https://bridgetown-beta-pr-546.onrender.com. Follow its progress at https://dashboard.render.com/static/srv-c9omnhb0tnugec4jg9vg. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This looks incredible. I have been attempting to do something like this performantly (without JS of course) similar to what 11ty has and this is way better than I had even dreamed. This will open up the possibility to do some really cool transformations now. Definitely an area that plugins can flourish!
I left a few nitpicks in the docs in a hope you may like one or two of the suggestions.
Amazing job on this @jaredcwhite 👏
module RunInspectors | ||
def self.call(resource, inspectors) # rubocop:disable Metrics/CyclomaticComplexity | ||
return resource.output if !inspectors || | ||
!resource.destination&.output_ext&.starts_with?(".htm") || |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
warning: I don't particularly have a concrete example use case for this and am just thinking out loud
How hard would it be to allow this to work for XML too? I am thinking about sitemaps and app manifests. If the answer is one or two lines of code changed, I could see it being useful to add. ¯_(ツ)_/¯
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nokogiri ships an XML inspector so I imagine it should be possible.
https://nokogiri.org/tutorials/parsing_an_html_xml_document.html#from-a-string
inspect_html do |document| | ||
document.query_selector_all("article h2[id], article h3[id]").each do |heading| | ||
heading << document.create_text_node(" ") | ||
heading << document.create_element( | ||
"a", "#", | ||
href: "##{heading[:id]}", | ||
class: "heading-anchor" | ||
) | ||
end | ||
end |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
😍 This is FANTASTIC
All resources which result in HTML output (rather than JSON or some other format) will be procssed through any defined inspectors. For greater performance and fidelity, the Nokogiri document for a single resource will be the same across all inspectors (rather than instantiating a new Nokogiri document for each inspector). | ||
|
||
{%@ Note type: :warning do %} | ||
Nokogiri [relies on a C extension](https://nokogiri.org/#guiding-principles_1) which in turn uses `libxml2`, so generally you should see very fast performance unless the number of resources in your project is extremely large. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nokogiri [relies on a C extension](https://nokogiri.org/#guiding-principles_1) which in turn uses `libxml2`, so generally you should see very fast performance unless the number of resources in your project is extremely large. | |
Nokogiri [relies on a C extension](https://nokogiri.org/#guiding-principles_1) which in turn uses `libxml2`. You should see fast performance unless the number of resources in your project is monstrous. |
Just a suggestion 🙂
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe enormous? Huge?
category: plugins | ||
--- | ||
|
||
The HTML Inspectors API, added in Bridgetown 1.1, provides a useful and safe way to manipulate the HTML output of your resources. Safe because instead of using string manipulation, regular expressions, and the like—which is prone to error—you'll be working on real node trees. This is thanks to [Nokogiri](https://nokogiri.org), a Ruby gem which lets you work with a DOM-like API directly on HTML documents. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Safe because instead of ...
I think the beginning of this sentence needs a little love. What is safe? Why would it not be? etc.
Thanks @andrewmcodes for taking a look and for your enthusiasm. It's been a super fun feature to work on and implement…once again gotta give Mr. Shoelace himself @claviska a big shoutout for coming up with the feature idea as he works on his own docs engine. I'm about to be OOTO for a few days, but I'll take a close look at your feedback as soon as I can. 🙏 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is sexy and looks great and is a surprisingly small amount of code, great work!
module RunInspectors | ||
def self.call(resource, inspectors) # rubocop:disable Metrics/CyclomaticComplexity | ||
return resource.output if !inspectors || | ||
!resource.destination&.output_ext&.starts_with?(".htm") || |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nokogiri ships an XML inspector so I imagine it should be possible.
https://nokogiri.org/tutorials/parsing_an_html_xml_document.html#from-a-string
fix typo Co-authored-by: Andrew Mason <[email protected]>
Co-authored-by: Andrew Mason <[email protected]>
Thanks again @andrewmcodes for the feedback! I'll clean up the first few sentences, and I'll also look into supporting |
Closes #536.