Skip to content
This repository has been archived by the owner on Jun 3, 2024. It is now read-only.

Refactor markdown syntax highlighting. #720

Merged
merged 17 commits into from
Dec 20, 2019
Merged
5 changes: 1 addition & 4 deletions dash_core_components_base/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,7 @@
'datepicker',
'dropdown',
'graph',
'highlight',
'markdown',
'upload'
]
Expand Down Expand Up @@ -70,10 +71,6 @@
} for async_resource in async_resources])

_js_dist.extend([
{
'relative_package_path': 'highlight.pack.js',
'namespace': 'dash_core_components'
},
Copy link
Contributor

@Marc-Andre-Rivet Marc-Andre-Rivet Dec 17, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🎉

{
'relative_package_path': '{}.min.js'.format(__name__),
'external_url': (
Expand Down
2 changes: 0 additions & 2 deletions dash_core_components_base/highlight.pack.js

This file was deleted.

26 changes: 26 additions & 0 deletions package-lock.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 1 addition & 0 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@
"dependencies": {
"color": "^3.1.0",
"fast-isnumeric": "^1.1.3",
"highlight.js": "^9.17.1",
"moment": "^2.20.1",
"plotly.js": "1.51.3",
"prop-types": "^15.6.0",
Expand Down
20 changes: 14 additions & 6 deletions src/fragments/Markdown.react.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,12 +2,20 @@ import React, {Component} from 'react';
import {type} from 'ramda';
import Markdown from 'react-markdown';

import MarkdownHighlighter from '../utils/MarkdownHighlighter';
import {propTypes, defaultProps} from '../components/Markdown.react';
import '../components/css/highlight.css';
Copy link
Contributor

@Marc-Andre-Rivet Marc-Andre-Rivet Dec 19, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If we're to keep this custom css, might as well import it dynamically when we request hljs instead of getting it dynamically with the markdown component.


export default class DashMarkdown extends Component {
constructor(props) {
super(props);
/* eslint-disable no-new */
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Disabling this error as we really only do want the side-effects of creating a new Highlight object. An alternative is wrapping all of the Highlight code with a function instead of a class.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you explain what's happening here?

new MarkdownHighlighter();
if (MarkdownHighlighter.isReady !== true) {
MarkdownHighlighter.isReady.then(() => {
this.setState({});
});
}
this.highlightCode = this.highlightCode.bind(this);
this.dedent = this.dedent.bind(this);
}
Expand All @@ -21,15 +29,15 @@ export default class DashMarkdown extends Component {
}

highlightCode() {
if (!window.hljs) {
// skip highlighting if highlight.js isn't found
return;
}
if (this.mdContainer) {
const nodes = this.mdContainer.querySelectorAll('pre code');

for (let i = 0; i < nodes.length; i++) {
window.hljs.highlightBlock(nodes[i]);
if (MarkdownHighlighter.hljs) {
for (let i = 0; i < nodes.length; i++) {
MarkdownHighlighter.hljs.highlightBlock(nodes[i]);
}
} else {
MarkdownHighlighter.loadhljs();
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think the existing implementation does not take full advantage of the react-markdown ability to get its renderer overriden -- similarly to https://github.com/plotly/dash-core-components/pull/711/files#diff-5b56ed82805404e28302aef5147d04a2R129 it should be possible to hook up the highlight directly onto the code renderer and simplify this component / usage.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Issue created: #724, will not be addressed as part of this PR.

}
}
}
Expand Down
32 changes: 32 additions & 0 deletions src/third-party/highlight.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
import highlightjs from 'highlight.js/lib/highlight';
import 'highlight.js/styles/github.css';

import bash from 'highlight.js/lib/languages/bash';
import css from 'highlight.js/lib/languages/css';
import http from 'highlight.js/lib/languages/http';
import javascript from 'highlight.js/lib/languages/javascript';
import json from 'highlight.js/lib/languages/json';
import markdown from 'highlight.js/lib/languages/markdown';
import python from 'highlight.js/lib/languages/python';
import r from 'highlight.js/lib/languages/r';
import ruby from 'highlight.js/lib/languages/ruby';
import shell from 'highlight.js/lib/languages/shell';
import sql from 'highlight.js/lib/languages/sql';
import xml from 'highlight.js/lib/languages/xml';
import yaml from 'highlight.js/lib/languages/yaml';

highlightjs.registerLanguage('bash', bash);
highlightjs.registerLanguage('css', css);
highlightjs.registerLanguage('http', http);
highlightjs.registerLanguage('javascript', javascript);
highlightjs.registerLanguage('json', json);
highlightjs.registerLanguage('markdown', markdown);
highlightjs.registerLanguage('python', python);
highlightjs.registerLanguage('r', r);
highlightjs.registerLanguage('ruby', ruby);
highlightjs.registerLanguage('shell', shell);
highlightjs.registerLanguage('sql', sql);
highlightjs.registerLanguage('xml', xml);
highlightjs.registerLanguage('yaml', yaml);

export default highlightjs;
7 changes: 7 additions & 0 deletions src/utils/LazyLoader.js
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,13 @@ export default {
import(/* webpackChunkName: "dropdown" */ '../fragments/Dropdown.react'),
graph: () =>
import(/* webpackChunkName: "graph" */ '../fragments/Graph.react'),
hljs: () =>
Promise.resolve(
window.hljs ||
import(/* webpackChunkName: "highlight" */ '../third-party/highlight.js').then(
result => result.default
)
),
markdown: () =>
import(/* webpackChunkName: "markdown" */ '../fragments/Markdown.react'),
plotly: () =>
Expand Down
19 changes: 19 additions & 0 deletions src/utils/MarkdownHighlighter.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
import LazyLoader from './LazyLoader';

export default class Highlight {
static hljsResolve() {}

constructor() {
if (!Highlight.isReady) {
Highlight.isReady = new Promise(resolve => {
Highlight.hljsResolve = resolve;
});
}
}

static async loadhljs() {
Highlight.hljs = await LazyLoader.hljs();
Highlight.hljsResolve();
Highlight.isReady = true;
}
}