pablo-abc / svelte-markdown

Markdown parser to svelte components
MIT License
359 stars 50 forks source link

feat: support for walkTokens #39

Open genericFJS opened 2 years ago

genericFJS commented 2 years ago

Currently svelte-markdown does not support walkTokens. This adds this feature (similar to the way marked itself does it).

genericFJS commented 2 years ago

I did some rudimentary tests concerning the impact of importing marked completely (as it's done here to get the walkTokens-function) on the bundle size. Bundle size of svelte template without SvelteMarkdown: ~38 KiB Bundle size with SvelteMarkdown without marked import (before this PR): ~96 KiB (SvelteMarkdown size: ~60 KiB) Bundle size with SvelteMarkdown with marked import (with this PR): ~106 KiB (increase: ~ 10 KiB). That means importing "marked" completely increases the "weight" of SvelteMarkdown by ~16% (for everyone, even walkTokens is not used). To keep SvelteMarkdown to a minimum size, one might consider not adding the walkTokens directly but providing a hook (e.g. afterLexing) as a parameter so the users who need to use walkTokens (or other post-processing) can add it themself.

genericFJS commented 2 years ago

Ok, I'm an idiot. There is already the parse event. Sorry.

0E9B061F commented 2 years ago

I'm not sure if the parsed event is a complete replacement for walkTokens. walkTokens allows you to transform the tokens before they're rendered. The parsed event allows you to inspect the tokens but any changes made aren't reflected in rendered content.

I'm using the approach you described in #38 to use marked extensions with svelte-markdown. The problem is that passing in a walkTokens function seems to have no effect, and I'm not sure why. In my attempts the function I passed in didn't even seem to be called.

For example, doing something like this, walkTokens seems to have no effect, despite my extensions working fine:

<script>
  import { extensions, walkTokens } from '../lib/wmd.js'

  marked.use({extensions, walkTokens})
  const options = marked.defaults
</script>

<SvelteMarkdown source={XXX} {options} />

Could this be re-opened? I'm not sure if I should bring this up here or elsewhere.

genericFJS commented 2 years ago

Yes, walkTokens will not be excuted, since SvelteMarkdown only calls the Lexer. Marked only seems to call walkTokens in its "main" script. My suggestion in #38 was only regarding extensions. I have not tried the parse event myself but you might be right: Since its only an outgoing event, the tokens are not reactive, even if they are changed. @pablo-abc Maybe this pull request has some merit after all?

pablo-abc commented 2 years ago

That means importing "marked" completely increases the "weight" of SvelteMarkdown by ~16% (for everyone, even walkTokens is not used).

This is the main reason why I'd also like to consider alternatives.

I'm using the approach you described in https://github.com/pablo-abc/svelte-markdown/pull/38 to use marked extensions with svelte-markdown. The problem is that passing in a walkTokens function seems to have no effect, and I'm not sure why. In my attempts the function I passed in didn't even seem to be called.

I'd have imagined this should work, maybe it's importing a different version of marked than the one used by SvelteMarkdown?

Another option I could see is making marked a peer dependency, so if you need the basic functionality SvelteMarkdown imports just the Lexer, but if you need extensions the user can import all of marked and add extensions. (Possibly providing a way to explicitly pass the Lexer as an argument to make sure you've extended the correct marked instance?)

I don't know what you think, @genericFJS

genericFJS commented 2 years ago

I'd have imagined this should work, maybe it's importing a different version of marked than the one used by SvelteMarkdown?

38 doesn't work with walkTokens since the Lexer does not call it. Only marked.parse/marked.parseInline calls it in the same way I implemented it.

This is the main reason why I'd also like to consider alternatives.

I agree but I don't know any easy ones.