Discuss: Website/browser build is not intended for use with Node.js.

[avih]

var <any-name-hljs-included> = require('./highlight.pack'); fails with ReferenceError: hljs is not defined. when executed in node.js.

This happens when downloading and extracting highlight.zip from https://highlightjs.org/download/ (tested both the default selected languages or with just one - javascript)

But it doesn't fail (and I can later use it nicely) if I build it on my own with node tools/build.js javascript or any other languages which I tried.

This happens with 9.50 but also happened with 9.40 (worked when I build it, didn't from the download).

Looking at highlight.pack.js - both the downloaded and the one which I built, it seems that the code is supposed to work also in a module environment (tests for exports availability at the beginning - identical first few LOC at both files), but it doesn't with the download.

Is it a bug? expected? IMO it should be possible to require it also with the downloaded package.

[isagalaev]

We have different build targets for node.js and for the browser/AMD environment. The highlight.pack.js you're getting from the site is the browser variant. Have a look here: http://highlightjs.readthedocs.io/en/latest/building-testing.html

[avih]

I know. I'm saying that if you build for the browser, like so: node tools/build.js -t browser :common then the resulting highlight.pack.js can still be used with node, but if you download it from the site then it cannot be used with node.

[isagalaev]

Aha… Then it might be due to the bug #1220 that I fixed just yesterday. Could you please check again?

[avih]

I just checked. It still cannot be used from node when downloading from the site, but can be used from node when building locally for the browser (with 9.6.0 - same as it was on 9.5.0 and 9.4.0).

At the site download, I selected a single language: javascript and clicked download.

Inside the downloaded zip I opened highlight.pack.js in an editor, and it starts with /*! highlight.js v9.6.0 | BSD3 License | git.io/hljslicense */ so this is the latest version.

I created a test file test.js which contains a single line: var x = require('./highlight.pack'); and ran node test.js .

It fails with the downloaded highlight.pack.js, but works with highlight.pack.js which I built myself for the browser.

I then unminified (or at least beautified) highlight.pack.js which I downloaded - using http://jsbeautifier.org/

This code tries to use hljs at the global scope (towards the end of the file):

});  // <-- this is the end of the big function
hljs.registerLanguage("javascript", function(e) {
    return {
    // ... the rest of the javascript support is here

However, there's no global object hljs (because the AMD/CommonJS detection sees exports when it runs in node, so it doesn't create it), so it fails.

When I build it myself for the browser, it doesn't try to use a global hljs object. Instead, the registration looks like this, and it's inside the big function where e exists:

    }, e.registerLanguage("javascript", function(e) {
        return {
        // ... the rest of the javascript support is here

and it works in node.

[avih]

Where is the website code which composes the package (according to the languages the user chooses via checkboxes) which is then downloaded?

[entibo]

Quick fix for anyone else having this problem:

(highlight.pack.js)

!function(e){var n="object"==typeof window&&window||"object"==typeof self&&self;"undefined"!=typeof exports?
->
!function(e){var n="object"==typeof window&&window||"object"==typeof self&&self;"undefined"!=typeof undefined?

[isagalaev]

A quick update. This is due to a discrepancy between the browser build and the CDN build. The site uses the latter to construct zips on the fly using simple concatenation of pre-compressed highlight.js and language files. At some point the real browser build started putting languages inside the contents of highlight.js. I'm not yet sure how best to fix it.

Out of curiosity, why would you want to use a browser build from node?

[avih]

Because it's simpler/quicker/easier to check boxes and download a custom pack instead of building it.

I have a small node script which highlights source code for the console and uses hljs as backend (still not public but It'll be at some stage) and it doesn't have other dependencies, so you just drop the pack at the same dir as the script and it works - except it doesn't now due to this issue (but it does work if I build the pack myself).

I wanted to let users pick the languages they need and download the latest package themselves instead of asking them to build the node package or providing packages myself.

[isagalaev]

Seems reasonable enough, even if unusual. And I want this fixed anyway. In the meantime however, you could probably just use the actual node build? If you need to constrain languages used in auto-detection it's as simple as hljs.configure({languages: [ ...]}) or pass the same language list as a second argument to highlightAuto() http://highlightjs.readthedocs.io/en/latest/api.html#highlightauto-value-languagesubset

[avih]

Yeah, I could always just create a pack with all the languages and post it someplace and update it when hljs updates - it's not a big file anyway, so this issue is not blocking me in any way.

Re auto detection, I actually found it a bit slow (relatively speaking), and found out that if I maintain a map of file extensions to languages as config then it works quite a bit faster than auto-detection (but I still have auto-detection as fallback).

[isagalaev]

I could always just create a pack with all the languages and post it someplace and update it when hljs updates

Why not the actual node package from npm https://www.npmjs.com/package/highlight.js? It has zero dependencies, just as the browser one.

[kurifodo]

+1, I ran into the same issue described above and would love the fix to be in place for downloading the custom pack of languages, based on check box selections, instead of having to build it manually.

Thanks for creating this great tool, @isagalaev.

[TinoN]

@entibo's suggestion worked for me (in Laravel 5.5)
in app.js

...
require('./highlight.pack.js').initHighlightingOnLoad();
...

[joshgoebel]

Is this resolved now? If not could someone please summarize the issue? Is this just a duplicate of "please include a binary distributable with npm packages"?

[joshgoebel]

Closing due to inactivity and no one responding here. If this is still an issue please let us know and we can always reopen this.

Although I'm pretty sure the right answer is to simply NOT use the browser build for nodejs includes... That seems wrong to me vs just requiring the library via npm which should "just work". If this was a big demand we could perhaps have another build target, but judging from the lack of response here it seems no such real demand.

Also moving forward we might do more "browser" specific things (better dynamic loading of languages) in the browser version that are purposely very different from the Node.js version... ie, there is a reason we have two very different build targets... They aren't expected to be interchangeable.

[avih]

Sorry, missed the question in October.

The issue is still unresolved, I just tried with the default download from https://highlightjs.org/download/ and it still fails in the same way.

The summary and steps to reproduce are described exactly at the first post.

Please ask if you don't understand something, but I think the issue is pretty clear and easy to reproduce. The only question is if the project wants to fix it. Judging by the several thumbs-up on this issue, I'd say others would like it fixed too.

[avih]

Just to be clear, the steps to reproduce are as follows:

1. Download a pack from https://highlightjs.org/download/ (the default or any other combination of supported languages).
2. extract the zip.
3. Create a file test.js with a single line in it: var x = require('./highlight.pack');
4. Run node test.js

Expected result: nothing interesting happens.

Actual result: the following error is shown:

$ node test.js
highlight.pack.js:2
!function(e){var n="object"==typeof window&&window||"object"==typeof self&&self;"undefined"==typeof exports||exports.nod
eType?n&&(n.hljs=e({}),"function"==typeof define&&define.amd&&define([],function(){return n.hljs})):e(exports)}(function
(a){var f=[],i=Object.keys,b={},u={},n=/^(no-?highlight|plain|text)$/i,l=/\blang(?:uage)?-([\w-]+)\b/i,t=/((^(<[^>]+>|\t
|)+|(?:\n)))/gm,r={case_insensitive:"cI",lexemes:"l",contains:"c",keywords:"k",subLanguage:"sL",className:"cN",begin:"b"
,beginKeywords:"bK",end:"e",endsWithParent:"eW",illegal:"i",excludeBegin:"eB",excludeEnd:"eE",returnBegin:"rB",returnEnd
:"rE",variants:"v",IDENT_RE:"IR",UNDERSCORE_IDENT_RE:"UIR",NUMBER_RE:"NR",C_NUMBER_RE:"CNR",BINARY_NUMBER_RE:"BNR",RE_ST
ARTERS_RE:"RSR",BACKSLASH_ESCAPE:"BE",APOS_STRING_MODE:"ASM",QUOTE_STRING_MODE:"QSM",PHRASAL_WORDS_MODE:"PWM",C_LINE_COM
MENT_MODE:"CLCM",C_BLOCK_COMMENT_MODE:"CBCM",HASH_COMMENT_MODE:"HCM",NUMBER_MODE:"NM",C_NUMBER_MODE:"CNM",BINARY_NUMBER_
MODE:"BNM",CSS_NUMBER_MOD

ReferenceError: hljs is not defined
    at Object.<anonymous> (highlight.pack.js:2:10771)
    at Module._compile (module.js:624:30)
    at Object.Module._extensions..js (module.js:635:10)
    at Module.load (module.js:545:32)
    at tryModuleLoad (module.js:508:12)
    at Function.Module._load (module.js:500:3)
    at Module.require (module.js:568:17)
    at require (internal/module.js:11:18)
    at Object.<anonymous> (test.js:1:71)
    at Module._compile (module.js:624:30)

[avih]

I'm pretty sure it should be fairly trivial to fix, but the issue doesn't happen locally when I build for the browser, so I don't know how to reproduce and fix it locally.

It only happens when when downloading from your site, and I don't know where is the code which is responsible for that composition of the content.

[avih]

Also note that your README.md does suggest CommonJS should work with the browser build:

You can get highlight.js as a hosted, or custom-build, browser script or as a server module. Right out of the box the browser script supports both AMD and CommonJS.

And it does work when building for the browser locally, but doesn't work when you download a browser build from your site.

[joshgoebel]

And it does work when building for the browser locally, but doesn't work when you download a browser build from your site.

How are you building the browser build locally exactly? Are you using :common?

[joshgoebel]

Also note that your README.md does suggest CommonJS should work with the browser build:

I think it's referring to the core library, not the BUNLDED library - which is very different. But it's obvious this could be cleared up or explained better.

I'll try this and see what I come up with.

[avih]

How are you building the browser build locally exactly? Are you using :common?

#1245 (comment)

[avih]

I think it's referring to the core library, not the BUNLDED library - which is very different. But it's obvious this could be cleared up or explained better.

But why not just fix it? the code clearly tries to detect the environment also when you download it from the site. It just fails to detect it correctly.

I've asked few times before, where is the code which is responsible for creating the download and/or concatenating the fragments into a single download?

I'm willing to have a go at it, but I don't know how to reproduce it except by downloading it from the site, but I need the actual source code of it to submit a patch.

[joshgoebel]

But why not just fix it? the code clearly tries to detect the environment also when you download it from the site. It just fails to detect it correctly.

I'm not saying we won't, first we have to clearly understand the issue - your exact steps were helpful. But my point before does stand that the browser build is NOT necessarily intended to be the same as a NPM build. These things may diverge in the future and if it's never worked quite right this might be a blessing in disguise.

I've asked few times before, where is the code which is responsible for creating the download and/or concatenating the fragments into a single download?

You'd think it was the build process, but it seems the results are different. Only @isagalaev knows I think. The code generated on the website is putting hljs.registerLanguage OUTSIDE the original enclosure and the code generated by the build process places it inside. @isagalaev Is the website using a special build system instead of the one in the repo for some reason? hljs as a global only works in the web environment (where anything on window is treated as a global), so that is why it's failing in node.js.

I'm willing to have a go at it, but I don't know how to reproduce it except by downloading it from the site, but I need the actual source code of it to submit a patch.

Yeah, I'm not sure it's fixable by any changes to the repo.

[joshgoebel]

@egor-rogov Any thoughts? If the browser build is NOT truly the browser build but more of a general purpose build then it should be renamed accordingly. Right now my understanding is that the browser build ONLY needs to work well in the browser environment (and I think this is a good limitation for that build). That's also the same assumption that's embedded in our tests currently.

Node build are tests with node, browser builds are tested in a fake browser environment.

---

This has implications on future development and the new build system.

[joshgoebel]

Obviously the discrepancy here needs to be resolved I think (which would likely fix this in the short-term), but I'm asking more about this idea that the browser build should "just work" as a node require. I think that idea may be misguided.

[avih]

Thanks for the reply. Would you mind reopening this issue? It's still valid IMO, and apparently others are interested in it as well.

I think it should either get fixed, or stated clearly that the browser build cannot be used with node, or stay open until one of those happen.

[joshgoebel]

Opening a new issue to track the actual discrepancy between local and website builds (which should NOT be the case IMHO).

I think it should either get fixed, or stated clearly that the browser build cannot be used with node, or stay open until one of those happen.

Agree with your either/or here.

[joshgoebel]

One example: Future browser builds might come in ES5, ES6, ES* flavors. Once we had a build process that could do such things one could imagine a new feature supported by browsers before it was supported by Node (or where we purposely wanted a longer compatibility curve with Node)... and therefore the browser build might include newer technology.

The browser build also currently includes a lot of code NOT necessary for Node.js usage at all... in a perfect world that wouldn't need to be parsed or compiled by JS in a node specific build.

Obviously maintaining two forks would likely not be worth the effort, but if the build process did all this for us automatically it seem far more plausible.

[avih]

FWIW, I think it would be easiest to manage if the code is as unified as possible. Handling different code for different scenarios will only create more overhead IMO.

But let's first wait for a way to reproduce/build the bad bundle locally, and then we can assess what can be done.

[joshgoebel]

FWIW, I think it would be easiest to manage if the code is as unified as possible. Handling different code for different scenarios will only create more overhead IMO.

Agree regarding the codebase in general. But the raw codebase itself and the built packages are a very, very different things - on only going to become more so over time. We already produce 3 different builds... browser, CDN, node.js... it's easy, and automated. I wouldn't want to maintain 3 sets of code by hand, but there are definitely good reasons to build and package the code differently for different environments. Though CDN is really browser + CDN assets...

Node.js build is very different than browser builds though.

But let's first wait for a way to reproduce/build the bad bundle locally

I assigned isagalaev to the other issue, so we'll have to see. I bet the server is simply using a slightly different/older build process and it really should be brought into alignment with the local build tools.

[avih]

I bet the server is simply using a slightly different/older build process and it really should be brought into alignment with the local build tools

Hopefully, because that sounds like it would be very easy to fix.

Thanks again for your comments and the other issue.

[joshgoebel]

Hopefully, because that sounds like it would be very easy to fix.

Well, we will see. :) I think the server is running Python, not JS... so that might be a small issue...

Pretty sure my new build process will (as it stands currently) simply break this again though as my assumption is the browser target environment is the BROWSER, hence the importance of resolving the deeper issue here also. :-)

[joshgoebel]

@egor-rogov Do you have a problem saying that what the website build is for WEB usage, and not Node? If someone wants to use Highlight.js with Node they should be using the npm package.

That's my gut feeling at least.

The exact issue that resulted in the creation of this issue isn't theoretically hard to fix (thought it'll require server changes that only isagalaev can do) but I think long-term I could see the web build as being a distinct thing from the node build.

They are honestly different environments and with smarter build tools I could imagine auto-magically delivering different optimized versions for each. So if this has NEVER worked (the website version has never worked in Node)... and only a few people are asking for it, I think it's worth considering whether it's a good idea for "fix" this vs the official line being "just use NPM"... and whether this will prevent us from doing nice things in the future.

[egor-rogov]

I agree. We have different build targets for different purposes, and requiring a compatibility can prove restrictive.

[joshgoebel]

I think it should either get fixed, or stated clearly that the browser build cannot be used with node , or stay open until one of those happen. [emphasis mine]

See comments above... So we're going to close this as "won't fix, working correctly"... but where exactly would you have expected to see this stated clearly? When you built the library on the website?

I have no issues with making this clearer and explaining the logic behind it.

[avih]

requiring a compatibility can prove restrictive.

If it's relatively easy to solve so that the web build can work in node, then you can always say that it works for now but not guaranteed to be maintained in the future. Afteral, there is some value in being able to use a single file instead of depending on npm or build deps.

[joshgoebel]

If it's relatively easy to solve so that the web build can work in node, then you can always say that it works for now but not guaranteed to be maintained in the future.

But we already know the long-term plan is likely to diverge - and then that would be a breaking change - which we do try to avoid except for major releases. So we'll just say it never worked and it likely never will. :-)

If you really want it to work today you could manually (or automatically) hack the file to fix the actual issue. Though no promises it'll be easy to continue to do so in the future.

Afteral, there is some value in being able to use a single file instead of depending on npm or build deps.

If it's a node.js project why wouldn't you simply put it in packages.json and track it as a proper dependency? You're free to hack together whatever you want, but right now I don't think we actually would want to encourage doing things the way you are suggesting. Perhaps I just don't understand your use case.