Produce HTML documentation using unified/remark/rehype #21490

rubys · 2018-06-23T19:44:28Z

First baby step towards implementing #21476

Checklist

make -j4 test (UNIX), or vcbuild test (Windows) passes
tests and/or benchmarks are updated
commit message follows commit guidelines

rubys · 2018-06-23T20:11:47Z

Replacing the remainder of calls to marked in tools/doc/html.js would be straightforward; basically remark/unified is a pipeline of filters, and depending on where you insert a processor, you have access to raw input, a tree representing the markdown, a tree representing the HTML, and the resulting HTML as a string. Processors can modify the data (as is done in this commit by adding class attributes to the TOC links). The resulting code should be more robust as we have the opportunity to deal with the data in tree form as opposed to attacking strings with regular expressions.

tools/doc/json.js in theory should be simpler as we can deal with the markdown in tree form, and converting a tree to into another tree, and then stringifying that tree as JSON.

The coding parts I'm comfortable with. It is the process by which code gets merged into the baseline that I'm interested in. I figured working on a documentation tool would be a good way to get started.

ChALkeR · 2018-06-23T20:19:11Z

tools/doc/html.js

@@ -24,6 +24,11 @@
 const common = require('./common.js');
 const fs = require('fs');
 const marked = require('marked');
+var unified = require('unified');


fixed and squashed.

Trott · 2018-06-23T22:14:09Z

From within tools/doc, you might want to run npx dmn -f clean to see if it removes a few files.

tools/license-build.sh will need an update before this can land.

I'm going to add the in progress label on this.

If we can get all the code changes in that would result in marked being removed, that would help us make a decent evaluation as to trade-off. But that may be asking a lot.

rubys · 2018-06-23T22:25:55Z

From within tools/doc, you might want to run npx dmn -f clean to see if it removes a few files.

Unlikely at this point as I have only added dependencies so far. Removing comes later.

tools/license-build.sh will need an update before this can land.

Please explain?

I'm going to add the in progress label on this.

Cool.

If we can get all the code changes in that would result in marked being removed, that would help us make a decent evaluation as to trade-off. But that may be asking a lot.

It is possible that I will be able to complete this tomorrow, but if not I don't expect to have much time Monday through Wednesday, but I should be able to wrap this up by next weekend.

I'm happiest if changes are merged incrementally, but that may be asking a lot. I will say that this change should be easy to revert should it come to that. If not, should I have separate branches, or a single branch for the total set of changes?

rubys · 2018-06-23T23:31:29Z

tools/license-build.sh will need an update before this can land.

Please explain?

Nevermind. Looks straightforward enough.

Trott · 2018-06-24T03:22:10Z

Unlikely at this point as I have only added dependencies so far. Removing comes later.

npx dmn -f clean will remove these files:

	deleted:    tools/doc/node_modules/define-properties/.editorconfig
	deleted:    tools/doc/node_modules/define-properties/.eslintrc
	deleted:    tools/doc/node_modules/define-properties/.jscs.json
	deleted:    tools/doc/node_modules/define-properties/.npmignore
	deleted:    tools/doc/node_modules/define-properties/.travis.yml
	deleted:    tools/doc/node_modules/define-properties/CHANGELOG.md
	deleted:    tools/doc/node_modules/define-properties/test/index.js
	deleted:    tools/doc/node_modules/extend/.eslintrc
	deleted:    tools/doc/node_modules/extend/.jscs.json
	deleted:    tools/doc/node_modules/extend/.npmignore
	deleted:    tools/doc/node_modules/extend/.travis.yml
	deleted:    tools/doc/node_modules/extend/CHANGELOG.md
	deleted:    tools/doc/node_modules/extend/component.json
	deleted:    tools/doc/node_modules/foreach/.npmignore
	deleted:    tools/doc/node_modules/foreach/component.json
	deleted:    tools/doc/node_modules/foreach/test.js
	deleted:    tools/doc/node_modules/is-nan/.eslintrc
	deleted:    tools/doc/node_modules/is-nan/.jscs.json
	deleted:    tools/doc/node_modules/is-nan/.npmignore
	deleted:    tools/doc/node_modules/is-nan/.travis.yml
	deleted:    tools/doc/node_modules/is-nan/CHANGELOG.md
	deleted:    tools/doc/node_modules/is-nan/test/index.js
	deleted:    tools/doc/node_modules/is-nan/test/shimmed.js
	deleted:    tools/doc/node_modules/is-nan/test/tests.js
	deleted:    tools/doc/node_modules/kebab-case/.editorconfig
	deleted:    tools/doc/node_modules/kebab-case/.npmignore
	deleted:    tools/doc/node_modules/kebab-case/.travis.yml
	deleted:    tools/doc/node_modules/kebab-case/test.js
	deleted:    tools/doc/node_modules/marked/.npmignore
	deleted:    tools/doc/node_modules/marked/.travis.yml
	deleted:    tools/doc/node_modules/marked/Gulpfile.js
	deleted:    tools/doc/node_modules/marked/bower.json
	deleted:    tools/doc/node_modules/marked/component.json
	deleted:    tools/doc/node_modules/marked/doc/broken.md
	deleted:    tools/doc/node_modules/marked/doc/todo.md
	deleted:    tools/doc/node_modules/mdurl/CHANGELOG.md
	deleted:    tools/doc/node_modules/object-keys/.editorconfig
	deleted:    tools/doc/node_modules/object-keys/.eslintrc
	deleted:    tools/doc/node_modules/object-keys/.jscs.json
	deleted:    tools/doc/node_modules/object-keys/.travis.yml
	deleted:    tools/doc/node_modules/object-keys/CHANGELOG.md
	deleted:    tools/doc/node_modules/object-keys/test/index.js
	deleted:    tools/doc/node_modules/trim/.npmignore
	deleted:    tools/doc/node_modules/trim/History.md
	deleted:    tools/doc/node_modules/trim/component.json
	deleted:    tools/doc/node_modules/x-is-array/.npmignore
	deleted:    tools/doc/node_modules/x-is-array/.travis.yml
	deleted:    tools/doc/node_modules/x-is-array/test/index.js
	deleted:    tools/doc/node_modules/x-is-string/.npmignore
	deleted:    tools/doc/node_modules/x-is-string/.travis.yml
	deleted:    tools/doc/node_modules/x-is-string/test/index.js
	deleted:    tools/doc/node_modules/xtend/.jshintrc
	deleted:    tools/doc/node_modules/xtend/.npmignore
	deleted:    tools/doc/node_modules/xtend/test.js

Trott · 2018-06-24T03:28:18Z

It is possible that I will be able to complete this tomorrow, but if not I don't expect to have much time Monday through Wednesday, but I should be able to wrap this up by next weekend.

Do it at whatever pace suits you. Having just one markdown processor in our dependencies is a nice-to-have for sure, but there's certainly no deadline or anything.

Also, be aware that there's always the chance it won't get merged. There may be reasons for using marked that I'm unaware or, or shortcomings of remark that I don't know about. Just mentioning this now because I don't want you to do a ton of work, have it ultimately not merged, and feel like that was never broached as a possibility. :-D

I'm happiest if changes are merged incrementally, but that may be asking a lot. I will say that this change should be easy to revert should it come to that. If not, should I have separate branches, or a single branch for the total set of changes?

Maybe one PR/branch but multiple atomic/incremental commits? Just as long as every commit contains a fully working system. No commit should be broken, of course.

vsemozhetbyt · 2018-06-24T10:57:10Z

tools/doc/html.js

@@ -36,13 +41,31 @@ marked.setOptions({ renderer });

 const docPath = path.resolve(__dirname, '..', '..', 'doc');

+// add class attributes to index navigation links


We usually use a first uppercase letter and a period in the end in comments lately)

// Add class attributes to index navigation links.

vsemozhetbyt · 2018-06-24T11:21:38Z

I've checked the diff between the results of this PR and the current master and it seems we have no significant changes so far, just these nits:

The order of attributes inside <a> elements is changed (class after or before href).
HTML entities format sometimes is different (& vs &).
Some empty lines are deleted between elements.

TimothyGu · 2018-06-24T17:35:18Z

Is there a way to reuse the remark modules that are already included in our source tree?

rubys · 2018-06-24T19:37:54Z

Hit a speed bump. I've converted the code to the point where HTML is produced that resembles the input (always a good sign!) for all but one of the files. That file is all.md. Processing that file runs out of heap space.

A small test case: https://gist.github.com/rubys/a061064e0744a5792b3e8331b0dae37d

The file in question is 52K lines. The problem is the size - processing either the first 26K or the last 26K lines works just fine.

Possible solutions include increasing the heap size; processing this one file in chunks, building this file using other means, or aborting this effort.

vsemozhetbyt · 2018-06-24T22:40:50Z

BTW, all.html generation is currently broken in some way, so it would be good to find any solution for this issue during experiments with other markdown modules.

rubys · 2018-06-25T01:29:09Z

There appears to be very few LICENSE files in the node_modules themselves:

~/git/node/tools/doc/node_modules$ find . | grep -i license  | egrep 'unified|remark|rehype'
./unified/LICENSE
./remark/node_modules/unified/LICENSE
./remark/node_modules/vfile/LICENSE
./remark-rehype/LICENSE
./rehype-raw/LICENSE

Trott · 2018-06-25T03:59:37Z

This is fantastic so far. Thanks for getting it moving so far so fast. I don't know what the right answer is to the 52K-line heap-exhaustion dilemma, but processing it in chunks seems like a promising idea given that the all.md file itself is a bunch of @includes of other .md files. Additionally, processing it one @include at a time would seem to make it possible to fix the issue noted by @vsemozhetbyt, since the origin of that problem is naming collisions between files.

vsemozhetbyt · 2018-06-25T04:45:28Z

It seems the cause of the issue is a collision between link references: now we join sources first and then parse/convert them, so all homonymous links are rewritten by the last one. If we could parse/convert the sources first and then join them, the issue may be fixed.

rubys · 2018-06-25T11:28:56Z

@vsemozhetbyt perhaps something like this: https://gist.github.com/rubys/4a140b5b61eab4cea34aca56098e900e ? Seems to be able to handle the full document without running out of heap space. Perhaps it could be cleaned up and landed separately as a fix for #20100 .

vsemozhetbyt · 2018-06-28T10:22:20Z

tools/doc/package.json

@@ -7,7 +7,15 @@
    "node": ">=6"
  },
  "dependencies": {
-    "marked": "^0.3.5"
+    "jsdom": "^11.11.0",


Is this dependency intended for this PR?

Good catch! Fixed.

vsemozhetbyt · 2018-06-28T15:17:49Z

What we also need to estimate is doc building time with various toolchains, as doc building is run on each CI and local tests. Currently on my machine, master with marked vs this PR branch with unified/remark/rehype:

real    0m46.983s

real    1m32.970s

I.e. this approach is twice as slow. If we use it also for JSON generation, it can be three times as slow. However, it can be faster (or slower) with another all.md processing (as in #21568 or #21544).

rubys · 2018-06-28T20:08:28Z

Some performance data:

{ gtoc: 31,
  parse: 207,
  firstHeader: 1,
  preprocessText: 8,
  preprocessElements: 131,
  buildToc: 46,
  remark2rehype: 26,
  raw: 262,
  serialize: 41 }

See https://gist.github.com/rubys/4798e9c5418814c8810b295dfb5cbbaa for how I instrumented the code. Here's the command I used:

node tools/doc/generate.js --node-version=v11.0.0 --format=html --analytics= doc/api/fs.md > /dev/null

I picked fs.md as it was the largest. The calls to .bind{} are because unified seems to optimize out multiple calls to the same function, so I made new functions.

The biggest times are for parsing HTML (raw) followed by parsing markdown (parse). Together, they make up 62% of the time.

I haven't looked closely at the JSON processing yet, but given that there is but a single occurrence of lexed.forEach((tok), it seems likely to me that it could be implemented as a single Unified plugin -- a plugin that doesn't modify the tree, but does write to a side file. If so, this plugin could be fed the same parsed tree structure; meaning that a single call to generate could produce both html and json output.

This one change won't eliminate the time difference between marked and remark tool chains, but would significantly reduce it.

vsemozhetbyt

I've only started to learn the toolchain and the new html.js, so this is just some nits so far) I will continue on Saturday if I find some more time)

vsemozhetbyt · 2018-06-28T20:36:24Z

tools/doc/html.js

+function navClasses() {
+  return walker;
+
+  function walker(node, file) {


As we do not use the file argument, maybe we can drop it here and in the recursive calls?

vsemozhetbyt · 2018-06-28T20:40:52Z

tools/doc/html.js

+    .use(markdown)
+    .use(firstHeader)
+    .use(preprocessText)
+    .use(preprocessElements, { filename: filename })


Just { filename } here and below?

vsemozhetbyt · 2018-06-28T20:52:17Z

tools/doc/html.js

                     .replace(/__VERSION__/g, nodeVersion)
-                     .replace('__TOC__', toc)
+                     .replace('__TOC__', content.toc)
+                     .replace('__CONTENT__', content.toString())


I am not sure if this is significant, but is it worth to make this change at the very end? We have some replacing below, and content.toString() is the biggest insertion here. Would the __GTOC__, analytics and docCreated insertions be faster if it is made before __CONTENT__ replacement? Because this will make .replace() calls operate on a smaller string.

vsemozhetbyt · 2018-06-30T12:59:38Z

I've read the rest of html.js, LGTM. If #21568 is incorporated and this approach is preferred to the #21081, this may be landable (JSON generation should also be considered though). But we may need to wait for more collaborators chime in.

vsemozhetbyt · 2018-06-30T13:02:09Z

CI: https://ci.nodejs.org/job/node-test-pull-request/15687/

rubys · 2018-06-30T15:16:59Z

If #21568 is incorporated, I'll clean up and complete this pull request (for example, the increase in heap size can be removed). I'd rather wait on the JSON generation until a decision is made on remark vs marked, but if this one is approved, I will start on the JSON conversion.

Trott · 2018-07-01T03:18:12Z

Should the in progress label be removed?

PR-URL: nodejs#21490 Reviewed-By: Vse Mozhet Byt <vsemozhetbyt@gmail.com> Reviewed-By: Rich Trott <rtrott@gmail.com>

Trott · 2018-07-20T20:41:39Z

Landed in 9466865. Fantastic work, @rubys!!!! 🎉

Trott · 2018-07-20T21:13:52Z

make test-doc and make doc-only now leave some cruft around in the source tree that git does not ignore. Not the end of the world or anything, but if someone feels motivated to clean that up in a subsequent PR, 👍.

rubys · 2018-07-20T21:17:01Z

I'll try to clean it up as a part of #21697, which is now unblocked.

Markdown interprets the syntax for optional arguments as a form of a link, so instead of trying to build up the contents using the node value, use grab the raw original markup instead. Fixes regression noted in nodejs#21490 (comment)

Markdown interprets the syntax for optional arguments as a form of a link, so instead of trying to build up the contents using the node value, use grab the raw original markup instead. Fixes regression noted in #21490 (comment) PR-URL: #21922 Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: Vse Mozhet Byt <vsemozhetbyt@gmail.com> Reviewed-By: Rich Trott <rtrott@gmail.com>

PR-URL: #21490 Reviewed-By: Vse Mozhet Byt <vsemozhetbyt@gmail.com> Reviewed-By: Rich Trott <rtrott@gmail.com>

Markdown interprets the syntax for optional arguments as a form of a link, so instead of trying to build up the contents using the node value, use grab the raw original markup instead. Fixes regression noted in #21490 (comment) PR-URL: #21922 Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: Vse Mozhet Byt <vsemozhetbyt@gmail.com> Reviewed-By: Rich Trott <rtrott@gmail.com>

vsemozhetbyt · 2018-07-22T11:50:01Z

@rubys Another breaking change: in headings, some optional arguments are linkified and next arguments disappear.

Compare, for example, this doc heading before this PR landed and in the last Nightly (the last one is after the #21922 has been landed so that PR has not fixed this issue).

ensure optional parameters are not treated as markedown links by replacing the children of headers nodes with a single text node containing the raw markup; Fixes issue identified in nodejs#21490 (comment)

ensure optional parameters are not treated as markedown links by replacing the children of headers nodes with a single text node containing the raw markup; Fixes issue identified in #21490 (comment) PR-URL: #21936 Reviewed-By: Vse Mozhet Byt <vsemozhetbyt@gmail.com> Reviewed-By: Minwoo Jung <minwoo@nodesource.com> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Rich Trott <rtrott@gmail.com> Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>

* remove obsolete `node_modules/js-yaml/package.json` target * remove `@touch` since `npm ci` is always destructive PR-URL: nodejs#22399 Refs: nodejs#21802 Refs: nodejs#21490 Reviewed-By: Tiancheng "Timothy" Gu <timothygu99@gmail.com> Reviewed-By: Sam Ruby <rubys@intertwingly.net> Reviewed-By: Richard Lau <riclau@uk.ibm.com> Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com> Reviewed-By: Daijiro Wachi <daijiro.wachi@gmail.com>

* remove obsolete `node_modules/js-yaml/package.json` target * remove `@touch` since `npm ci` is always destructive PR-URL: #22399 Refs: #21802 Refs: #21490 Reviewed-By: Tiancheng "Timothy" Gu <timothygu99@gmail.com> Reviewed-By: Sam Ruby <rubys@intertwingly.net> Reviewed-By: Richard Lau <riclau@uk.ibm.com> Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com> Reviewed-By: Daijiro Wachi <daijiro.wachi@gmail.com>

nodejs-github-bot added doc Issues and PRs related to the documentations. tools Issues and PRs related to the tools directory. labels Jun 23, 2018

ChALkeR reviewed Jun 23, 2018

View reviewed changes

rubys force-pushed the process_gtoc_with_remark branch from 515edb5 to 9b5344b Compare June 23, 2018 22:04

Trott added the wip Issues and PRs that are still a work in progress. label Jun 23, 2018

vsemozhetbyt reviewed Jun 24, 2018

View reviewed changes

rubys changed the title ~~Process gtoc with remark~~ Produce HTML documentation using unified/remark/rehype Jun 25, 2018

vsemozhetbyt mentioned this pull request Jun 26, 2018

tools: build doc/api/all.html by combining generated HTML #21544

Closed

3 tasks

vsemozhetbyt reviewed Jun 28, 2018

View reviewed changes

Trott pushed a commit to Trott/io.js that referenced this pull request Jul 20, 2018

tools: create HTML docs with unified/remark/rehype

9466865

PR-URL: nodejs#21490 Reviewed-By: Vse Mozhet Byt <vsemozhetbyt@gmail.com> Reviewed-By: Rich Trott <rtrott@gmail.com>

Trott closed this Jul 20, 2018

This comment has been minimized.

Sign in to view

rubys mentioned this pull request Jul 21, 2018

tools: build API TOC using raw headers #21922

Closed

2 tasks

targos pushed a commit that referenced this pull request Jul 21, 2018

tools: create HTML docs with unified/remark/rehype

5606f0b

PR-URL: #21490 Reviewed-By: Vse Mozhet Byt <vsemozhetbyt@gmail.com> Reviewed-By: Rich Trott <rtrott@gmail.com>

rubys mentioned this pull request Jul 22, 2018

tools: flatten apidoc headers #21936

Closed

2 tasks

targos mentioned this pull request Jul 31, 2018

v10.8.0 proposal #22040

Merged

refack mentioned this pull request Aug 19, 2018

build: use npm ci #22399

Merged

2 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Produce HTML documentation using unified/remark/rehype #21490

Produce HTML documentation using unified/remark/rehype #21490

rubys commented Jun 23, 2018 •

edited

Loading

rubys commented Jun 23, 2018

ChALkeR Jun 23, 2018

rubys Jun 23, 2018

Trott commented Jun 23, 2018

rubys commented Jun 23, 2018 •

edited

Loading

rubys commented Jun 23, 2018

Trott commented Jun 24, 2018

Trott commented Jun 24, 2018

vsemozhetbyt Jun 24, 2018 •

edited

Loading

vsemozhetbyt commented Jun 24, 2018 •

edited

Loading

TimothyGu commented Jun 24, 2018

rubys commented Jun 24, 2018

vsemozhetbyt commented Jun 24, 2018 •

edited

Loading

rubys commented Jun 25, 2018

Trott commented Jun 25, 2018

vsemozhetbyt commented Jun 25, 2018

rubys commented Jun 25, 2018

vsemozhetbyt Jun 28, 2018

rubys Jun 28, 2018

vsemozhetbyt commented Jun 28, 2018 •

edited

Loading

rubys commented Jun 28, 2018

vsemozhetbyt left a comment

vsemozhetbyt Jun 28, 2018

vsemozhetbyt Jun 28, 2018

vsemozhetbyt Jun 28, 2018 •

edited

Loading

vsemozhetbyt commented Jun 30, 2018

vsemozhetbyt commented Jun 30, 2018

rubys commented Jun 30, 2018

Trott commented Jul 1, 2018

Trott commented Jul 20, 2018

Trott commented Jul 20, 2018

rubys commented Jul 20, 2018

This comment has been minimized.

vsemozhetbyt commented Jul 22, 2018 •

edited

Loading

		@@ -36,13 +41,31 @@ marked.setOptions({ renderer });

		const docPath = path.resolve(__dirname, '..', '..', 'doc');

		// add class attributes to index navigation links

Produce HTML documentation using unified/remark/rehype #21490

Produce HTML documentation using unified/remark/rehype #21490

Conversation

rubys commented Jun 23, 2018 • edited Loading

Checklist

rubys commented Jun 23, 2018

ChALkeR Jun 23, 2018

Choose a reason for hiding this comment

rubys Jun 23, 2018

Choose a reason for hiding this comment

Trott commented Jun 23, 2018

rubys commented Jun 23, 2018 • edited Loading

rubys commented Jun 23, 2018

Trott commented Jun 24, 2018

Trott commented Jun 24, 2018

vsemozhetbyt Jun 24, 2018 • edited Loading

Choose a reason for hiding this comment

vsemozhetbyt commented Jun 24, 2018 • edited Loading

TimothyGu commented Jun 24, 2018

rubys commented Jun 24, 2018

vsemozhetbyt commented Jun 24, 2018 • edited Loading

rubys commented Jun 25, 2018

Trott commented Jun 25, 2018

vsemozhetbyt commented Jun 25, 2018

rubys commented Jun 25, 2018

vsemozhetbyt Jun 28, 2018

Choose a reason for hiding this comment

rubys Jun 28, 2018

Choose a reason for hiding this comment

vsemozhetbyt commented Jun 28, 2018 • edited Loading

rubys commented Jun 28, 2018

vsemozhetbyt left a comment

Choose a reason for hiding this comment

vsemozhetbyt Jun 28, 2018

Choose a reason for hiding this comment

vsemozhetbyt Jun 28, 2018

Choose a reason for hiding this comment

vsemozhetbyt Jun 28, 2018 • edited Loading

Choose a reason for hiding this comment

vsemozhetbyt commented Jun 30, 2018

vsemozhetbyt commented Jun 30, 2018

rubys commented Jun 30, 2018

Trott commented Jul 1, 2018

Trott commented Jul 20, 2018

Trott commented Jul 20, 2018

rubys commented Jul 20, 2018

This comment has been minimized.

vsemozhetbyt commented Jul 22, 2018 • edited Loading

rubys commented Jun 23, 2018 •

edited

Loading

rubys commented Jun 23, 2018 •

edited

Loading

vsemozhetbyt Jun 24, 2018 •

edited

Loading

vsemozhetbyt commented Jun 24, 2018 •

edited

Loading

vsemozhetbyt commented Jun 24, 2018 •

edited

Loading

vsemozhetbyt commented Jun 28, 2018 •

edited

Loading

vsemozhetbyt Jun 28, 2018 •

edited

Loading

vsemozhetbyt commented Jul 22, 2018 •

edited

Loading