Skip to content
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

move most options to Hooks #1878

Closed
wants to merge 7 commits into from
Closed

move most options to Hooks #1878

wants to merge 7 commits into from

Conversation

UziTech
Copy link
Member

@UziTech UziTech commented Dec 15, 2020

Description

Move most options to Hooks.

This PR might not get merged until v3 but I wanted to throw it out there and see what people think.

The idea behind this is to add a way for extensions to hook into different parts of marked to do things like (sanitizing, escaping, highlighting, etc.) and allowing us to move a lot of the options to extensions.

Example:

marked.use({
  hooks: {
    preprocess(markdown) {
      // Process markdown before marked
    }

    postprocess(html) {
      // Process HTML after marked is finished
    }

    sanitize(html) {
      // Sanitize HTML
      // replace sanitize and sanitizer options
    }

    smartypants(text) {
      // smarty pants
      // replace smartypants option
    }

    error(ex) {
      // Handle marked error
      // replaces silent option
    }

    escape(text) {
      // escape html entities for attributes
    }

    unescape(html) {
      // Unescape header text for id
    }

    languageClass(lang) {
      // set code class
      // replaces langPrefix option
    }

    encode(text) {
      // encode html entities for code blocks
    }

    headerId(text, slugger) {
      // create header id
      // replace headerIds option
    }

    cleanUrl(href) {
      // clean href
      // replace baseUrl option
    }

    mangle(text) {
      // mangle email address
      // replace mangle option
    }

    highlight(code, lang, callback) {
      // highlight code
      // replace highlight option
    }
  }
})

TODO

  • docs
  • create hooks tests
  • create extensions
  • map old options to new hooks and alert user to use extensions

@vercel
Copy link

vercel bot commented Dec 15, 2020

This pull request is being automatically deployed with Vercel (learn more).
To see the status of your deployment, click below or on the icon next to each commit.

🔍 Inspect: https://vercel.com/markedjs/markedjs/1q7bppk1q
✅ Preview: https://markedjs-git-hooks.markedjs.now.sh

@calculuschild
Copy link
Contributor

calculuschild commented Dec 19, 2020

I'm all for more extendability. This seems fine to me, just skimming through, but maybe you @UziTech could help me understand better what this does differently than the existing .use and .options?

@UziTech
Copy link
Member Author

UziTech commented Dec 19, 2020

For one it adds more opportunities suggested for extensibility in #1232 (comment) (namely preprocess and postprocess) it also allows more flexibility for these hooks.

The current highlight and sanitizer are overwritten if multiple extensions provide one. hooks are able to fallback to the next option if they return false. (sidenote: I was actually thinking about changing hooks, tokenizers, and renderers to take a next parameter that they can call instead of returning false to make it more like middleware)

Also it provides a better way than using options like baseUrl and headerPrefix.

@calculuschild
Copy link
Contributor

A middleware style would be interesting.... Would that be more performant too?

@janosh
Copy link

janosh commented Feb 17, 2021

Seems like marked is undergoing a lot of internal changes atm. Would now be good time to tackle async renderers (#458) or would it be better to wait until this is finished?

@UziTech
Copy link
Member Author

UziTech commented Feb 17, 2021

It would be a great time to tackle async renderers. This PR is mostly a proof of concept. Sort of to flesh out the idea and see what problems we run into.

@huaichaow
Copy link

want the lifecycle hooks badly.

currently working on an extension that needs cleanup work:

https://github.com/huaichaow/marked-toc-extension/blob/main/src/index.ts

@UziTech can we prioritize this PR?

@UziTech
Copy link
Member Author

UziTech commented Feb 8, 2023

@huaichaow ya I think the preprocess and postprocess hooks would be the most beneficial I'll start with just those. I'll try to get a PR for that this week.

@UziTech
Copy link
Member Author

UziTech commented Feb 11, 2023

@huaichaow I'm not completely done with it but #2730 is what I am thinking. Feel free to review it and leave comments.

@UziTech UziTech closed this Mar 22, 2023
@UziTech UziTech deleted the hooks branch August 26, 2023 03:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants