-
Notifications
You must be signed in to change notification settings - Fork 109
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
Adds options object and syntax highlighting API #28
base: main
Are you sure you want to change the base?
Conversation
src/index.js
Outdated
last = 0, | ||
chunk, prev, token, inner, t; | ||
export default function snarkdown(md, options = {}) { | ||
const highlight = options.highlight; |
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.
Idea: since parsing is synchronous, rather than having to wrap parse()
in a function to collect options
, we could set a global highlight
value that gets used by the other functions. Right now this PR adds 100b, I'm thinking with that change we could trim it down to keep things below 1kb. Thoughts?
Good call. Is this what you were thinking? Technically, it's still just over 1kb at 1027b according to your build script (which I'm totally stealing for my libs, btw). I removed the After all this: perhaps from a perf standpoint, it would be better to let a syntax lib like Prism or Highlight do their thing asynchronously after page load. FWIW, I do still like the ease of a highlight option, but I might skip it for SSR perf on this project. Example usage with Prism: export default ({ string = '' }) => (
<div className='karla markdown' dangerouslySetInnerHTML={{
__html: md(string, {
highlight: (str, lang) => prism.highlight(str, prism.languages[lang || 'markup'])
})
}} />
) |
Also, happy to squash those commits too, sorry to spam the history. |
More or less yup. I can always tweak sizing a bit to get things down. What are your thoughts on this in the context of your comment about client-side lazy rendering? That's personally how I'd be setting things up, though rather than |
Personally I think most projects could do fine with lazily highlighting client-side, though the perf hit is probably only very slight. I like the idea of converting to VDOM, but I'm also not super against Do you find a lot of utility with converting to VDOM? Or would you recommend that most people just lazily highlight on the client? I'm leaning towards the latter for now, but am really interested in what could be done with markdown-to-VDOM. Didn't know that was a thing! BTW thanks for taking the time on this, I know you must be busy! |
@estrattonbailey any update on this one ? |
@ritz078 I decided to do the client-side highlighting, actually. There are probably good reasons to do it on the server, but I can't think of any off the top of my head. I would recommend going the VDOM route as Jason has linked to on the Preact site. I've also worked with remark since this PR and its remark-react plugin and it's a nice experience. They also have a VDOM highlighting library that I haven't tried but would like to remark-react-lowlight. That said, I imagine this PR can be closed, but I'll leave that up to @developit. |
Hey! I'm working on a project that calls for syntax highlighting so I figured I'd add it and see what you think.
Small sticky point with this PR: AFAIK, most highlighters escape HTML on their own, so if a user provides a
options.hightlight
function, I'm opting to skip theencodeAttr
call and pass on that responsibility to the highlighter. If the code is pre-escaped, the highlighter re-escapes them i.e.>
fromsnarkdown
becomes<span className>&</span>gt<span className>;</span>
.Anyway, I'm just installing from my fork for now. Let me know if you want this refactored or anything else cleaned up. Also, if you feel this doesn't fit with this library, feel free to close this 👍🏼