Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: fix code display in release blog #3977

Merged
merged 2 commits into from
Jul 14, 2021
Merged

fix: fix code display in release blog #3977

merged 2 commits into from
Jul 14, 2021

Conversation

Trott
Copy link
Member

@Trott Trott commented Jul 14, 2021

Fixes: #3976

@Trott Trott merged commit da826ba into nodejs:master Jul 14, 2021
@Trott Trott deleted the js branch July 14, 2021 10:34
@targos
Copy link
Member

targos commented Jul 14, 2021

The problem will come back next time we put an mjs code block in release notes. Node's linter forbids ESM syntax in js blocks

@Trott
Copy link
Member Author

Trott commented Jul 14, 2021

The problem will come back next time we put an mjs code block in release notes. Node's linter forbids ESM syntax in js blocks

Best option is probably to add mjs and cjs options for code highlighting in this repo. What do you think, @aduh95?

Refs: nodejs/node#37162

We probably don't need anything as elaborate as what's in the docs (with a toggle switch and all that). We probably just need to map mjs and cjs to js in the highlighter somehow, or else add something in our build process that converts mjs and cjs code fences to js code fences.

@richardlau
Copy link
Member

If it's just the release posts we already remap console in

// Make sure that all the console has been replaced
// by "```shell-session" for metalsmith-prism's check to pass
const rxSectionConsole = /```console/igm
const matches = rxSectionBody.exec(section)
return matches
? matches[1].trim().replace(rxSectionConsole, '```shell-session')
: Promise.reject(new Error(`Could not find changelog body of ${version} release`))

@aduh95
Copy link
Contributor

aduh95 commented Jul 14, 2021

I think we need to do the same thing I did in nodejs/node#37311.

@Trott
Copy link
Member Author

Trott commented Jul 14, 2021

I think we need to do the same thing I did in nodejs/node#37311.

If I understand correctly, that would fix lint errors but not the display error that we saw here.

@aduh95
Copy link
Contributor

aduh95 commented Jul 14, 2021

Hum indeed, I misread the issue. It looks like a bug in Prism:

Failed to load prism syntax: mjs
Error: Cannot find module 'prismjs/components/prism-mjs.js'

I'm able to workaround the issue by adding these lines at the top of build.js:

const { languages } = require('prismjs');
languages.mjs = languages.js;

@nschonni
Copy link
Member

I wonder if maybe the "preLoad" option is also applicable https://www.npmjs.com/package/metalsmith-prism#preload-optional

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
5 participants