Separate marked into modules

[UziTech]

Marked version: master

Description

Split marked.js into separate modules and uses rollup and babel to combine them to build ./lib/marked.js

Also uses rollup to build ./lib/marked.esm.js for anyone who wants to use <script type="module" ...

• closes feat: ES Module entry point #1452
• closes move to ESM and what not #1349
• fixes ESM (ES6 module) distribution #1288
• fixes Modernizing the codebase #1539

TODO

• Test speed difference
  • Node is slightly faster and browser speed has no change.
• Test changing defaults
  • defaults were a little tricky since marked.defaults is no longer a global in every module, but setting defaults should be backwards compatible.
• Update docs where necessary

Contributor

• Test(s) exist to ensure functionality and minimize regression.

Committer

In most cases, this should be a different person than the contributor.

• Draft GitHub release notes have been updated.
• CI is green (no forced merge required).
• Merge PR

[vercel]

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

🔍 Inspect: https://zeit.co/markedjs/markedjs/8qhlyysmp
🌍 Preview: https://markedjs-git-fork-uzitech-rollup.markedjs.now.sh

[UziTech]

Running the benchmark on ./lib/marked.js after using rollup and babel was consistently about 20% slower than master.

marked completed in 6957ms and passed 100.00%
old marked completed in 5496ms and passed 100.00%
marked (gfm) completed in 6432ms and passed 100.00%
old marked (gfm) completed in 6148ms and passed 100.00%
marked (pedantic) completed in 6407ms and passed 100.00%
old marked (pedantic) completed in 5378ms and passed 100.00%

I guess we have to decide if it is worth the slow down to have easier development.

There may also be some way that we can speed it up but I can't think of a way off the top of my head.

@joshbruce @styfle @davisjam @Feder1co5oave

[styfle]

You should probably use "type: module" to make this work with Node.js (however it is still experimental)

https://nodejs.org/api/esm.html#esm_code_package_json_code_code_type_code_field

[styfle]

@UziTech Why did you mark this as "resolved"? This does not follow the Node.js standard for ESM.

[styfle]

I just realized this PR is not using ESM at all?

[UziTech]

Ya ES Modules are still experimental and I found that CommonJS is faster in node lts right now.

It wouldn't be hard to change to ESM in the future.

[styfle]

@UziTech I guess I don't see the difference between this PR and #1349 or #1452

• move to ESM and what not #1349 implements ESM source files and separates into separate modules instead of one source file
• feat: ES Module entry point #1452 implements CJS => ESM as an output via rollup

We rejected both of them with the plan to wait for ESM to go stable in Node. So why make this PR?

[UziTech]

This PR doesn't move to ESM it uses CommonJS for node. Other than that they are pretty much the same.

When I started creating this PR it was more or less just to test the speed of making marked modular and it turns out moving to CommonJS on node made it a bit faster , and using rollup and babel to output browser versions kept the speed about the same.

[joshbruce]

Yeah. @chjj did mention that he did a lot of JS voodoo to get Marked to be as fast is it is.

So, just to make sure I'm seeing this correctly. We're splitting things up - like the issues we've put out for updating the architecture. However, we are still compiling back to something that's backward compatible, which is would give us that move before the 1.0 release (where I think we have that forecasted)?

The splits seem perfectly rational.

Any idea what's causing that shift?? (Like what non-API changes are we making to causing it to run slower??)

[UziTech]

I'm trying to move the different components (e.g. Parser, Renderer, etc.) to different files and allow us to use es6+ to make development easier but keep lib/marked.js and marked.min.js es5 so anyone using marked in browsers can still use

<script src="https://cdn.jsdelivr.net/gh/markedjs/marked/lib/marked.js"></script>

I'm looking into the slowdown. I'm guessing it has to do with babel using slow polyfills.

[UziTech]

It must be babel. When I kept the code as es5 and just used rollup to combine the modules the speed difference was negligible.

marked completed in 5467ms and passed 100.00%
old marked completed in 5350ms and passed 100.00%
marked (gfm) completed in 5676ms and passed 100.00%
old marked (gfm) completed in 5632ms and passed 100.00%
marked (pedantic) completed in 5294ms and passed 100.00%
old marked (pedantic) completed in 5377ms and passed 100.00%

[UziTech]

I have done a lot of testing and here is my findings:

1. ES6 classes are about 5% faster than function.prototype, and about the same when compiled by babel
2. commonjs is about 10% faster than es modules. This may change in the future when es modules are not experimental in node.

bench:
Node 12.13.0
Windows 10

es5 marked completed in 4963ms and passed 100.00%
es6 marked completed in 5078ms and passed 100.00%
master marked completed in 5203ms and passed 100.00%
es5 marked (gfm) completed in 6048ms and passed 100.00%
es6 marked (gfm) completed in 5540ms and passed 100.00%
master marked (gfm) completed in 5854ms and passed 100.00%
es5 marked (pedantic) completed in 5829ms and passed 100.00%
es6 marked (pedantic) completed in 5328ms and passed 100.00%
master marked (pedantic) completed in 5326ms and passed 100.00%
commonmark completed in 5981ms and passed 100.00%
markdown-it completed in 5784ms and passed 100.00%
markdown.js completed in 10596ms and passed 100.00%

[UziTech]

@davisjam I removed the redos test with vuln-regex-detector because it looks like it has just been timing out in travis for a while so it doesn't provide any benefit.

However, with this move to make marked more modular I did move the regular expressions into their own file so we could run some tests on that file to look for vulnerable regular expressions.

[joshbruce]

Appreciate the separation of the regex.

I also do like the idea being able to take "automatic" advantage of future language improvements. I'm not as embedded in the JS world, but I imagine the move to...pure (??)... native classes would be almost inevitable if only for adoption reasons.

Leave that to the more JS steeped though.

Is ~6s the baseline for the performance tests?

[UziTech]

The benchmark time is very dependent on the machine running it but the important thing is the relative difference between each test.

[alasdairhurst]

@UziTech do you recall the reason for bumping the minimum node version? This seemed to go in without any comment or any note of it in the release notes. Was this PR actually introducing a breaking change causing it to no longer work on older node versions or was it just updated to reflect an incorrect state of the module?

[UziTech]

@alasdairhurst I think it just had to do with some of our dev dependencies not working on older versions of node. lib/marked.js is transpiled to es5 and should work in older versions but we aren't testing on them.