Add RunKit embeds to Node documentation [WIP]

[tolmasky]

This addresses #21723.

This is functionally complete and only has a few cosmetic changes to be made hopefully, namely, choosing the the node examples to turn on in this initial run, and separately choosing the right syntax highlighting theme. I figured it was worth starting the discussion now as it touched a few minor other things, namely:

1. This change changes nodejs.org to syntax highlight examples at compile time instead of at runtime in the browser.

   Previous to this change, the syntax highlighting on nodejs.org was done at runtime using two scripts that were included in the repo in their minified form, sh_javascript.min.js and sh_main.min.js (which I believe remained untouched since they were originally committed 5 years ago). This made the the RunKit process a bit trickier than necessary, as we would have to first wait for the highlighting to finish before swapping in the RunKit embed. This had a number of other downsides though:

   1. Anyone with JavaScript turned off, or on a slow connection, would see the documentation without syntax highlighting.
   2. Only JavaScript was syntax highlighted.
   3. It is hard to modify syntax highlighting at all due to the minified nature of the scripts.
   4. There was unnecessary load and work done on the user's browser.

   With this change, we're using the standard highlight.js library, which makes it easy to change theme, and additionally means the many many C++ examples on nodejs.org are now syntax highlighted as well (hurray!). Beyond that, the examples will be highlighted instantly of course and it's kind of cool that with the RunKit change most people will actually be loading less JavaScript.

2. As part of the change above, I've taken the standard hopscotch theme.

   I have no particular strong feelings either way about this. Hopscotch seemed to fit well and I was going to translate the existing theme, but it seemed to only highlight keywords as green. I am happy to just do that as well, but hopscotch is very subtle too and, especially for the C++ examples for example, enhances readability. Again, I'm happy to directly translate the existing CSS, or alternatively, if anyone wants to just suggest any of the many many existing highlight.js themes I'm happy to switch to that.

OK, with that out of the way, the meat of the change:

This introduces the ability to add "runkit" to code blocks, turning them into runnable examples. For example:

    ```javascript runkit
    console.log("This example is runnable!")
    ```

The nice thing about this is that the examples continue to syntax highlight correctly in Github as Github just ignores everything after the initial language tag. As explained in the above bug, we're hoping to make this a major part of the full API redesign, but want to do some initial tests (scaling, etc.) on the current site. Currently this commit only includes a crypto example we know to work well, but we're hopefully going to be turning on around 5 - 10 total examples, which we'll be updating this PR with in the next couple of days.

A quick note on the way we've added RunKit: since there is only one template file used for every page, I've created a super tiny script that checks whether any example is runnable on load, and only then adds the runkit script tag. Again, since we're removing much more JavaScript than this already with the syntax highlighting change described above, this should improve load time overall. When we make the full API redesign, the ideal would be to include a defer-ed runkit script tag on pages that use it, and leave it out completely on those that don't.

Checklist

• make -j4 test (UNIX), or vcbuild test (Windows) passes
• tests and/or benchmarks are included
• documentation is changed or added
• commit message follows commit guidelines

[devsnek]

this looks so cool 😄

[devsnek]

can we make sure that this script and others are still using our eslint rules?

[tolmasky]

Sure thing!

meant to comment instead of approve

[Trott]

@nodejs/documentation @rubys

[vsemozhetbyt]

Does eslint-plugin-markdown still lint code blocks with runkit label?

[vsemozhetbyt]

Does eslint-plugin-markdown still lint code blocks with runkit label?

From my local tests, no. So we need to address this upstream with a proposal for eslint-plugin-markdown or find another way to mark the blocks.

[tolmasky]

I'll take a quick look at it, hopefully there's some setting I can do on our end that doesn't require changing and then upgrading the package.

[tolmasky]

There doesn't appear to be a simple way to do this, but it is a very simple change in the eslint-plugin-markdown code. I've filed the issue here and will be submitting the PR today: eslint/eslint-plugin-markdown#98

[rubys]

The two build failures can be reproduced locally by using the following command:

make lint-js test-doc

Essentially what this means is that tools/doc/html.js needs to conform to node.js coding standards, and an updated tools/doc/package-lock.json needs to be committed.

[tolmasky]

@rubys do you know what is failing now? Everything checks out when I run locally (except for "missing subsystem"?). The title formatting seems to flip back and forth (and I do not get it locally). I did a force push to rebase with master, is there any chance that confuses travis?

[vsemozhetbyt]

It is new commit linting in action:

  ✖  4101442a07256c38bf05149efc862b61970e5a1b
     ✔  0:0      skipping fixes-url                        fixes-url
     ✔  0:0      blank line after title                    line-after-title
     ✔  0:0      line-lengths are valid                    line-length
     ✖  0:0      Missing subsystem.                        subsystem
     ✖  0:31     Do not use punctuation at end of title.   title-format
     ✔  0:0      Title is <= 50 columns.                   title-length

Just edit the first commit title to something like:

doc,tools: add RunKit embeds to documentation

[tolmasky]

@vsemozhetbyt: thanks!

[vsemozhetbyt]

CI-lite: https://ci.nodejs.org/job/node-test-pull-request-lite-pipeline/1228/

[tolmasky]

Sorry to turn this into node build system 101 for me, but the current failures seem unrelated to anything in my branch (and locally master fails make -j2 test as well). However, I see other PRs passing, so I'm not sure what I'm doing wrong.

[vsemozhetbyt]

If you mean the status under this PR, this is often flaky. If you check CI itself, you can see it is OK:

https://ci.nodejs.org/job/node-test-pull-request-lite-pipeline/1228/

https://ci.nodejs.org/blue/organizations/jenkins/node-test-pull-request-lite-pipeline/detail/node-test-pull-request-lite-pipeline/1228/pipeline

[Trott]

The failure of test-buffer-alloc on Travis was unrelated to this change and was fixed in another commit on master already. (That one was my fault. Sorry!) The other stuff here is green. So I think this is good, at least as far as tests go.

[Trott]

(To fix your local failures, you probably just need to rebase against current upstream/master. If that doesn't do it, the next most obvious likely problem is a bug in macOS Mojave that causes two or three tests to fail. And if that's not it...let's talk more.)

[tolmasky]

Alright, so I have just issued a pull request to eslint-plugin-markdown to add the fix I implemented.

Currently this branch uses a fork of eslint-plugin-markdown I published on npm to test to make sure these changes would work, and it appears they do. I'm going to give it a few days to see if the PR to eslint-plugin-markdown is accepted, but given that there has been relatively little activity on that repo for a while (and the original bug was never responded to) I'm curious if it would be acceptable to use our fork here if the PR is not addressed. If not, I can try to find another workaround.

[tolmasky]

Quick update: eslint-plugin-markdown has merged the Pull Request, and they believe they'll roll a release over the weekend, at which point I'll be able to include it here.

[btmills]

I just published eslint-plugin-markdown@v1.0.0-rc.0, which includes eslint/eslint-plugin-markdown#99 🚢

[tolmasky]

I've now changed it to use eslint-plugin-markdown@1.0.0-rc.0. Along with this, I've updated update-eslint.sh to refer to ^1.0.0-rc.0 since next no longer refers to anything, and I don't think we want to rely on latest (but let me know if you prefer me to change it to latest). As part of this, I had to upgrade to eslint 5.8.0 because its non-trivial to update just eslint-plugin-markdown with the script, so that's why the PR has gotten much larger. I've tried to contain the update to its own commit, and if I make more changes I'll rebase it in a way where they hopefully come in before that, so that its possible to easily see all the "logical" changes and then have the big vendored libraries change come last.

As all tests seem to now pass we'll try to get the cosmetic changes in this week (choosing which embeds, the theme, etc).

[vsemozhetbyt]

It seems ESLint will be updated soon on master: #23904

[HarshithaKP]

@tolmasky, this needs rebase.

[himself65]

use hsl instead of hsla (because alpha is always 1)?

[Trott]

@tolmasky Would you still consider this WIP? Or is this ready-to-go in your opinion?

[tolmasky]

Hi @Trott,

Sorry for the delay, still in paternity but I have been able to get us almost all the way there in the last week, we're only waiting on two relatively minor things:

1. We are in the process of getting Node 14 up and running (currently this requires a background process on RunKit so we trail the main release by a few days).
2. We are in the process of getting a separate domain ready for embeds that run on nodejs. We've put in considerable work removing any cookies, but for browsers that don't partition cookies, it could still appear to users erroneously that cookies are being set if they previously visited RunKit and logged in for example. As such, we are working to get the actual embed code on a wholly separate domain to avoid any possibility of cross-leak whatsoever with the main site.

For a quick update on the things we've changed since the last update:

1. The big embed update has shipped -- the performance should now be very improved, and the script to load the embeds has also gotten about 50% smaller thanks to the updated API.
2. In response to Matteo's request, we have implemented a way to make sure that the embed only shows up if the exact version of node is available on RunKit. This will allow it to be safely back-ported to 10.x, and 12.x. For the time being, that means the embed will show up on the latest version of these (and correctly refrain from appearing on older versions). In the future, when RunKit gets support for all node versions (including old ones), it will "just work" with older versions of the docs too.

[tolmasky]

Just wanted to provide a quick update here. The first part of what I mentioned is now complete, Node 14 is up and running on RunKit. The second portion s taking longer since we didn't predict that our work on fully separating RunKit embeds from the main site would throw off some CSPs for a number of sites. We're working with them to get that updated and that's why that piece is moving a bit slower. Hopefully will complete the last part of this very soon.

[tolmasky]

Another very delayed update: we are doing our best to get existing users onto the new system that we created for the Node stuff, but its going slower than we hoped. As such, I figured it might be good to break this PR into two pieces, so that we can get some of the unrelated-to-RunKit benefits in sooner. Specifically, I've opened up this PR which only includes the syntax highlighting on compile stuff. I was very excited about this when I first put this PR together, and it saddens me that the rest of the changes blocked that from going live. This also means that it will be a lot easier to make sense of the RunKit specific changes in this PR. We are currently evaluating options as to how to have a "legacy" version of embeds up for non-Node users to hopefully decouple these events and finally let this go through.

[jasnell]

Given that this is still a work in progress, I've converted the PR into a draft

[halgurdhusen]

vx_lorstani1

[halgurdhusen]

vx_lorstani1

[bnb]

@tolmasky how are you feeling about this now? It seems like #34148 landed :)

[tolmasky]

@bnb Hi Tierney, thanks for following up! Unfortunately, a later PR to the docs undid some of the elements in that branch that were required for this one (totally innocently of course, they couldn't have known that those elements were necessary for a non-landed branch). It's not too difficult to get things back working, but we've been punting as the other component (RunKit supporting every version, not just the latest of every major version), will be shipping soon, which will actually remove some of the complexity in this PR. For some context, in this PR we currently have to take the extra step to only show the embed if the version of the docs matches the version we support on RunKit (IOW, if you load old minor/patch version docs, like say 16.3.0 instead of 16.3.1, we don't show the embed at all, as someone mentioned they'd think it was confusing if the embed was 16.3.1 but the docs was for 16.3.0). We thus need runtime logic to check the version, etc. However, very soon RunKit will just be able to load any version, and this logic can be removed.

[bnb]

However, very soon RunKit will just be able to load any version, and this logic can be removed.

sick, happy to hear that. If you plan to tidy up this PR when that lands in RunKit, happy to leave this open.

[tolmasky]

However, very soon RunKit will just be able to load any version, and this logic can be removed.

sick, happy to hear that. If you plan to tidy up this PR when that lands in RunKit, happy to leave this open.

Yes, would definitely appreciate that, thank you!