Skip to content
This repository was archived by the owner on Feb 2, 2021. It is now read-only.

Conversation

@arrkiin
Copy link
Contributor

@arrkiin arrkiin commented Nov 17, 2017

To support the twemoji output for markdown-it-emoji, I had to figure out how to pass the described mapping function to the rules object of markdown-it.

Example from markdown-it-emoji:

md.renderer.rules.emoji = function(token, idx) {
  return twemoji.parse(token[idx].content);
};

So I extended the mdParser so that it receives rules that are specified within the config and passes them to the markdown parser. An example for a config file shows like this:

var emoji = require('markdown-it-emoji');
var twemoji = require('twemoji')

module.exports = {
  content: {
    permalink: ':slug',
    page: '/_content',
    generate: [ // for static build
      'get', 'getAll'
    ],
    isPost: false
  },
  parsers: {
    md: {
        use: [
            emoji
        ],
        rules: {
            emoji: function(token, idx) {
                return twemoji.parse(token[idx].content);
            }
        }
    }
  }
}

mdParser receives rules that are specified within the config and passes them to the markdown parser
@dahrens
Copy link
Contributor

dahrens commented Nov 19, 2017

You are right - currently this lacks flexibility.

But I personally don't like the idea to include parts of markdown-it's API in nuxtents configuration. Next to renderer rules markdown-it also supports inline token rules, block token rules and other things.

Currently use is already part of nuxtents configuration and it might be an exception as it makes it possible to load all available plugins. Initially I thought that writing a plugin for markdown-it would be sufficient, but in your case the plugin says: "use the plugin API of markdown-it to change my behaviour"

I'd suggest one of the following approaches.

Make it optional to pass the whole renderer to nuxtent

From the configuration point of view this might look like this

var md = require('markdown-it-emoji');
var emoji = require('markdown-it-emoji');
var twemoji = require('twemoji')

const myMD = md()
myMD.use(emoji)
myMD.renderer.rules['emoji'] = function(token, idx) {
  return twemoji.parse(token[idx].content);
}

// do whatever with the parser

module.exports = {
    // ...
    parsers: {
        md: myMD
    }
}

For this solution the plugins that nuxtent currently applies behind the scenes need to be applied on the given instance. This might lead to problems when someone already configured them in front.

Provide a hook with the constructed

Another option would be to support a hook method that receives the parser constructed by nuxtent.

var md = require('markdown-it-emoji');
var emoji = require('markdown-it-emoji');
var twemoji = require('twemoji')

module.exports = {
    // ...
    parsers: {
      md: {
          use: [
              emoji
          ],
          postParserConstruct: function (parser) {
              parser.renderer.rules[emoji] = function(token, idx) {
                  return twemoji.parse(token[idx].content);
              }
          }
      }
    }
}

This one should be quite easy to implement.

@arrkiin
Copy link
Contributor Author

arrkiin commented Nov 19, 2017

@dahrens, good point. Thank you for your suggestions. Regarding your description, I don't like the downsides of approach number one. Looks too much like a workaround. I think I will make a proposal for your second suggestion. In every software where I want to change or add behaviour, I really like the possibility of using hooks. This can offer more flexibility in terms of different renderer and configuration options and decouples the nuxtent configuration from all the other possible configurations.

You are right, the implementation for integrating that hook should be quite simple. I will do a first proposal next week.

@alidcast
Copy link
Contributor

alidcast commented Nov 19, 2017

Another option could be to configure it similar to nuxtent modules:

md: {
  plugins: [
     emoji
   ]
}

with rules: [use: Object, rules: Function]

md: {
  plugins: [ 
    [emoji, (token, idx) => {
       return twemoji.parse(token[idx].content);
    }]
  ]
}

@alidcast
Copy link
Contributor

Or to get rid of use option all together and only expose a hook like you guys mentioned

@arrkiin
Copy link
Contributor Author

arrkiin commented Nov 20, 2017

@alidcastano, in the first step I introduced the discussed postConstructionHook. Indeed it is also possible to integrate all the configuration mechanic of markdown-it in this function hook and therefore eleminate the use parameter in md. What is your oppinion on this topic @dahrens?

The corresponding configuration for the introduced extension looks like already mentioned by @dahrens.

var emoji = require('markdown-it-emoji');
var twemoji = require('twemoji')

module.exports = {
  ...
  parsers: {
    md: {
        use: [
            emoji
        ],
        postConstructionHook: parser => {
            parser.renderer.rules['emoji'] = function(token, idx) {
                return twemoji.parse(token[idx].content);
            }
        }
    }
  }
  ...
}

@dahrens
Copy link
Contributor

dahrens commented Nov 20, 2017

If we throw out use it would be straight forward to remove highlight, too.

The configuration should than look like that IMO:

module.exports = {
  // ...
  parsers: {
    md: parser => {
      // do whatever you want to do with the parser
    }
  }
}

I like that as it gives the user all possibilities to do whatever with markdown-it.

@alidcastano what is the problem with appveyor raising errors here? Looks like the fail is not related to this PR, but to the overall configuration.

@arrkiin
Copy link
Contributor Author

arrkiin commented Nov 20, 2017

@dahrens and @alidcastano, I thought about your ideas of stripping the code in parser.js. So I made some consolidations and moved the whole configuration code of markdown-it into an config object and integrated the plugin assembly into the new postConstructionHook. To check the behaviour I build up a configuration example with all the example-code that I found in the repository. For my local test case everything is working as expected.

const Prism = require('prismjs')
const externalLinks = require('markdown-it-link-attributes')
const emoji = require('markdown-it-emoji')
const twemoji = require('twemoji')
const markdownAnchors = require('markdown-it-anchor')

const plugins = [
    [
        markdownAnchors,
        {
            level: 1
        }
    ],
    [
        externalLinks,
        {
            target: '_blank',
            rel: 'noopener'
        }
    ],
    emoji
]

module.exports = {
  ...
  parsers: {
    md: {
        config: {
            preset: 'default',
            html: true,
            typographer: true,
            linkify: true,
            highlight: (code, lang) => {
                return Prism.highlight(
                    code,
                    Prism.languages[lang] || Prism.languages.markup
                )
            }
        },
        postConstructionHook: parser => {
            plugins.forEach(plugin => {
              Array.isArray(plugin)
                ? parser.use.apply(parser, plugin)
                : parser.use(plugin)
            })
            parser.renderer.rules['emoji'] = function(token, idx) {
                return twemoji.parse(token[idx].content);
            }
        }
    }
  }
}

@arrkiin
Copy link
Contributor Author

arrkiin commented Nov 20, 2017

@dahrens, I read your comment a second time and thought about it. Did you imagine a configuration like this?

const plugins = [ ... ]
module.exports = {
  ...
  parsers: {
    md: () => {
      const parser = markdownit({...})
      plugins.forEach(plugin => {
        Array.isArray(plugin)
          ? parser.use.apply(parser, plugin)
          : parser.use(plugin)
      })
      parser.renderer.rules['emoji'] = function(token, idx) {
        return twemoji.parse(token[idx].content);
      }
      return parser
    }
  }
}

@dahrens
Copy link
Contributor

dahrens commented Nov 20, 2017

There is currently at least one features in nuxtent that relies on markdown-it to be configured as it is. This is how markdown-it-anchors gets configured based on the anchor level provided in the nuxtent content configuration.

I'd suggest to keep the initial configuration of the parser where it is - but instead of merging this with incoming use parameter I would just provide the postConstructionHook to do whatever with the parser.

@alidcastano are there any other parts of the code, that rely on the config passed to markdown-it initially or can we move this configuration as is into the nuxtent configuration?

I'll add commets to the changeset to make this more clear.

highlight
})

const plugins = [
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This part should stay here.

{
level: [anchorsLevel]
}
]
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

But we must not merge this with incoming use.

const parser = markdownit(config)

if (postConstructionHook !== undefined ){
postConstructionHook(parser)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Because every other plugin might get loaded in the hook afterwards.

import yamlit from 'js-yaml'
import markdownit from 'markdown-it'
import markdownAnchors from 'markdown-it-anchor'

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I like the idea to pass the configuration from the outside, but I'm currently not aware of all possible side effects.

@alidcast
Copy link
Contributor

I think it should work similar to how Nuxt uses webpack but allows users to extend it. Nuxtent can use markdown it (provide default configuration along with anchor plugin - yes @dahrens that is the only part that uses it directly).

@alidcast
Copy link
Contributor

alidcast commented Nov 21, 2017

Yeah the build is failing due to previous push. Was waiting for @medfreeman module refactor to check it but he's been busy with work so I'll see if I can find time this week to get this all working

@arrkiin
Copy link
Contributor Author

arrkiin commented Nov 21, 2017

Ok, I reintroduced the default config object, I named it baseConfig, for markdown-it and the default plugin markdownAnchors with the corresponding anchorsLevel parameter. For flexibility purposes the optional config parameter overrides the specified configuration options in baseConfig with values set in nuxtent.config.js.

@arrkiin
Copy link
Contributor Author

arrkiin commented Nov 21, 2017

During studying the nuxt.js configuration docs I realized, that I could change the mechanism of extending the config so that it behaves exactly the same way like the postConstructionHook. So this looks more consistent. What is your opinion on this?

  parsers: {
    md: {
        configureHook: config => {
            config['highlight'] = (code, lang) => {
                return Prism.highlight(
                    code,
                    Prism.languages[lang] || Prism.languages.markup
                )
            }
        },
        postConstructionHook: parser => {
            plugins.forEach(plugin => {
              Array.isArray(plugin)
                ? parser.use.apply(parser, plugin)
                : parser.use(plugin)
            })
            parser.renderer.rules['emoji'] = (token, idx) => {
                return twemoji.parse(token[idx].content);
            }
        }
    }

@alidcast
Copy link
Contributor

alidcast commented Nov 21, 2017

Hi @arrkiin thanks for devoting time to this. The last API you proposed seems too complicated. Since Nuxtent already uses markdown-it, we can make things simpler for users.

What do you think of this:

{
  md: {
   extend: Function, 
    plugins: 2D Array - [plugin: Object or Array, options: Object, customRules: Function]
  }
}

And can be used as such:

md: { 
 extend (config) {
    config['highlight'] = (code, lang) => {
           return Prism.highlight(
               code,
               Prism.languages[lang] || Prism.languages.markup
            )
        }
  },
  plugins: [
     [[emoji, (parser) => {
          parser.renderer.rules['emoji'] = (token, idx) => {
             return twemoji.parse(token[idx].content);
          }
     }]
 ] 
}

The first part is similar to what you had, except API copies how Nuxt does extends Webpack so that usage is familiar with users.

The second part, rather than having users initialize plugins themselves, we do it for them, and pass the parser as an extra parameter for advanced customization. While the usage is similar, now customizing markdown-it seems less daunting :)

@arrkiin
Copy link
Contributor Author

arrkiin commented Nov 22, 2017

I think I would split the parser-extension-callback off the plugins array. What about the variant below? There we have customize(parser) as a similar mechanism like the extend(config) for customizing the parser after it was created. It's clean to write and linguistically understandable. After doing some experiments with linkify and emoji, I think this is quite ease to use and offers all the flexibility to customize the behaviour of markdown-it and all the available plugins.

  parsers: {
    md: {
        extend(config) {
            config.highlight = (code, lang) => {
                return Prism.highlight(
                    code,
                    Prism.languages[lang] || Prism.languages.markup
                )
            }
        },
        plugins: [
            emoji,
            [ externalLinks, { target: '_blank', rel: 'noopener' } ]
        ],
        customize(parser) {
            parser.linkify.tlds('onion')
            parser.linkify.add(... custom code to recognize and handle twitter handles ...)
            parser.renderer.rules['emoji'] = (token, idx) => {
                return twemoji.parse(token[idx].content);
            }
        }
    }
  }

@arrkiin arrkiin changed the title Extend mdParser to pass markdown-it rules Modify mdParser to offer config extension like nuxtjs and expose a hook for customizing of created parser Nov 22, 2017
@arrkiin arrkiin changed the title Modify mdParser to offer config extension like nuxtjs and expose a hook for customizing of created parser Modify mdParser to offer config extension like nuxtjs and expose an hook for customizing of created parser Nov 22, 2017
@alidcast
Copy link
Contributor

API looks good to me :) @dahrens What do you think?

@arrkiin This PR will have to wait till I can go back and fix build for release

@dahrens
Copy link
Contributor

dahrens commented Nov 22, 2017

Delicious! ;)

@alidcast alidcast merged commit c2e2b30 into nuxt-community:master Dec 25, 2017
@arrkiin arrkiin deleted the rules branch December 25, 2017 10:42
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants