ngx-highlightjs icon indicating copy to clipboard operation
ngx-highlightjs copied to clipboard

Highlight.js library was not imported and cannot read property 'themePath' of undefined

Open GregoireLgt opened this issue 2 years ago • 16 comments

  • [x] bug report
  • [ ] feature request
  • [ ] question

Repro steps

  1. Install ngx-hightlightjs 6.1.0

  2. Add the following to app.module.ts:

import {HighlightModule, HIGHLIGHT_OPTIONS} from 'ngx-highlightjs';

imports: [
    ...
    HighlightModule
  ],
providers: [
    ...
    {
      provide: HIGHLIGHT_OPTIONS,
      useValue: {
        coreLibraryLoader: () => import('highlight.js/lib/core'),
        languages: {
          xml: () => import('highlight.js/lib/languages/xml')
        }
      }
    }
  ],

Console logs

image

Debug mode : image

Environment

  • Angular: 13
  • ngx-highlightjs: 6.1.0
  • Browser(s): Chrome 87.0.4280.141 (Build officiel) (32 bits)
  • Operating System (e.g. Windows, macOS, Ubuntu): Windows 10 Professionnel

GregoireLgt avatar Jan 04 '22 17:01 GregoireLgt

This error is because your project is missing the original package npm i highlight.js

It is weird because it should be installed automatically!

MurhafSousli avatar Jan 04 '22 23:01 MurhafSousli

Hi @MurhafSousli

I am sorry but highlight.js is already installed. I figured out the "Highlight.js library was not imported" error because this was missing in my angular.json :

"scripts": [ "src/assets/lib/hljs/highlight.pack.js" ],

But i am still getting cannot read property themePath of null, there are no options inside constructor : image

GregoireLgt avatar Jan 05 '22 08:01 GregoireLgt

I don't know why you are importing highlight.pack.js in angular.json, this is not mention anywhere in the docs!

can you make a reproduction?

MurhafSousli avatar Jan 05 '22 18:01 MurhafSousli

The HLJS token you are talking about is the HIGHLIGHT_OPTIONS token ? If yes, i use it as a provider in the app.module.ts

EDIT @MurhafSousli

I am not having the cannot read property themePath of null with the 6.0.0 (everything works fine). But when i switch to the 6.1.0 then the error appears

GregoireLgt avatar Jan 06 '22 08:01 GregoireLgt

Hi @MurhafSousli, sorry to insist but the 6.0.0 of your package works like a charm whereas the 6.1.0 and above are not working anymore with the 'cannot read property themePath of null' error :(

Regards,

GregoireLgt avatar Jan 25 '22 14:01 GregoireLgt

I had this issue too. I think this is related to lazy-loaded modules importing this one. In fact, application lazy loaded modules provide the options in their own scope, and because HighlightLoader & HighlightJS services are provided in root, they don't have access to the options.

https://github.com/MurhafSousli/ngx-highlightjs/blob/c1229aa2f914f8be74ed6d0f5066b1c2856c2050/projects/ngx-highlightjs/src/lib/highlight.loader.ts#L8-L11 https://github.com/MurhafSousli/ngx-highlightjs/blob/c1229aa2f914f8be74ed6d0f5066b1c2856c2050/projects/ngx-highlightjs/src/lib/highlight.service.ts#L14-L17

Cloning this repo and changing providedIn scopes to any fixed it for me.

Maybe a static forRoot pattern might offer more options to avoid multiple loadings of highlight.js.

GerkinDev avatar Apr 05 '22 13:04 GerkinDev

@GerkinDev Sure thing! the injection token should be used in the root module only! but the module can be imported at child module level.

That's why I didn't introduce HighlightModule.withConfig() so users don't make the mistake of setting the config in child module

Adding forRoot will make the users use it in the root module, and not think about lazy loading it in a child module. an injection token is the best in my opinion.

MurhafSousli avatar Apr 05 '22 18:04 MurhafSousli

Yeah I get your point, but in my case, I use this module in my dev tools, and I'd like to not ship it at all in the main bundle. Thus, including it in the root module is not an option for me.

GerkinDev avatar Apr 05 '22 21:04 GerkinDev

@GerkinDev You are not shipping it using the injection token! you are only passing a function that returns a promise which will load the library. so it won't be included in your main script.

Can you show that it is increasing it?

MurhafSousli avatar Apr 06 '22 18:04 MurhafSousli

It must. The injection token declaration, along with the configured value & imports declaration processed by webpack ARE in the root module output.

This is not even close to the size of the full lib, I know. But still, there is a hard reference to multiple files, including highlightjs & your module, anyway. Tree shaking should make it as small as possible, just a few bytes, but a few bytes used by 0.01% of users isn't really worth it.

Moreover, if your module is shipped conditionally (in dev environment only for example), constraining its declaration in the root module can be troublesome, though I suppose there are handy ways to come around this.

Still. I'll go with my fork changing the providers scope.

GerkinDev avatar Apr 09 '22 17:04 GerkinDev

@GerkinDev When webpack process the import script, it will only include that script in the dist directory, but won't merge it in the main script, meaning that the script won't slow the app launch. it will be loaded separately on parallel with the loader service.

If you are referring to the few bytes reserved for the config value the user pass, like the script string path, that is does not worth lazy loading in my opinion.

But anyway, are you proposing to remove the providedIn: root from the services?

MurhafSousli avatar Apr 09 '22 19:04 MurhafSousli

Are there any real downside you expect, that I should be aware of to consider declaring providers is a necessity ?

GerkinDev avatar Apr 10 '22 18:04 GerkinDev

Not really! the only thing that matters is that it should not load the script twice if the highlight module is being used in different lazy modules. that's why I choice a singleton service.

MurhafSousli avatar Apr 10 '22 19:04 MurhafSousli

Well, Highlight.js is not exported via UMD globals, yet, I see in your loader service that you expose it as global if using line numbers plugin. Maybe this could be used also to check if the lib was already loaded (& eventually supports loading via a CDN). Globals are evil. But here, we're talking about replacing a global service with a global variable.

Or maybe another service could be globally used without any dependency, that serves just as a memo for configured loaders. Dunno.

And anyway, in most case, if the same JS asset is served twice, a cached version would be used, reducing the loading time to 0-ish

GerkinDev avatar Apr 11 '22 08:04 GerkinDev

I don't see dependencies in the current loader service!

The line number script requires hljs to be defined in the global variables. otherwise it will not work (you can have a look at their repo).

While I am not against the idea of removing the loader service from root, loading the script twice sounds fundamentally wrong to me (even if it is cached). but anyway, the function hljs.registerLanguages() will be called twice too. and we have the load theme function which appends a style element to the head.

In case of we want to change it, we should have a clear alternative design that execute the code only once

MurhafSousli avatar Apr 11 '22 10:04 MurhafSousli

I was talking about those, that force the loader to have HIGHLIGHT_OPTIONS provider in scope:

https://github.com/MurhafSousli/ngx-highlightjs/blob/c1229aa2f914f8be74ed6d0f5066b1c2856c2050/projects/ngx-highlightjs/src/lib/highlight.loader.ts#L22-L24

GerkinDev avatar Apr 11 '22 15:04 GerkinDev

For me it shows the ngx-highlightjs code taking 17.83 kB of my main bundle

image

Just because I had to provide the HIGHLIGHT_OPTIONS in the AppModule

ng new highlighttest --style=scss --routing
cd highlighttest
npm install --save-dev webpack-bundle-analyzer
code .
npm run build -- --stats-json
npx webpack-bundle-analyzer dist/highlighttest/stats.json

image

-> Parsed size = 194.88 kB

Install ngx-highlightjs:

npm i ngx-highlightjs

Now provide the HighlightOptions in the AppModule (because we must)

providers: [{
  provide: HIGHLIGHT_OPTIONS,
  useValue: <HighlightOptions>{
    fullLibraryLoader: () => import('highlight.js'),
    themePath: 'assets/styles/solarized-dark.css'
  }
}],

Now build again

npm run build -- --stats-json
npx webpack-bundle-analyzer dist/highlighttest/stats.json

image

-> Parsed size = 195.03 kB

So this gives me only a mere 123 bytes increase in main bundle size. However I don't understand where the 6.75 kB from ngx-highlightjs comes from in my own application..

PieterjanDeClippel avatar Sep 28 '22 14:09 PieterjanDeClippel