Refactor as ES6 Modules and Classes

[saragarmee]

What pain point are you perceiving?.
Marked at present isn't really compatible with ES6 Modules and Imports
As a result, a lot of cruft is added because we need to use external tools to use something that is pure JS and completely compatible with the browser. There is nothing in this tool that requires anything node specific.

Describe the solution you'd like
I've already taken the liberty to refactor this code into modules and classes. The code economy (i.e. savings in terms of KBs used) is fairly limited but there are other significant advantages that I believe ought to be taken into consideration.

Broken down into classes and modules, the entire marked.js file breaks down into 7 files with a total line count of 1544 lines, vs the current 1605 lines.

filename	line count
block.js	119 lines
marked.js	130 lines
helpers.js	163 lines
parser.js	178 lines
renderer.js	190 lines
lexer-inline.js	371 lines
lexer.js	393 lines

• (these numbers include comments) I broke the lexer into 2 components to try to keep file sizes down

Besides saving a few lines, this approach is also (at least to my mind), far more maintainable. Individual aspects are essentially broken out into their respective files and can be upgraded or modified from time to time without the need to update the monolithic marked.js file. Furthermore this approach doesn't pollute the global space in the way that the present code base does.

I'm sheepish about submitting a PR at the current time because there is so much other cruft external to marked.js in this repository that appears to be relying on a monolithic IIFE. Therefore I am wondering if I post what I have as a gist, or possibly a repo over in my own repositories, if the developers would be interested in taking a look at it and begin a discussion on what it would take to migrate over to the ES6 modules system, which is now a standard and widely supported, even by node.js.

The biggest blocking issue would likely be the unit tests, which would probably have to be completely rewritten. After that, I'm not even certain what other externalities there may be, much of this code goes above my head. Obviously a refactor of this size would be best handled as a new major version so as not to break the innumerable install base.

Thanks for reading this!

[styfle]

There is a proposal already: #1288

which is now a standard and widely supported, even by node.js

Node ESM support is experimental and may change. It also requires a different file extension .mjs.

From the docs: "Stability: 1 - Experimental. This feature is still under active development and subject to non-backward compatible changes or removal in any future version"

[saragarmee]

Oh that was fast, thank you. I searched and I didn't see that proposal. Anyways, I'll fork, add my code and seek commentary over in the official proposal.

[joshbruce]

@saragarmee: We are also primarily focusing on compliance with the Markdown standards we support and trying to maximize backward compatibility until the 1.0 release - at which point we may start changing the internals more.

If you fix anything in your fork, please do push back up. 😀

[saragarmee]

If I find anything I promise I will, but like I said, there's a lot of your code that is going way over my head. Regex's in particular give me headaches.

My long term plan is to integrate marked or some version thereof into telepathic-elements core.
Telepathy is a project to simplify webcomponents in general. I didn't like injecting a ton of HTML markup into my JS. So I created a templating engine to allow you to specify the HTML for the custom component in it's own separate HTML file, then bind to the custom element using Javascript's template literal syntax.

e.g.

cow-element.js

class CowElement extends TelepathicElement{
    constructor(){
      super();
      this.says = "moo!";
   }
}

cow-element.html

<template>
The cow says <b>${this.says}</b>
</template>

usage is like...

<cow-element></cow-element>

So far this is working remarkably well and I can do stuff now in a few lines that would take hundreds of lines of react, such as two-way databinding. But I wanted to give my users the ability to specify the template in markdown while still keeping the databinding capabilities. So I started to explore the various markdown libraries out there and decided to integrate marked.js into the core of telepathy. This is my ongoing work, but initial results are promising.

cow-element.md

The cow says *${this.says}*

So my hope here is that this will be very fruitful and ongoing because I see this project as a major part of my plans for telepathy going forward.

[joshbruce]

@saragarmee: Interesting, I like your project. Sounds interesting so far.

No worries on the going over your head bit, that is also one of the missions of Marked - help folks learn JS and REGEX things. In other words, we'd be here to help - well they would be better at helping. lol

For this project I'm more valuable with architecture, roadmapping, and community stuff. 😀

Definitely keep us posted!

Have you looked at VueJS? What you're describing feel familiar from there. Just wondering if using them as a reference like you did with React would be beneficial.

[saragarmee]

Thanks, I wasn't coming here with the intention of drawing attention to my own custom framework. But yes I looked at Vue, React, Polymer, all of it.
We're closer I think to Polymer3 than any of the others. But telepathy was created because we found all of them limiting in some fashion. Either they require a bunch of boiler plate to use, or they add way too much complexity for what we're trying to do, which is to make templated custom elements with databinding, easy to use.

But we try to stick strictly with standards and seek to ease the amount of boilerplate required. We value convention over configuration with the option to override convention when needed.

For instance, to load any element and start using it, you only need to include the telepathic-loader script.
It will analyze whatever html file it finds itself in, it will look for anything ending in -element and start loading the elements it finds, even registering them to the browser's customElements registry on the fly, so it doesn't ever load the same thing twice.

Since we are using template literal syntax for binding, you get 2 way binding if you declare it to be the "value" attribute of an element and one way binding if you declare it anywhere else.

You don't actually need to specify bindings, if the class has a this.something and you declare ${this.something} anywhere in the template, an observer is created automatically and binding is created for you.
my-element.html

<div>${says}</div><br>
<input value="${says}>

my-element.js

export default class MyElement extends TelepathicElement{
   constructor(){
     super();
  }
}

index.html

<head>
<script type="module" src="telepathic-loader.js">
</head>
<body>
....
<my-element></my-element>
</body

That element and that binding requires 0 custom code, you don't even have to import my-element, the loader detects it and picks it up automatically.

Basically we view laziness as a virtue.

Also telepathic-loader will only pull in what is needed and nothing extra. So for instance to include the cow-element you only need to put

<cow-element></cow-element>

Somewhere in the HTML file along with telepathic-loader.js,
If you have a cow-element.js in your local telepathic-elements directory it will use that, otherwise it will check telepathic-elements.githubpages.io/cow-element and throw an error if it can't find it. No need to bundle in otherwords and no need for declarations in 99% of cases.

So what I'm doing now, and the reason I'm here is because I'd like to give users the option of maintaining all the power of telepathy while allowing them to specify their element template in markdown. For that to work I need to refactor marked.js to be class based.

I think I've about got it, but there were some mind blowing bits of code, not just the regex stuff, but places like https://github.com/markedjs/marked/blob/master/lib/marked.js
Lines 48 through 51

block.item = /^( *)(bull) [^\n]*(?:\n(?!\1bull )[^\n]*)*/;
block.item = edit(block.item, 'gm')
  .replace(/bull/g, block.bullet)
  .getRegex();

I don't really understand how to refactor this, because it looks like you're declaring block.item and then redeclaring it, which I can understand if this code is meant to run as it's being parsed, but I don't really know how to refactor that in the context of block being a standalone function, class or object.

[saragarmee]

I was able to eventually solve my own problem by breaking the declarations and usages into separate objects. I placed all of singular regex definitions into a "grammar" object and all of the places where the regex's are used into the block object.

So the first "block.item" becomes "grammar.item" and the second block.item remains intact with the minor change that block.item passed to the edit function is now grammar.item

Assuming that is how it is supposed to work. I wonder if there are speed ups to be had here.
Under the initial IIFE this procedure of upgrading the block.item grammar would only happen once, but under my new refactor it looks like some of these will happen multiple times.

I think I can correct this, by doing the replacements in situ using template literal syntax.

block.item = `/^( *)(${grammar.bullet}) [^\n]*(?:\n(?!\1${grammar.bullet} )[^\n]*)*/`

Or some variation thereof
I should probably profile this first to see if it's even going to be a bottle neck though.

[joshbruce]

Very cool!

This breaking up sounds like something we're planning to do as well. Very nice. Definitely helps making the future easier for the community.

Our roadmap, in simple terms:

0.x - spec compliance
1.x - make the hard changes simple
2.x - make the simple changes

Of course, we are almost purely community driven. :) If the community waits for us to do it, might be a minute. 😝

[saragarmee]

I'm willing to pitch in and roll up my sleeves if needed. Right now I'm busy fixing all the things I broke when refactoring, but it's coming along nicely. Feel free to use anything from my fork that's useful.

I would love to learn what is needed to ensure test coverage on my version. Unit tests have never been my strong suit.

[joshbruce]

Thanks. Hopefully that didn't come off as a pull for you - more of a conversation between us to the universe. 😀

I've flagged this issue into our 2.x milestone with a reference to the other issue talking about modularization; so, we should be good, once we get there. Thanks!

[saragarmee]

I see that and thanks. Actually I'm excited to be here. It's been quite the rabbit hole, but I'm learning and relearning a lot in the process.

[saragarmee]

I have a demo up using marked refactored into classes.
https://telepathic-elements.github.io/demos/marked-demo.html

It's nothing spectacular but it is working.

I am a bit concerned though because initially it was trying to URL encode / escape everything and I'm not certain why. I was able to hack around it by putting a ton of console.logs in and tracing it to this line...

          // text
          if (cap = this.rules.text.exec(src)) {
            console.log('text');
            src = src.substring(cap[0].length);
            //if (this.inRawBlock) {
              console.log('text inRawBlock');
              out += this.renderer.text(cap[0]);
           // } else {
             // console.log('text needs escaping');
              //out += this.renderer.text(escape(this.smartypants(cap[0])));
            //}
            continue;
          }

This is inside of
https://github.com/telepathic-elements/marked/blob/master/lib/lexer-inline.mjs

Lines 312...323 which corresponds to lines 830...843 in the original lib/marked.js file.

As you can see the point where I commented out, fixed things. But I'm not at all certain why.

I'm loading the markdown template as...

let marked = new Marked();
this.templateStr = await marked.parse(file);

The files are basically ordinary, but I did encounter a circular dependency between marked and it's renderer and had to remove the insantiated renderer from the defaults returned by getDefaults() in marked but I don't see how that could have any effect.

So basically I'm stuck pondering why everything wants to become escaped if I don't comment out those lines, and any help or advice is appreciated.

Thanks!

[styfle]

Closing as a duplicate of #1288