-
Notifications
You must be signed in to change notification settings - Fork 51
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 extensions support #38
Conversation
Added a test and documentation. |
types/index.d.ts
Outdated
/** | ||
* Parameters for the function [marked.use](https://marked.js.org/using_pro#use) | ||
*/ | ||
use?: Parameters<typeof marked.use> |
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.
Since this can add renderers, if you're using TypeScript it would conflict a bit with the Renderers
type above. Maybe extending said type with something like & Record<string, InstantiableSvelteComponentTyped>
would work?
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.
Since we don't use the renderer either way, maybe we should discourage passing one with the use parameter:
use?: Omit<marked.MarkedExtension, 'renderer'>[]
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.
I just realized you were not talking about the use-type but the the Renderers type (just as you've written…). And there you are right, of course. The type would have to be added.
marked.setOptions(combinedOptions) | ||
marked.use(use) | ||
|
||
lexer = new Lexer() |
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.
Since we're using all of marked now, is there a difference on build size between importing Lexer
vs doing new marked.Lexer()
?
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.
I did some rudimentary tests with the svelte template where I added a SvelteMarkdown component with an extension (as in the first comment of this PR):
There were no differences in files size (after yarn build
) when using it as is VS removing the Lexer/Slugger imports entirely and writing
slugger = source ? new marked.Slugger : undefined
and
tokens = isInline ? marked.Lexer.lexInline(source) : marked.Lexer.lex(source)
Removing the imports seems to have no effect on build size (and could just be done for cosmetic reasons).
This looks great! Thanks a lot! I just left a couple of questions |
Make it more specific by omitting the never-used renderer
While testing a bit more I found a huge problem with my approach: by using |
I think we should close this pull request, since <script>
// [… as in first comment]
// let marked save extension information to global options:
marked.use({ extensions: [latexTokenizerExtension] });
// use global marked options as parameter for SvelteMarkdown-component
const options = marked.defaults;
</script>
<!-- works without this PR -->
<SvelteMarkdown {source} {options} {renderers} /> |
Closing in favor of manually doing it by passing the appropriate options. |
Quick and dirty: Add "use" parameter which allows extensions (see #17 ).
Example usage (replaces token
[[latex…]]
with a coloredspan
):File
App.svelte
Imported file
LatexMarkdown.svelte