allow option to attach target="_blank" on links

[abhishiv]

hard to do outside

[developit]

Seems like a good idea, though maybe a little bit presumptive? An option seems good. Also, this library actually uses a little-known method String.prototype.link to generate HTML links with proper encoding, and there's no option to add attributes. Will have to switch to an inline creation in order to get the target attribute. https://github.com/developit/snarkdown/blob/master/snarkdown.js#L82

FWIW I tend to pipe the output of this lib into something that converts the HTML to Virtual DOM, which makes it super easy to attach arbitrary behavior (and attributes) to any element.

[ShimShamSam]

I would go a step further and ask that we can also modify the link's rel attribute so we can add things like nofollow, noopener, etc...

[developit]

@ShimShamSam yup - I had originally tested out a target change, but then realized target isn't good enough on it's own since that's actually a vulnerability without proper rel set.

I want to collect some use-cases here: are you guys piping the HTML straight into the DOM, or through something like a VDOM renderer?

[ShimShamSam]

I use templates for a project, not a virtual DOM, so I'm ultimately setting innerHTML directly.

[developit]

Ah ok, that helps me understand. Maybe we can add something really low-level like a "chunk processor" API. Just thinking of ways to keep the size down. Issue I ran into immediately after adding target="_blank" was that it doesn't make sense for same-domain and #anchor links.

[developit]

FWIW here's a function that would do it after innerHTML:

div.innerHTML = snarkdown(md);
retarget(div);

function retarget(el) {
  if (el.nodeName==='A' && !el.target && el.origin!==location.origin) {
    el.setAttribute('target', '_blank');
    el.setAttribute('rel', 'noreferrer noopener');
  }
  for (let i=el.children.length; i--; ) retarget(el.children[i]);
}

Personally I think it's nice to be able to take advantage of DOM traversal here.

[abhishiv]

@developit yeah same for me, setting innerHTML.

What about a url builder?

  md.parse(str, {
    linkBuilder: function(href) {
      return {target: "_blank", rel: "nofollow"}
    }
  })

[tunnckoCore]

Yea maybe we can allow kinda "plugin" api. Just a function or array as second argument which will be passed with the token in final else {} (after all the if else-es)

edit: @abhishiv seems like a too much code :D but yea, maybe

[developit]

@abhishiv I like the idea, but it's probably impossible to convert tokens to nice function names like that given the filesize.

Currently it'd be like:

snarkdown(str, (md, html) => {
  return html.replace(/^<a /, '<a target="_blank" rel="nofollow" ')
});

edit: @tunnckoCore pretty much exactly what I was thinking haha. just would do it after all the blocks.

Just thinking though, you could pretty much apply that transformation anyway:

let html = snarkdown(md).replace(
    /(<a href="(https?:)?\/\/.*?")>/g,
    '$1 target="_blank" rel="nofollow">'
);

edit 2: here's Showdown's target="_blank" plugin

[tunnckoCore]

Mm, maybe. I was thinking to pass the token array to that transform function, too. But, not sure because... nah.. maybe would need too much docs which token is what :laughing:

Just thinking though, you could pretty much apply that transformation anyway:

Yea.. So it just ends up that it should be enough to say (and emphasize somewhere) that Snarkdown does not do anything so magical and relies absolutely on RegExps, and so you can just continue to do regex things after it returns its result, haha.

edit: but yea, people likes "plugins", haha

[ShimShamSam]

If links are already being processed by their own logic (currently String.prototype.link), it makes sense to allow devs to hook into it. Running a regexp replace on the entire output to fix links seems inefficient.

Why not just add some global options to the parse function? Something like:

snarkdown(text, {
    links : {
        nofollow : true,
        noopener : true,
    }
});

[developit]

@ShimShamSam String.protoype.link is provided by the browser, it's not modifiable. The reason we can't add global options like that is because the size impact would be enormous. Just for the snippet you posted, we'd have to add serialization support for arbitrary attributes, classify tokens as named entities ("link"), etc - essentially that would require an AST, and this module just can't afford that. The reason snarkdown is small is because of design decisions, not just compact code - breaking out of those design decisions/constraints makes it just another Markdown parser, and in that regard a fairly poor one.

Now that I'm maintaining a fair number of these micro-modules, I've been finding it difficult to explain the importance of constraints. Everyone has a set of features they want, and no two people want the same set. The problem is, a project with the goal of being minimal simply cannot accommodate the superset of features that would provide everyone with their desired features - that would make it a normal module, not a minimal one. There are already excellent Markdown parser modules out there that do these things better than Snarkdown - it's important to understand that small modules are by nature an exercise in tradeoff management.

[ShimShamSam]

String.protoype.link is provided by the browser, it's not modifiable.

I know that. What I meant was we already have branching logic that says "this is a link, do x". Being able to hook into that should be as easy as setting a few options. Are you open to a pull request so I can take a stab at it?

[developit]

I'm always happy to review a PR, just I don't want to give you the impression it's going to be something we can necessarily merge. There's very little that can be done to mitigate the file size hit it will incur, which I am fairly certain makes it a non-starter.