Highlight.js: Proposal: JIT loading of Javascript grammars via CDN
The idea is to be able to have the largest assortment of languages on tap without greatly increasing the size of the core download - and without manually having to adding individual script tags to load via CDN. This is great for use cases like one-off languages or blogs where every once in a while you pull in another language but most of the time you only use the core set.
I suggest we accomplish this with a configuration key for AllowedCDNAddress or something where you set the CDN prefix such as:
configure({
allowedCDNAddress: "https://cdnjs.cloudflare.com/ajax/libs/highlight.js/9.18.1/languages/"
})
Then whenever anyone specified an explicit language (ie, lang-fortran77) we'd probe the CDN and if the .../languages/fortran77.min.js file is available we'd fetch/load it - and after it loaded successfully we'd then queue the highlight to happen. If the JS file can't be fetched, the highlight would fail and only the hljs style would be applied.
It might also be useful to allow the user to specify the list of approved languages to load:
configure({
allowedCDNAddress: "https://cdnjs.cloudflare.com/ajax/libs/highlight.js/9.18.1/languages/",
allowedCDNLanguages: ["fortran77", "fortran90"]
})
This would have the side benefit of increasing security (although you shouldn't be using the CDN if you don't trust it anyways) and also then allowing the simpler fortran77 class naming vs lang-fortran77 - since we have a list on hand to check each class name against and see if it potentially MIGHT be a valid language.
I think this would be a pretty simple add actually and a fun little project if anyone wanted to tackle it. Rough idea:
- When
highlightBlockis called, see if the language is available (or potentially available) and not loaded - Load/fetch it... (and here we're assuming it's one of our standard auto-registering CDN modules)
- Add the block to a queue for the given language of "pending" highlights.
- When
registerLanguageis called, check the queue for the just registered language to see if any blocks are waiting.
.5 If there are any items in the queue, nowhighlightBlockcan be called again and this time it will immediately work since the language is now loaded.
joshgoebel
All 26 comments
I think the current hook system is able to do this task without having to touch the core library. I’m thinking moving this proposal as a plugin idea:
~~~ .js
hljs.addPlugin({
'before:highlightBlock': ({block, language}) => {
if (!hljs.getLanguage(language)) {
let scriptElement = createScriptFunc(cdnPath + '/' + language + '.js');
document.head.appendChild(scriptElement);
scriptElement.addEventListener('load', () => {
hljs.initHighlightingButSkipAlreadyHighlighted();
});
}
}
});
hljs.initHighlightingButSkipAlreadyHighlighted = () => { ... };
~~~
taufik-nurrohman
on 31 May 2020
Inside the cdnPath + '/' + language + '.js' file, we have this:
~ .js
hljs.registerLanguage('foo-bar', self => { ... });
~
taufik-nurrohman
on 31 May 2020
I like the thinking here, and we should continue this discussion.
hljs.initHighlightingButSkipAlreadyHighlighted();
Adding new API means this can't be done "with purely the hook/plugin system". Is there a reason the plugin can't just call highlightBlock again recursively? Once the language is loaded the behavior would be as expected.
The problem is right now I'm not sure we have a "do nothing" return value so that a plugin could essentially prevent highlighting and schedule it for later (or do it recursively).
There is also the fact that blockLanguage is run BEFORE the before block highlight call... so we'd have to review that code to make sure it wouldn't break in light of an "unknown" language.
If you wanted to whip up a WORKING version of this and see what the real and imaginary issues are it'd be worth having a look at I think.
joshgoebel
on 1 Jun 2020
Inside the cdnPath + '/' + language + '.js' file, we have this:
Yes, the existing CDN files should work "as is" for this type of system AFAIK.
joshgoebel
on 1 Jun 2020
Is there a reason the plugin can’t just call
highlightBlockagain recursively.
Yes, but looks like current implementation doesn’t care whether a code block is already highlighted or not.
The problem is right now I’m not sure we have a “do nothing” return value so that a plugin could essentially prevent highlighting and schedule it for later (or do it recursively).
So, need an extra check whether a block already has a hljs class or not here:
.js
hljs.initHighlightingButSkipAlreadyHighlighted = () => {
document.querySelectorAll('pre code:not(.hljs)').forEach(hljs.highlightBlock);
};
taufik-nurrohman
on 2 Jun 2020
If you wanted to whip up a WORKING version of this and see what the real and imaginary issues are it’d be worth having a look at I think.
Let’s see :wink:
taufik-nurrohman
on 2 Jun 2020
Yes, but looks like current implementation doesn鈥檛 care whether a code block is already highlighted or not.
So? That shouldn't matter... the default implementation only calls highlightBlock once for all blocks... if the plugin behavior is correct it should "just work". It should just hook the load event on the JS file... and then "do nothing" the first time... then the load event fires, blockHighlight is called again and runs to competition.
Although we need to check for infinite loops if the load fails for some reason, etc. So the plugin might want to keep track of which languages are "loading" so it can avoid trying to load more than once. Unless we just detect the fail and then do nothing.
So, need an extra check whether a block already has a hljs class or not here:
This shouldn't be necessary. But I wonder if it's not an unreasonable idea. :-)
joshgoebel
on 2 Jun 2020
So it can avoid trying to load more than once.
This should detect if language already registered. Once registered, no need to load the language file:
.js
if (!hljs.getLanguage(language)) { ... }
But I wonder if it’s not an unreasonable idea.
Because scriptElement.addEventListener('load', () => { ... }) need a time, so we need a way to refresh the highlight task on every script-load event. Unless we have something like async, await system so that initHighlighting() can be executed after ALL language files has been loaded.
taufik-nurrohman
on 2 Jun 2020
Still feel like you are WAY over thinking this problem:
# pseudocode
before("highlightBlock", [block, lang, etc] ) do
return if languageLoaded(lang) # do normal thing
loadLanguageFromURL(urlFor(lang)).then do
hljs.highlightBlock(block) # this will now work since language is loaded
end
end
Nothing regarding onload needs to change. The plugin just needs to handle calling highlight block itself once it's knows the language is loaded.
joshgoebel
on 2 Jun 2020
Ah, now I see that before:highlightBlock hook runs on every block, so no need to iterate over again :laughing:
.js
hljs.addPlugin({
'before:highlightBlock': ({block, language}) => {
if (!hljs.getLanguage(language)) {
let scriptElement = createScriptFunc(cdnPath + '/' + language + '.js');
document.head.appendChild(scriptElement);
scriptElement.addEventListener('load', () => {
if (notYetHighlightedByWrongAutoDetectionViaInitHighlighting()) {
hljs.highlightBlock(block);
}
});
} else {
// Let `hljs.initHighlighting()` or `hljs.initHighlightingOnLoad()` do the job!
}
}
});
I’ll try to make this plugin, maybe tomorrow.
PS: Making a line-numbers plugin shouldn’t be difficult.
taufik-nurrohman
on 3 Jun 2020
Done! :grin:
https://github.com/taufik-nurrohman/highlight.jit.js
FYI, language property returns undefined if no language is registered. So, in my plugin, I need to loop through the class name to temporarily treat them as a language name.
taufik-nurrohman
on 4 Jun 2020
Pretty nice.
hljs.currentLanguagePath = currentLanguagePath;
hljs.currentStyle = currentStyle;
hljs.currentStylePath = currentStylePath;
This makes me wonder if we should open up options somehow (to read), but you got by just fine without it...
joshgoebel
on 4 Jun 2020
So, in my plugin, I need to loop through the class name to temporarily treat them as a language name.
Kind of the same way we go about it... Really you should REQUIRE lang- or language- prefix so that you aren't just loading random JS files based on EVERY class. I could imagine some HTML with a crazy number of classes for some reason. Making one web request for each class is a little crazy.
joshgoebel
on 4 Jun 2020
Just curious what is the point of the CSS loader?
joshgoebel
on 4 Jun 2020
Now we need to figure out how to link to 3rd party plugins like we link to 3rd party languages.
joshgoebel
on 4 Jun 2020
This makes me wonder if we should open up options somehow (to read).
Related: https://github.com/highlightjs/highlight.js/issues/1969
Really you should REQUIRE
lang-orlanguage-prefix so that you aren’t just loading random JS files based on EVERY class.
Already added to my TODO list.
Just curious what is the point of the CSS loader?
Ahahahha, you don’t like it? I should remove it then :laughing:
Now we need to figure out how to link to 3rd party plugins like we link to 3rd party languages.
Isn’t stacking the plugin files next to the core _HighlightJS_ file enough for us?
taufik-nurrohman
on 4 Jun 2020
@yyyc514 Unfortunately, this line causes the before:highlightBlock to be discarded because the language variable returns no-highlight:
.html
Could not find the language 'fortran', did you forget to load/include a language module? highlight.min.js:6:12959
Falling back to no-highlight mode for this block.
<code class="html css javascript language-fortran php">
Here, blockLanguage() returns no-highlight:
And so this:
Which caused this condition to be true:
Without language- prefix, result will be undefined, which is okay. Since no language- prefix was set, the shouldNotHighlight(language) part will never be called:
Bug: https://taufik-nurrohman.js.org/highlight.jit.js/test/priority.html
taufik-nurrohman
on 5 Jun 2020
@yyyc514 Is calling highlightBlock() within before:highlightBlock or after:highlightBlock will make that hook fired recursively?
I don’t see that happen but based on the current highlightBlock() function, it should be.
taufik-nurrohman
on 5 Jun 2020
Ahahahha, you don鈥檛 like it? I should remove it then
Dude, your plugin. :-) I was just asking what the purpose was?
Isn鈥檛 stacking the plugin files next to the core HighlightJS file enough for us?
Stacking?
joshgoebel
on 5 Jun 2020
Stacking?
~~~ .html
~~~
taufik-nurrohman
on 5 Jun 2020
I meant we need to read me index for plug-ins so that users of the library know where they can find third-party plug-ins.
joshgoebel
on 5 Jun 2020
@yyyc514 Is calling highlightBlock() within before:highlightBlock or after:highlightBlock will make that hook fired recursively?
I think I missed this before? Yes, you'd have to guard against that. You may not be seeing an issue because the second time you are called the JS is already loaded? But if someone called you too quickly then you might get "stuck".
I'm really not sure highlightBlock is intended to be hooked this way as it has no good way to say "pause and resume later"... So right now using it this way is going to be pretty hacky.
joshgoebel
on 5 Sep 2020
And I'd think the first highlight call would blow up with an error because the language wasn't found?
joshgoebel
on 5 Sep 2020
Makes me wonder if we should perhaps expose "shouldNotHighlight" as a plugin callback... so a plugin would have a way of "rejecting" language and this could be used to build auto-load functionality... reject the first time (if language is missing), download it, when download complete, call highlightBlock again.
joshgoebel
on 5 Sep 2020
And I'd think the first highlight call would blow up with an error because the language wasn't found?
taufik-nurrohman
on 5 Sep 2020
I'm going to close this issue. This was kind of my pet desire because I could use it in a project I was working on... but upon further reflection I'm not sure this belongs in core. It's easy enough to do with a small wrapper (as you've shown).
joshgoebel
on 6 Oct 2020
Related issues
Rarst
路
8Comments
starikovs
路
5Comments
Suyash2810
路
8Comments
Vad1mo
路
7Comments
sustained
路
4Comments