From 5e6b334936f6f09f1718001309f9780f4a497516 Mon Sep 17 00:00:00 2001 From: David Taylor Date: Wed, 8 Jul 2020 16:53:59 +0100 Subject: [PATCH 1/5] PERF: Move highlightjs to a background worker, and add result cache Syntax highlighting is a CPU-intensive process which we run a lot while rendering posts and while using the composer preview. Moving it to a background worker releases the main thread to the browser, which makes the UX much smoother. --- .../admin/components/highlighted-code.js | 2 +- app/assets/javascripts/admin/models/theme.js | 2 +- .../app/initializers/post-decorations.js | 2 +- .../discourse/app/lib/highlight-syntax.js | 163 +++++++++++++++--- .../pre-initializers/discourse-bootstrap.js | 1 + app/assets/javascripts/highlightjs-worker.js | 47 +++++ app/helpers/application_helper.rb | 1 + config/application.rb | 1 + .../components/highlighted-code-test.js | 24 +-- 9 files changed, 206 insertions(+), 37 deletions(-) create mode 100644 app/assets/javascripts/highlightjs-worker.js diff --git a/app/assets/javascripts/admin/components/highlighted-code.js b/app/assets/javascripts/admin/components/highlighted-code.js index 9159bb574ada0b..6dfd98e25b5ec8 100644 --- a/app/assets/javascripts/admin/components/highlighted-code.js +++ b/app/assets/javascripts/admin/components/highlighted-code.js @@ -6,6 +6,6 @@ export default Component.extend({ @on("didInsertElement") @observes("code") _refresh: function() { - highlightSyntax($(this.element)); + highlightSyntax(this.element); } }); diff --git a/app/assets/javascripts/admin/models/theme.js b/app/assets/javascripts/admin/models/theme.js index 0b96660f99b2ca..d7bede73ef2259 100644 --- a/app/assets/javascripts/admin/models/theme.js +++ b/app/assets/javascripts/admin/models/theme.js @@ -321,7 +321,7 @@ const Theme = RestModel.extend({ } } ); - highlightSyntax(); + highlightSyntax(document.querySelector(".bootbox.modal")); } else { return this.save({ remote_update: true }).then(() => this.set("changed", false) diff --git a/app/assets/javascripts/discourse/app/initializers/post-decorations.js b/app/assets/javascripts/discourse/app/initializers/post-decorations.js index 7b7d543aa87680..aaaaa9e12958ce 100644 --- a/app/assets/javascripts/discourse/app/initializers/post-decorations.js +++ b/app/assets/javascripts/discourse/app/initializers/post-decorations.js @@ -9,7 +9,7 @@ export default { initialize(container) { withPluginApi("0.1", api => { const siteSettings = container.lookup("site-settings:main"); - api.decorateCooked(highlightSyntax, { + api.decorateCookedElement(highlightSyntax, { id: "discourse-syntax-highlighting" }); api.decorateCookedElement(lightbox, { id: "discourse-lightbox" }); diff --git a/app/assets/javascripts/discourse/app/lib/highlight-syntax.js b/app/assets/javascripts/discourse/app/lib/highlight-syntax.js index 404e73cadcce84..c756ed258b7ecb 100644 --- a/app/assets/javascripts/discourse/app/lib/highlight-syntax.js +++ b/app/assets/javascripts/discourse/app/lib/highlight-syntax.js @@ -1,40 +1,159 @@ -/*global hljs:true */ -let _moreLanguages = []; - +import { Promise } from "rsvp"; +import { getURLWithCDN } from "discourse-common/lib/get-url"; +import { next } from "@ember/runloop"; import loadScript from "discourse/lib/load-script"; +import { isTesting } from "discourse-common/config/environment"; -export default function highlightSyntax($elem) { - const selector = Discourse.SiteSettings.autohighlight_all_code - ? "pre code" - : "pre code[class]", - path = Discourse.HighlightJSPath; +const _moreLanguages = []; +let _worker = null; +let _workerPromise = null; +const _pendingResolution = {}; +let _counter = 0; +let _cachedResultsMap = new Map(); - if (!path) { - return; - } +const CACHE_SIZE = 100; - $(selector, $elem).each(function(i, e) { +export function registerHighlightJSLanguage(name, fn) { + _moreLanguages.push({ name: name, fn: fn }); +} + +export default function highlightSyntax(elem) { + const selector = Discourse.SiteSettings.autohighlight_all_code + ? "pre code" + : "pre code[class]"; + + elem.querySelectorAll(selector).forEach(function(e) { // Large code blocks can cause crashes or slowdowns if (e.innerHTML.length > 30000) { return; } - $(e).removeClass("lang-auto"); - loadScript(path).then(() => { - customHighlightJSLanguages(); - hljs.highlightBlock(e); + e.classList.remove("lang-auto"); + let lang = null; + e.classList.forEach(c => { + if (c.startsWith("lang-")) { + lang = c.slice("lang-".length); + } }); + + asyncHighlight(e.textContent, lang).then(res => + // Batch DOM changes using the Ember runloop + next(() => { + e.innerHTML = res; + e.classList.add("hljs"); + }) + ); }); } -export function registerHighlightJSLanguage(name, fn) { - _moreLanguages.push({ name: name, fn: fn }); +function getWorker() { + if (_worker) return Promise.resolve(_worker); + if (_workerPromise) return _workerPromise; + + const w = new Worker(Discourse.HighlightJSWorkerURL); + w.onmessage = onWorkerMessage; + w.postMessage({ + type: "loadHighlightJs", + path: highlightJSUrl() + }); + + _workerPromise = setupCustomLanguages(w).then(() => (_worker = w)); + return _workerPromise; } -function customHighlightJSLanguages() { - _moreLanguages.forEach(l => { - if (hljs.getLanguage(l.name) === undefined) { - hljs.registerLanguage(l.name, l.fn); +function setupCustomLanguages(worker) { + if (_moreLanguages.length === 0) return Promise.resolve(); + // To build custom language definitions we need to have hljs loaded + // Plugins/themes can't run code in a worker, so we have to load hljs in the main thread + // But the actual highlighting will still be done in the worker + + return loadScript(Discourse.HighlightJSPath).then(() => { + _moreLanguages.forEach(({ name, fn }) => { + const definition = fn(window.hljs); + worker.postMessage({ + type: "registerLanguage", + definition, + name + }); + }); + }); +} + +function asyncHighlight(text, language) { + return getWorker().then(w => { + let result; + if ((result = _cachedResultsMap.get(cacheKey(text, language)))) { + return result; } + + let resolve; + const promise = new Promise(f => (resolve = f)); + + w.postMessage({ + type: "highlight", + id: _counter, + text, + language + }); + + _pendingResolution[_counter] = { + promise, + resolve, + text, + language + }; + + _counter++; + + return promise; + }); +} + +function onWorkerMessage(message) { + const id = message.data.id; + const request = _pendingResolution[id]; + delete _pendingResolution[id]; + request.resolve(message.data.result); + + cacheResult({ + text: request.text, + language: request.language, + result: message.data.result + }); +} + +function cacheResult({ text, language, result }) { + _cachedResultsMap.set(cacheKey(text, language), result); + while (_cachedResultsMap.size > CACHE_SIZE) { + _cachedResultsMap.delete(_cachedResultsMap.entries().next().value[0]); + } +} + +function cacheKey(text, lang) { + return `${lang}:${text}`; +} + +function highlightJSUrl() { + let hljsUrl = getURLWithCDN(Discourse.HighlightJSPath); + + // Need to use full URL including protocol/domain + // for use in a worker + if (hljsUrl.startsWith("/")) { + hljsUrl = window.location.protocol + "//" + window.location.host + hljsUrl; + } + + return hljsUrl; +} + +// To be used in qunit tests. Running highlight in a worker means that the +// normal system which waits for ember rendering in tests doesn't work. +// This promise will resolve once all pending highlights are done +export function waitForHighlighting() { + if (!isTesting()) { + throw "This function should only be called in a test environment"; + } + const promises = Object.values(_pendingResolution).map(r => r.promise); + return new Promise(resolve => { + Promise.all(promises).then(() => next(resolve)); }); } diff --git a/app/assets/javascripts/discourse/app/pre-initializers/discourse-bootstrap.js b/app/assets/javascripts/discourse/app/pre-initializers/discourse-bootstrap.js index d8eb01dadc6971..277756e7afd115 100644 --- a/app/assets/javascripts/discourse/app/pre-initializers/discourse-bootstrap.js +++ b/app/assets/javascripts/discourse/app/pre-initializers/discourse-bootstrap.js @@ -82,6 +82,7 @@ export default { } app.HighlightJSPath = setupData.highlightJsPath; + app.HighlightJSWorkerURL = setupData.highlightJsWorkerUrl; app.SvgSpritePath = setupData.svgSpritePath; if (app.Environment === "development") { diff --git a/app/assets/javascripts/highlightjs-worker.js b/app/assets/javascripts/highlightjs-worker.js new file mode 100644 index 00000000000000..45413b302ec4ef --- /dev/null +++ b/app/assets/javascripts/highlightjs-worker.js @@ -0,0 +1,47 @@ +// Standalone worker for highlightjs syntax generation + +// The highlightjs path changes based on site settings, +// so we wait for Discourse to pass the path into the worker +const loadHighlightJs = path => { + self.importScripts(path); +}; + +const highlight = ({ id, text, language }) => { + if (!self.hljs) { + throw "HighlightJS is not loaded"; + } + + const result = language + ? self.hljs.highlight(language, text, true).value + : self.hljs.highlightAuto(text).value; + + postMessage({ + type: "highlightResult", + id: id, + result: result + }); +}; + +const registerLanguage = ({ name, definition }) => { + if (!self.hljs) { + throw "HighlightJS is not loaded"; + } + self.hljs.registerLanguage(name, () => { + return definition; + }); +}; + +onmessage = event => { + const data = event.data; + const messageType = data.type; + + if (messageType === "loadHighlightJs") { + loadHighlightJs(data.path); + } else if (messageType === "registerLanguage") { + registerLanguage(data); + } else if (messageType === "highlight") { + highlight(data); + } else { + throw `Unknown message type: ${messageType}`; + } +}; diff --git a/app/helpers/application_helper.rb b/app/helpers/application_helper.rb index f6bda658a38b53..d9747c653c5d98 100644 --- a/app/helpers/application_helper.rb +++ b/app/helpers/application_helper.rb @@ -470,6 +470,7 @@ def client_side_setup_data asset_version: Discourse.assets_digest, disable_custom_css: loading_admin?, highlight_js_path: HighlightJs.path, + highlight_js_worker_url: script_asset_path('highlightjs-worker'), svg_sprite_path: SvgSprite.path(theme_ids), enable_js_error_reporting: GlobalSetting.enable_js_error_reporting, } diff --git a/config/application.rb b/config/application.rb index 6120831f6dd140..194cc2623505ed 100644 --- a/config/application.rb +++ b/config/application.rb @@ -171,6 +171,7 @@ def config.database_configuration confirm-new-email/bootstrap.js onpopstate-handler.js embed-application.js + highlightjs-worker.js } # Precompile all available locales diff --git a/test/javascripts/components/highlighted-code-test.js b/test/javascripts/components/highlighted-code-test.js index 27cfe5696d30dc..6b9c41df742669 100644 --- a/test/javascripts/components/highlighted-code-test.js +++ b/test/javascripts/components/highlighted-code-test.js @@ -1,4 +1,5 @@ import componentTest from "helpers/component-test"; +import { waitForHighlighting } from "discourse/lib/highlight-syntax"; const LONG_CODE_BLOCK = "puts a\n".repeat(15000); @@ -9,30 +10,28 @@ componentTest("highlighting code", { beforeEach() { Discourse.HighlightJSPath = - "assets/highlightjs/highlight-test-bundle.min.js"; - this.set("code", "def test; end"); + "/assets/highlightjs/highlight-test-bundle.min.js"; + Discourse.HighlightJSWorkerURL = "/assets/highlightjs-worker.js"; }, async test(assert) { + this.set("code", "def test; end"); + + await waitForHighlighting(); + assert.equal( find("code.ruby.hljs .hljs-function .hljs-keyword") .text() .trim(), "def" ); - } -}); - -componentTest("large code blocks are not highlighted", { - template: "{{highlighted-code lang='ruby' code=code}}", - - beforeEach() { - Discourse.HighlightJSPath = - "assets/highlightjs/highlight-test-bundle.min.js"; - this.set("code", LONG_CODE_BLOCK); }, + async test(assert) { + this.set("code", LONG_CODE_BLOCK); + await waitForHighlighting(); + assert.equal( find("code") .text() @@ -40,4 +39,5 @@ componentTest("large code blocks are not highlighted", { LONG_CODE_BLOCK.trim() ); } + }); From 94dfa967c63031b2f1bb6ecbc164a50ce025237c Mon Sep 17 00:00:00 2001 From: David Taylor Date: Thu, 9 Jul 2020 20:47:13 +0100 Subject: [PATCH 2/5] Refactor to use Ember Service --- .../admin/components/highlighted-code.js | 13 +- app/assets/javascripts/admin/models/theme.js | 2 - .../app/initializers/post-decorations.js | 11 +- .../discourse/app/lib/highlight-syntax.js | 159 --------------- .../discourse/app/lib/plugin-api.js | 2 +- .../app/services/syntax-highlighter.js | 181 ++++++++++++++++++ .../components/highlighted-code-test.js | 16 +- 7 files changed, 207 insertions(+), 177 deletions(-) delete mode 100644 app/assets/javascripts/discourse/app/lib/highlight-syntax.js create mode 100644 app/assets/javascripts/discourse/app/services/syntax-highlighter.js diff --git a/app/assets/javascripts/admin/components/highlighted-code.js b/app/assets/javascripts/admin/components/highlighted-code.js index 6dfd98e25b5ec8..0a152d48344521 100644 --- a/app/assets/javascripts/admin/components/highlighted-code.js +++ b/app/assets/javascripts/admin/components/highlighted-code.js @@ -1,11 +1,12 @@ import Component from "@ember/component"; -import { on, observes } from "discourse-common/utils/decorators"; -import highlightSyntax from "discourse/lib/highlight-syntax"; +import { inject as service } from "@ember/service"; export default Component.extend({ - @on("didInsertElement") - @observes("code") - _refresh: function() { - highlightSyntax(this.element); + syntaxHighlighter: service(), + + didRender() { + if (!this.element.querySelector("code").classList.contains("hljs")) { + this.syntaxHighlighter.highlightElements(this.element); + } } }); diff --git a/app/assets/javascripts/admin/models/theme.js b/app/assets/javascripts/admin/models/theme.js index d7bede73ef2259..464dedb54d105d 100644 --- a/app/assets/javascripts/admin/models/theme.js +++ b/app/assets/javascripts/admin/models/theme.js @@ -7,7 +7,6 @@ import discourseComputed from "discourse-common/utils/decorators"; import { popupAjaxError } from "discourse/lib/ajax-error"; import { ajax } from "discourse/lib/ajax"; import { escapeExpression } from "discourse/lib/utilities"; -import highlightSyntax from "discourse/lib/highlight-syntax"; import { url } from "discourse/lib/computed"; const THEME_UPLOAD_VAR = 2; @@ -321,7 +320,6 @@ const Theme = RestModel.extend({ } } ); - highlightSyntax(document.querySelector(".bootbox.modal")); } else { return this.save({ remote_update: true }).then(() => this.set("changed", false) diff --git a/app/assets/javascripts/discourse/app/initializers/post-decorations.js b/app/assets/javascripts/discourse/app/initializers/post-decorations.js index aaaaa9e12958ce..0d01dafd8ade9e 100644 --- a/app/assets/javascripts/discourse/app/initializers/post-decorations.js +++ b/app/assets/javascripts/discourse/app/initializers/post-decorations.js @@ -1,4 +1,3 @@ -import highlightSyntax from "discourse/lib/highlight-syntax"; import lightbox from "discourse/lib/lightbox"; import { setupLazyLoading } from "discourse/lib/lazy-load-images"; import { setTextDirections } from "discourse/lib/text-direction"; @@ -9,9 +8,13 @@ export default { initialize(container) { withPluginApi("0.1", api => { const siteSettings = container.lookup("site-settings:main"); - api.decorateCookedElement(highlightSyntax, { - id: "discourse-syntax-highlighting" - }); + const syntaxHighligher = container.lookup("service:syntax-highlighter"); + api.decorateCookedElement( + elem => syntaxHighligher.highlightElements(elem), + { + id: "discourse-syntax-highlighting" + } + ); api.decorateCookedElement(lightbox, { id: "discourse-lightbox" }); if (siteSettings.support_mixed_text_direction) { api.decorateCooked(setTextDirections, { diff --git a/app/assets/javascripts/discourse/app/lib/highlight-syntax.js b/app/assets/javascripts/discourse/app/lib/highlight-syntax.js deleted file mode 100644 index c756ed258b7ecb..00000000000000 --- a/app/assets/javascripts/discourse/app/lib/highlight-syntax.js +++ /dev/null @@ -1,159 +0,0 @@ -import { Promise } from "rsvp"; -import { getURLWithCDN } from "discourse-common/lib/get-url"; -import { next } from "@ember/runloop"; -import loadScript from "discourse/lib/load-script"; -import { isTesting } from "discourse-common/config/environment"; - -const _moreLanguages = []; -let _worker = null; -let _workerPromise = null; -const _pendingResolution = {}; -let _counter = 0; -let _cachedResultsMap = new Map(); - -const CACHE_SIZE = 100; - -export function registerHighlightJSLanguage(name, fn) { - _moreLanguages.push({ name: name, fn: fn }); -} - -export default function highlightSyntax(elem) { - const selector = Discourse.SiteSettings.autohighlight_all_code - ? "pre code" - : "pre code[class]"; - - elem.querySelectorAll(selector).forEach(function(e) { - // Large code blocks can cause crashes or slowdowns - if (e.innerHTML.length > 30000) { - return; - } - - e.classList.remove("lang-auto"); - let lang = null; - e.classList.forEach(c => { - if (c.startsWith("lang-")) { - lang = c.slice("lang-".length); - } - }); - - asyncHighlight(e.textContent, lang).then(res => - // Batch DOM changes using the Ember runloop - next(() => { - e.innerHTML = res; - e.classList.add("hljs"); - }) - ); - }); -} - -function getWorker() { - if (_worker) return Promise.resolve(_worker); - if (_workerPromise) return _workerPromise; - - const w = new Worker(Discourse.HighlightJSWorkerURL); - w.onmessage = onWorkerMessage; - w.postMessage({ - type: "loadHighlightJs", - path: highlightJSUrl() - }); - - _workerPromise = setupCustomLanguages(w).then(() => (_worker = w)); - return _workerPromise; -} - -function setupCustomLanguages(worker) { - if (_moreLanguages.length === 0) return Promise.resolve(); - // To build custom language definitions we need to have hljs loaded - // Plugins/themes can't run code in a worker, so we have to load hljs in the main thread - // But the actual highlighting will still be done in the worker - - return loadScript(Discourse.HighlightJSPath).then(() => { - _moreLanguages.forEach(({ name, fn }) => { - const definition = fn(window.hljs); - worker.postMessage({ - type: "registerLanguage", - definition, - name - }); - }); - }); -} - -function asyncHighlight(text, language) { - return getWorker().then(w => { - let result; - if ((result = _cachedResultsMap.get(cacheKey(text, language)))) { - return result; - } - - let resolve; - const promise = new Promise(f => (resolve = f)); - - w.postMessage({ - type: "highlight", - id: _counter, - text, - language - }); - - _pendingResolution[_counter] = { - promise, - resolve, - text, - language - }; - - _counter++; - - return promise; - }); -} - -function onWorkerMessage(message) { - const id = message.data.id; - const request = _pendingResolution[id]; - delete _pendingResolution[id]; - request.resolve(message.data.result); - - cacheResult({ - text: request.text, - language: request.language, - result: message.data.result - }); -} - -function cacheResult({ text, language, result }) { - _cachedResultsMap.set(cacheKey(text, language), result); - while (_cachedResultsMap.size > CACHE_SIZE) { - _cachedResultsMap.delete(_cachedResultsMap.entries().next().value[0]); - } -} - -function cacheKey(text, lang) { - return `${lang}:${text}`; -} - -function highlightJSUrl() { - let hljsUrl = getURLWithCDN(Discourse.HighlightJSPath); - - // Need to use full URL including protocol/domain - // for use in a worker - if (hljsUrl.startsWith("/")) { - hljsUrl = window.location.protocol + "//" + window.location.host + hljsUrl; - } - - return hljsUrl; -} - -// To be used in qunit tests. Running highlight in a worker means that the -// normal system which waits for ember rendering in tests doesn't work. -// This promise will resolve once all pending highlights are done -export function waitForHighlighting() { - if (!isTesting()) { - throw "This function should only be called in a test environment"; - } - const promises = Object.values(_pendingResolution).map(r => r.promise); - return new Promise(resolve => { - Promise.all(promises).then(() => next(resolve)); - }); -} diff --git a/app/assets/javascripts/discourse/app/lib/plugin-api.js b/app/assets/javascripts/discourse/app/lib/plugin-api.js index a157f01a0fe420..575e14913c05b2 100644 --- a/app/assets/javascripts/discourse/app/lib/plugin-api.js +++ b/app/assets/javascripts/discourse/app/lib/plugin-api.js @@ -7,7 +7,7 @@ import ComposerEditor from "discourse/components/composer-editor"; import DiscourseBanner from "discourse/components/discourse-banner"; import { addButton, removeButton } from "discourse/widgets/post-menu"; import { includeAttributes } from "discourse/lib/transform-post"; -import { registerHighlightJSLanguage } from "discourse/lib/highlight-syntax"; +import { registerHighlightJSLanguage } from "discourse/services/syntax-highlighter"; import { addToolbarCallback } from "discourse/components/d-editor"; import { addWidgetCleanCallback } from "discourse/components/mount-widget"; import { addGlobalNotice } from "discourse/components/global-notice"; diff --git a/app/assets/javascripts/discourse/app/services/syntax-highlighter.js b/app/assets/javascripts/discourse/app/services/syntax-highlighter.js new file mode 100644 index 00000000000000..6e4ab456454496 --- /dev/null +++ b/app/assets/javascripts/discourse/app/services/syntax-highlighter.js @@ -0,0 +1,181 @@ +import Service from "@ember/service"; +import { Promise } from "rsvp"; +import { getURLWithCDN } from "discourse-common/lib/get-url"; +import { next, schedule } from "@ember/runloop"; +import loadScript from "discourse/lib/load-script"; +import { isTesting } from "discourse-common/config/environment"; + +const _moreLanguages = []; +let _worker = null; +let _workerPromise = null; +const _pendingResolution = {}; +let _counter = 0; +let _cachedResultsMap = new Map(); +const CACHE_SIZE = 100; + +export function registerHighlightJSLanguage(name, fn) { + _moreLanguages.push({ name: name, fn: fn }); +} + +export default Service.extend({ + highlightElements(elem) { + const selector = + this.siteSettings && this.siteSettings.autohighlight_all_code + ? "pre code" + : "pre code[class]"; + + elem.querySelectorAll(selector).forEach(e => this.highlightElement(e)); + }, + + highlightElement(e) { + // Large code blocks can cause crashes or slowdowns + if (e.innerHTML.length > 30000) { + return; + } + + e.classList.remove("lang-auto"); + let lang = null; + e.classList.forEach(c => { + if (c.startsWith("lang-")) { + lang = c.slice("lang-".length); + } + }); + + const requestString = e.textContent; + this.asyncHighlightText(e.textContent, lang).then( + ({ result, fromCache }) => { + // Ensure the code hasn't changed since highlighting was triggered: + if (requestString !== e.textContent) return; + + const doRender = () => { + e.innerHTML = result; + e.classList.add("hljs"); + }; + + if (fromCache) { + // This happened synchronously, we can safely add rendering + // to the end of the current Runloop + schedule("afterRender", doRender); + } else { + // This happened async, we are probably not in a runloop + // If we call `schedule`, a new runloop will be triggered immediately + // So schedule rendering to happen in the next runloop + next(doRender); + } + } + ); + }, + + asyncHighlightText(text, language) { + return this._getWorker().then(w => { + let result; + if ((result = _cachedResultsMap.get(this._cacheKey(text, language)))) { + return Promise.resolve({ result, fromCache: true }); + } + + let resolve; + const promise = new Promise(f => (resolve = f)); + + w.postMessage({ + type: "highlight", + id: _counter, + text, + language + }); + + _pendingResolution[_counter] = { + promise, + resolve, + text, + language + }; + + _counter++; + + return promise; + }); + }, + + _getWorker() { + if (_worker) return Promise.resolve(_worker); + if (_workerPromise) return _workerPromise; + + const w = new Worker(Discourse.HighlightJSWorkerURL); + w.onmessage = message => this._onWorkerMessage(message); + w.postMessage({ + type: "loadHighlightJs", + path: this._highlightJSUrl() + }); + + _workerPromise = this._setupCustomLanguages(w).then(() => (_worker = w)); + return _workerPromise; + }, + + _setupCustomLanguages(worker) { + if (_moreLanguages.length === 0) return Promise.resolve(); + // To build custom language definitions we need to have hljs loaded + // Plugins/themes can't run code in a worker, so we have to load hljs in the main thread + // But the actual highlighting will still be done in the worker + + return loadScript(Discourse.HighlightJSPath).then(() => { + _moreLanguages.forEach(({ name, fn }) => { + const definition = fn(window.hljs); + worker.postMessage({ + type: "registerLanguage", + definition, + name + }); + }); + }); + }, + + _onWorkerMessage(message) { + const id = message.data.id; + const request = _pendingResolution[id]; + delete _pendingResolution[id]; + request.resolve({ result: message.data.result, fromCache: false }); + + this._cacheResult({ + text: request.text, + language: request.language, + result: message.data.result + }); + }, + + _cacheResult({ text, language, result }) { + _cachedResultsMap.set(this._cacheKey(text, language), result); + while (_cachedResultsMap.size > CACHE_SIZE) { + _cachedResultsMap.delete(_cachedResultsMap.entries().next().value[0]); + } + }, + + _cacheKey(text, lang) { + return `${lang}:${text}`; + }, + + _highlightJSUrl() { + let hljsUrl = getURLWithCDN(Discourse.HighlightJSPath); + + // Need to use full URL including protocol/domain + // for use in a worker + if (hljsUrl.startsWith("/")) { + hljsUrl = + window.location.protocol + "//" + window.location.host + hljsUrl; + } + + return hljsUrl; + } +}); + +// To be used in qunit tests. Running highlight in a worker means that the +// normal system which waits for ember rendering in tests doesn't work. +// This promise will resolve once all pending highlights are done +export function waitForHighlighting() { + if (!isTesting()) { + throw "This function should only be called in a test environment"; + } + const promises = Object.values(_pendingResolution).map(r => r.promise); + return new Promise(resolve => { + Promise.all(promises).then(() => next(resolve)); + }); +} diff --git a/test/javascripts/components/highlighted-code-test.js b/test/javascripts/components/highlighted-code-test.js index 6b9c41df742669..e9e8d364ccd161 100644 --- a/test/javascripts/components/highlighted-code-test.js +++ b/test/javascripts/components/highlighted-code-test.js @@ -1,5 +1,5 @@ import componentTest from "helpers/component-test"; -import { waitForHighlighting } from "discourse/lib/highlight-syntax"; +import { waitForHighlighting } from "discourse/services/syntax-highlighter"; const LONG_CODE_BLOCK = "puts a\n".repeat(15000); @@ -18,20 +18,27 @@ componentTest("highlighting code", { this.set("code", "def test; end"); await waitForHighlighting(); - assert.equal( find("code.ruby.hljs .hljs-function .hljs-keyword") .text() .trim(), "def" ); + } +}); + +componentTest("highlighting code limit", { + template: "{{highlighted-code lang='ruby' code=code}}", + + beforeEach() { + Discourse.HighlightJSPath = + "/assets/highlightjs/highlight-test-bundle.min.js"; + Discourse.HighlightJSWorkerURL = "/assets/highlightjs-worker.js"; }, - async test(assert) { this.set("code", LONG_CODE_BLOCK); await waitForHighlighting(); - assert.equal( find("code") .text() @@ -39,5 +46,4 @@ componentTest("highlighting code", { LONG_CODE_BLOCK.trim() ); } - }); From d2b3f9a5c7a6a09c7481ab384dffa8a77e3fc07e Mon Sep 17 00:00:00 2001 From: David Taylor Date: Thu, 9 Jul 2020 21:52:25 +0100 Subject: [PATCH 3/5] Avoid using Discourse. to store js urls --- .../app/pre-initializers/discourse-bootstrap.js | 8 ++++++-- .../discourse/app/services/syntax-highlighter.js | 15 ++++++++++++--- app/helpers/application_helper.rb | 2 +- 3 files changed, 19 insertions(+), 6 deletions(-) diff --git a/app/assets/javascripts/discourse/app/pre-initializers/discourse-bootstrap.js b/app/assets/javascripts/discourse/app/pre-initializers/discourse-bootstrap.js index 277756e7afd115..6cc511c05e8f12 100644 --- a/app/assets/javascripts/discourse/app/pre-initializers/discourse-bootstrap.js +++ b/app/assets/javascripts/discourse/app/pre-initializers/discourse-bootstrap.js @@ -9,6 +9,7 @@ import { } from "discourse-common/config/environment"; import { setupURL, setupS3CDN } from "discourse-common/lib/get-url"; import deprecated from "discourse-common/lib/deprecated"; +import { setupHighlightJs } from "discourse/services/syntax-highlighter"; export default { name: "discourse-bootstrap", @@ -81,8 +82,11 @@ export default { Session.currentProp("safe_mode", setupData.safeMode); } - app.HighlightJSPath = setupData.highlightJsPath; - app.HighlightJSWorkerURL = setupData.highlightJsWorkerUrl; + setupHighlightJs({ + highlightJsUrl: setupData.highlightJsUrl, + highlightJsWorkerUrl: setupData.highlightJsWorkerUrl + }); + app.SvgSpritePath = setupData.svgSpritePath; if (app.Environment === "development") { diff --git a/app/assets/javascripts/discourse/app/services/syntax-highlighter.js b/app/assets/javascripts/discourse/app/services/syntax-highlighter.js index 6e4ab456454496..23653f5429be9e 100644 --- a/app/assets/javascripts/discourse/app/services/syntax-highlighter.js +++ b/app/assets/javascripts/discourse/app/services/syntax-highlighter.js @@ -5,14 +5,23 @@ import { next, schedule } from "@ember/runloop"; import loadScript from "discourse/lib/load-script"; import { isTesting } from "discourse-common/config/environment"; +let highlightJsUrl; +let highlightJsWorkerUrl; + const _moreLanguages = []; let _worker = null; let _workerPromise = null; const _pendingResolution = {}; let _counter = 0; let _cachedResultsMap = new Map(); + const CACHE_SIZE = 100; +export function setupHighlightJs(args) { + highlightJsUrl = args.highlightJsUrl; + highlightJsWorkerUrl = args.highlightJsWorkerUrl; +} + export function registerHighlightJSLanguage(name, fn) { _moreLanguages.push({ name: name, fn: fn }); } @@ -100,7 +109,7 @@ export default Service.extend({ if (_worker) return Promise.resolve(_worker); if (_workerPromise) return _workerPromise; - const w = new Worker(Discourse.HighlightJSWorkerURL); + const w = new Worker(highlightJsWorkerUrl); w.onmessage = message => this._onWorkerMessage(message); w.postMessage({ type: "loadHighlightJs", @@ -117,7 +126,7 @@ export default Service.extend({ // Plugins/themes can't run code in a worker, so we have to load hljs in the main thread // But the actual highlighting will still be done in the worker - return loadScript(Discourse.HighlightJSPath).then(() => { + return loadScript(highlightJsUrl).then(() => { _moreLanguages.forEach(({ name, fn }) => { const definition = fn(window.hljs); worker.postMessage({ @@ -154,7 +163,7 @@ export default Service.extend({ }, _highlightJSUrl() { - let hljsUrl = getURLWithCDN(Discourse.HighlightJSPath); + let hljsUrl = getURLWithCDN(highlightJsUrl); // Need to use full URL including protocol/domain // for use in a worker diff --git a/app/helpers/application_helper.rb b/app/helpers/application_helper.rb index d9747c653c5d98..c051ec5b316e93 100644 --- a/app/helpers/application_helper.rb +++ b/app/helpers/application_helper.rb @@ -469,7 +469,7 @@ def client_side_setup_data default_locale: SiteSetting.default_locale, asset_version: Discourse.assets_digest, disable_custom_css: loading_admin?, - highlight_js_path: HighlightJs.path, + highlight_js_url: HighlightJs.path, highlight_js_worker_url: script_asset_path('highlightjs-worker'), svg_sprite_path: SvgSprite.path(theme_ids), enable_js_error_reporting: GlobalSetting.enable_js_error_reporting, From 38fc6f8ba50d0e1b66244ac1f8e22d8045b2798d Mon Sep 17 00:00:00 2001 From: David Taylor Date: Tue, 14 Jul 2020 15:06:10 +0100 Subject: [PATCH 4/5] Switch back to basic module, and add setupHighlightJs method for config --- .../admin/components/highlighted-code.js | 37 +++- .../templates/components/highlighted-code.hbs | 2 +- .../app/initializers/post-decorations.js | 11 +- .../discourse/app/lib/highlight-syntax.js | 181 +++++++++++++++++ .../discourse/app/lib/plugin-api.js | 2 +- .../pre-initializers/discourse-bootstrap.js | 2 +- .../app/services/syntax-highlighter.js | 190 ------------------ .../components/highlighted-code-test.js | 20 +- 8 files changed, 231 insertions(+), 214 deletions(-) create mode 100644 app/assets/javascripts/discourse/app/lib/highlight-syntax.js delete mode 100644 app/assets/javascripts/discourse/app/services/syntax-highlighter.js diff --git a/app/assets/javascripts/admin/components/highlighted-code.js b/app/assets/javascripts/admin/components/highlighted-code.js index 0a152d48344521..30f89e408505c4 100644 --- a/app/assets/javascripts/admin/components/highlighted-code.js +++ b/app/assets/javascripts/admin/components/highlighted-code.js @@ -1,12 +1,37 @@ import Component from "@ember/component"; -import { inject as service } from "@ember/service"; +import { highlightText } from "discourse/lib/highlight-syntax"; +import { escapeExpression } from "discourse/lib/utilities"; +import discourseComputed from "discourse-common/utils/decorators"; +import { htmlSafe } from "@ember/template"; export default Component.extend({ - syntaxHighlighter: service(), + didReceiveAttrs() { + this._super(...arguments); + if (this.code === this.previousCode) return; - didRender() { - if (!this.element.querySelector("code").classList.contains("hljs")) { - this.syntaxHighlighter.highlightElements(this.element); - } + this.set("previousCode", this.code); + this.set("highlightResult", null); + const toHighlight = this.code; + highlightText(escapeExpression(toHighlight), this.lang).then( + ({ result }) => { + if (toHighlight !== this.code) return; // Code has changed since highlight was requested + this.set("highlightResult", result); + } + ); + }, + + @discourseComputed("code", "highlightResult") + displayCode(code, highlightResult) { + if (highlightResult) return htmlSafe(highlightResult); + return code; + }, + + @discourseComputed("highlightResult", "lang") + codeClasses(highlightResult, lang) { + const classes = []; + if (lang) classes.push(lang); + if (highlightResult) classes.push("hljs"); + + return classes.join(" "); } }); diff --git a/app/assets/javascripts/admin/templates/components/highlighted-code.hbs b/app/assets/javascripts/admin/templates/components/highlighted-code.hbs index 4d67dd6fd2e834..5dd40c0476b3c6 100644 --- a/app/assets/javascripts/admin/templates/components/highlighted-code.hbs +++ b/app/assets/javascripts/admin/templates/components/highlighted-code.hbs @@ -1 +1 @@ -
{{code}}
+
{{displayCode}}
diff --git a/app/assets/javascripts/discourse/app/initializers/post-decorations.js b/app/assets/javascripts/discourse/app/initializers/post-decorations.js index 0d01dafd8ade9e..d1205bcfdb15cd 100644 --- a/app/assets/javascripts/discourse/app/initializers/post-decorations.js +++ b/app/assets/javascripts/discourse/app/initializers/post-decorations.js @@ -2,19 +2,16 @@ import lightbox from "discourse/lib/lightbox"; import { setupLazyLoading } from "discourse/lib/lazy-load-images"; import { setTextDirections } from "discourse/lib/text-direction"; import { withPluginApi } from "discourse/lib/plugin-api"; +import highlightSyntax from "discourse/lib/highlight-syntax"; export default { name: "post-decorations", initialize(container) { withPluginApi("0.1", api => { const siteSettings = container.lookup("site-settings:main"); - const syntaxHighligher = container.lookup("service:syntax-highlighter"); - api.decorateCookedElement( - elem => syntaxHighligher.highlightElements(elem), - { - id: "discourse-syntax-highlighting" - } - ); + api.decorateCookedElement(highlightSyntax, { + id: "discourse-syntax-highlighting" + }); api.decorateCookedElement(lightbox, { id: "discourse-lightbox" }); if (siteSettings.support_mixed_text_direction) { api.decorateCooked(setTextDirections, { diff --git a/app/assets/javascripts/discourse/app/lib/highlight-syntax.js b/app/assets/javascripts/discourse/app/lib/highlight-syntax.js new file mode 100644 index 00000000000000..302fd4e62a0bf0 --- /dev/null +++ b/app/assets/javascripts/discourse/app/lib/highlight-syntax.js @@ -0,0 +1,181 @@ +import { Promise } from "rsvp"; +import { getURLWithCDN } from "discourse-common/lib/get-url"; +import { next, schedule } from "@ember/runloop"; +import loadScript from "discourse/lib/load-script"; +import { isTesting } from "discourse-common/config/environment"; + +let highlightJsUrl; +let highlightJsWorkerUrl; + +const _moreLanguages = []; +let _worker = null; +let _workerPromise = null; +const _pendingResolution = {}; +let _counter = 0; +let _cachedResultsMap = new Map(); + +const CACHE_SIZE = 100; + +export function setupHighlightJs(args) { + highlightJsUrl = args.highlightJsUrl; + highlightJsWorkerUrl = args.highlightJsWorkerUrl; +} + +export function registerHighlightJSLanguage(name, fn) { + _moreLanguages.push({ name: name, fn: fn }); +} + +export default function highlightSyntax(elem, { autoHighlight = false } = {}) { + const selector = autoHighlight ? "pre code" : "pre code[class]"; + + elem.querySelectorAll(selector).forEach(e => highlightElement(e)); +} + +function highlightElement(e) { + e.classList.remove("lang-auto"); + let lang = null; + e.classList.forEach(c => { + if (c.startsWith("lang-")) { + lang = c.slice("lang-".length); + } + }); + + const requestString = e.textContent; + highlightText(e.textContent, lang).then(({ result, fromCache }) => { + const doRender = () => { + // Ensure the code hasn't changed since highlighting was triggered: + if (requestString !== e.textContent) return; + + e.innerHTML = result; + e.classList.add("hljs"); + }; + + if (fromCache) { + // This happened synchronously, we can safely add rendering + // to the end of the current Runloop + schedule("afterRender", null, doRender); + } else { + // This happened async, we are probably not in a runloop + // If we call `schedule`, a new runloop will be triggered immediately + // So schedule rendering to happen in the next runloop + next(() => schedule("afterRender", null, doRender)); + } + }); +} + +export function highlightText(text, language) { + // Large code blocks can cause crashes or slowdowns + if (text.length > 30000) { + return Promise.resolve({ result: text, fromCache: true }); + } + + return getWorker().then(w => { + let result; + if ((result = _cachedResultsMap.get(cacheKey(text, language)))) { + return Promise.resolve({ result, fromCache: true }); + } + + let resolve; + const promise = new Promise(f => (resolve = f)); + + w.postMessage({ + type: "highlight", + id: _counter, + text, + language + }); + + _pendingResolution[_counter] = { + promise, + resolve, + text, + language + }; + + _counter++; + + return promise; + }); +} + +function getWorker() { + if (_worker) return Promise.resolve(_worker); + if (_workerPromise) return _workerPromise; + + const w = new Worker(highlightJsWorkerUrl); + w.onmessage = onWorkerMessage; + w.postMessage({ + type: "loadHighlightJs", + path: fullHighlightJsUrl() + }); + + _workerPromise = setupCustomLanguages(w).then(() => (_worker = w)); + return _workerPromise; +} + +function setupCustomLanguages(worker) { + if (_moreLanguages.length === 0) return Promise.resolve(); + // To build custom language definitions we need to have hljs loaded + // Plugins/themes can't run code in a worker, so we have to load hljs in the main thread + // But the actual highlighting will still be done in the worker + + return loadScript(highlightJsUrl).then(() => { + _moreLanguages.forEach(({ name, fn }) => { + const definition = fn(window.hljs); + worker.postMessage({ + type: "registerLanguage", + definition, + name + }); + }); + }); +} + +function onWorkerMessage(message) { + const id = message.data.id; + const request = _pendingResolution[id]; + delete _pendingResolution[id]; + request.resolve({ result: message.data.result, fromCache: false }); + + cacheResult({ + text: request.text, + language: request.language, + result: message.data.result + }); +} + +function cacheResult({ text, language, result }) { + _cachedResultsMap.set(cacheKey(text, language), result); + while (_cachedResultsMap.size > CACHE_SIZE) { + _cachedResultsMap.delete(_cachedResultsMap.entries().next().value[0]); + } +} + +function cacheKey(text, lang) { + return `${lang}:${text}`; +} + +function fullHighlightJsUrl() { + let hljsUrl = getURLWithCDN(highlightJsUrl); + + // Need to use full URL including protocol/domain + // for use in a worker + if (hljsUrl.startsWith("/")) { + hljsUrl = window.location.protocol + "//" + window.location.host + hljsUrl; + } + + return hljsUrl; +} + +// To be used in qunit tests. Running highlight in a worker means that the +// normal system which waits for ember rendering in tests doesn't work. +// This promise will resolve once all pending highlights are done +export function waitForHighlighting() { + if (!isTesting()) { + throw "This function should only be called in a test environment"; + } + const promises = Object.values(_pendingResolution).map(r => r.promise); + return new Promise(resolve => { + Promise.all(promises).then(() => next(resolve)); + }); +} diff --git a/app/assets/javascripts/discourse/app/lib/plugin-api.js b/app/assets/javascripts/discourse/app/lib/plugin-api.js index 575e14913c05b2..a157f01a0fe420 100644 --- a/app/assets/javascripts/discourse/app/lib/plugin-api.js +++ b/app/assets/javascripts/discourse/app/lib/plugin-api.js @@ -7,7 +7,7 @@ import ComposerEditor from "discourse/components/composer-editor"; import DiscourseBanner from "discourse/components/discourse-banner"; import { addButton, removeButton } from "discourse/widgets/post-menu"; import { includeAttributes } from "discourse/lib/transform-post"; -import { registerHighlightJSLanguage } from "discourse/services/syntax-highlighter"; +import { registerHighlightJSLanguage } from "discourse/lib/highlight-syntax"; import { addToolbarCallback } from "discourse/components/d-editor"; import { addWidgetCleanCallback } from "discourse/components/mount-widget"; import { addGlobalNotice } from "discourse/components/global-notice"; diff --git a/app/assets/javascripts/discourse/app/pre-initializers/discourse-bootstrap.js b/app/assets/javascripts/discourse/app/pre-initializers/discourse-bootstrap.js index 6cc511c05e8f12..54f55d897f35f5 100644 --- a/app/assets/javascripts/discourse/app/pre-initializers/discourse-bootstrap.js +++ b/app/assets/javascripts/discourse/app/pre-initializers/discourse-bootstrap.js @@ -9,7 +9,7 @@ import { } from "discourse-common/config/environment"; import { setupURL, setupS3CDN } from "discourse-common/lib/get-url"; import deprecated from "discourse-common/lib/deprecated"; -import { setupHighlightJs } from "discourse/services/syntax-highlighter"; +import { setupHighlightJs } from "discourse/lib/highlight-syntax"; export default { name: "discourse-bootstrap", diff --git a/app/assets/javascripts/discourse/app/services/syntax-highlighter.js b/app/assets/javascripts/discourse/app/services/syntax-highlighter.js deleted file mode 100644 index 23653f5429be9e..00000000000000 --- a/app/assets/javascripts/discourse/app/services/syntax-highlighter.js +++ /dev/null @@ -1,190 +0,0 @@ -import Service from "@ember/service"; -import { Promise } from "rsvp"; -import { getURLWithCDN } from "discourse-common/lib/get-url"; -import { next, schedule } from "@ember/runloop"; -import loadScript from "discourse/lib/load-script"; -import { isTesting } from "discourse-common/config/environment"; - -let highlightJsUrl; -let highlightJsWorkerUrl; - -const _moreLanguages = []; -let _worker = null; -let _workerPromise = null; -const _pendingResolution = {}; -let _counter = 0; -let _cachedResultsMap = new Map(); - -const CACHE_SIZE = 100; - -export function setupHighlightJs(args) { - highlightJsUrl = args.highlightJsUrl; - highlightJsWorkerUrl = args.highlightJsWorkerUrl; -} - -export function registerHighlightJSLanguage(name, fn) { - _moreLanguages.push({ name: name, fn: fn }); -} - -export default Service.extend({ - highlightElements(elem) { - const selector = - this.siteSettings && this.siteSettings.autohighlight_all_code - ? "pre code" - : "pre code[class]"; - - elem.querySelectorAll(selector).forEach(e => this.highlightElement(e)); - }, - - highlightElement(e) { - // Large code blocks can cause crashes or slowdowns - if (e.innerHTML.length > 30000) { - return; - } - - e.classList.remove("lang-auto"); - let lang = null; - e.classList.forEach(c => { - if (c.startsWith("lang-")) { - lang = c.slice("lang-".length); - } - }); - - const requestString = e.textContent; - this.asyncHighlightText(e.textContent, lang).then( - ({ result, fromCache }) => { - // Ensure the code hasn't changed since highlighting was triggered: - if (requestString !== e.textContent) return; - - const doRender = () => { - e.innerHTML = result; - e.classList.add("hljs"); - }; - - if (fromCache) { - // This happened synchronously, we can safely add rendering - // to the end of the current Runloop - schedule("afterRender", doRender); - } else { - // This happened async, we are probably not in a runloop - // If we call `schedule`, a new runloop will be triggered immediately - // So schedule rendering to happen in the next runloop - next(doRender); - } - } - ); - }, - - asyncHighlightText(text, language) { - return this._getWorker().then(w => { - let result; - if ((result = _cachedResultsMap.get(this._cacheKey(text, language)))) { - return Promise.resolve({ result, fromCache: true }); - } - - let resolve; - const promise = new Promise(f => (resolve = f)); - - w.postMessage({ - type: "highlight", - id: _counter, - text, - language - }); - - _pendingResolution[_counter] = { - promise, - resolve, - text, - language - }; - - _counter++; - - return promise; - }); - }, - - _getWorker() { - if (_worker) return Promise.resolve(_worker); - if (_workerPromise) return _workerPromise; - - const w = new Worker(highlightJsWorkerUrl); - w.onmessage = message => this._onWorkerMessage(message); - w.postMessage({ - type: "loadHighlightJs", - path: this._highlightJSUrl() - }); - - _workerPromise = this._setupCustomLanguages(w).then(() => (_worker = w)); - return _workerPromise; - }, - - _setupCustomLanguages(worker) { - if (_moreLanguages.length === 0) return Promise.resolve(); - // To build custom language definitions we need to have hljs loaded - // Plugins/themes can't run code in a worker, so we have to load hljs in the main thread - // But the actual highlighting will still be done in the worker - - return loadScript(highlightJsUrl).then(() => { - _moreLanguages.forEach(({ name, fn }) => { - const definition = fn(window.hljs); - worker.postMessage({ - type: "registerLanguage", - definition, - name - }); - }); - }); - }, - - _onWorkerMessage(message) { - const id = message.data.id; - const request = _pendingResolution[id]; - delete _pendingResolution[id]; - request.resolve({ result: message.data.result, fromCache: false }); - - this._cacheResult({ - text: request.text, - language: request.language, - result: message.data.result - }); - }, - - _cacheResult({ text, language, result }) { - _cachedResultsMap.set(this._cacheKey(text, language), result); - while (_cachedResultsMap.size > CACHE_SIZE) { - _cachedResultsMap.delete(_cachedResultsMap.entries().next().value[0]); - } - }, - - _cacheKey(text, lang) { - return `${lang}:${text}`; - }, - - _highlightJSUrl() { - let hljsUrl = getURLWithCDN(highlightJsUrl); - - // Need to use full URL including protocol/domain - // for use in a worker - if (hljsUrl.startsWith("/")) { - hljsUrl = - window.location.protocol + "//" + window.location.host + hljsUrl; - } - - return hljsUrl; - } -}); - -// To be used in qunit tests. Running highlight in a worker means that the -// normal system which waits for ember rendering in tests doesn't work. -// This promise will resolve once all pending highlights are done -export function waitForHighlighting() { - if (!isTesting()) { - throw "This function should only be called in a test environment"; - } - const promises = Object.values(_pendingResolution).map(r => r.promise); - return new Promise(resolve => { - Promise.all(promises).then(() => next(resolve)); - }); -} diff --git a/test/javascripts/components/highlighted-code-test.js b/test/javascripts/components/highlighted-code-test.js index e9e8d364ccd161..1cde451bcabb42 100644 --- a/test/javascripts/components/highlighted-code-test.js +++ b/test/javascripts/components/highlighted-code-test.js @@ -1,5 +1,8 @@ import componentTest from "helpers/component-test"; -import { waitForHighlighting } from "discourse/services/syntax-highlighter"; +import { + waitForHighlighting, + setupHighlightJs +} from "discourse/lib/highlight-syntax"; const LONG_CODE_BLOCK = "puts a\n".repeat(15000); @@ -9,14 +12,14 @@ componentTest("highlighting code", { template: "{{highlighted-code lang='ruby' code=code}}", beforeEach() { - Discourse.HighlightJSPath = - "/assets/highlightjs/highlight-test-bundle.min.js"; - Discourse.HighlightJSWorkerURL = "/assets/highlightjs-worker.js"; + setupHighlightJs({ + highlightJsUrl: "/assets/highlightjs/highlight-test-bundle.min.js", + highlightJsWorkerUrl: "/assets/highlightjs-worker.js" + }); }, async test(assert) { this.set("code", "def test; end"); - await waitForHighlighting(); assert.equal( find("code.ruby.hljs .hljs-function .hljs-keyword") @@ -31,9 +34,10 @@ componentTest("highlighting code limit", { template: "{{highlighted-code lang='ruby' code=code}}", beforeEach() { - Discourse.HighlightJSPath = - "/assets/highlightjs/highlight-test-bundle.min.js"; - Discourse.HighlightJSWorkerURL = "/assets/highlightjs-worker.js"; + setupHighlightJs({ + highlightJsUrl: "/assets/highlightjs/highlight-test-bundle.min.js", + highlightJsWorkerUrl: "/assets/highlightjs-worker.js" + }); }, async test(assert) { From 634f1c99651c9560b67fbde1dca91584cb58dad5 Mon Sep 17 00:00:00 2001 From: David Taylor Date: Tue, 14 Jul 2020 15:11:16 +0100 Subject: [PATCH 5/5] Restore for theme --- app/assets/javascripts/admin/models/theme.js | 2 ++ 1 file changed, 2 insertions(+) diff --git a/app/assets/javascripts/admin/models/theme.js b/app/assets/javascripts/admin/models/theme.js index 464dedb54d105d..d17b87dfd0dc07 100644 --- a/app/assets/javascripts/admin/models/theme.js +++ b/app/assets/javascripts/admin/models/theme.js @@ -8,6 +8,7 @@ import { popupAjaxError } from "discourse/lib/ajax-error"; import { ajax } from "discourse/lib/ajax"; import { escapeExpression } from "discourse/lib/utilities"; import { url } from "discourse/lib/computed"; +import highlightSyntax from "discourse/lib/highlight-syntax"; const THEME_UPLOAD_VAR = 2; const FIELDS_IDS = [0, 1, 5]; @@ -320,6 +321,7 @@ const Theme = RestModel.extend({ } } ); + highlightSyntax(document.querySelector(".bootbox.modal")); } else { return this.save({ remote_update: true }).then(() => this.set("changed", false)