highlightjs / vue-plugin

Highlight.js Vue Plugin
BSD 3-Clause "New" or "Revised" License
207 stars 27 forks source link

Allow injection of Highlight.js instance (module singleton not guaranteed across submodules) #1

Closed joshgoebel closed 3 years ago

joshgoebel commented 3 years ago

Any import of Highlight.js refers to the same singleton instance of the library, so configuring the library anywhere configures it everywhere.

The module singleton works most of the time but it's not guaranteed across submodules in node.js.

A module "singleton" is based on the resolved file path of the imported module url (that's not a "great article" but it does define the import process).

Normally, when version requirements match, all packages will collapse down to a single highlight.js in the parent package. It is perfectly acceptable for the main project code and every sub module to rely on a separate version of highlight.js though as they can each resolve to their own node_modules copy of the module and each have their own "singleton" (per resolved file path).

I haven't dug into the new vue plugin completely so if there is some window.hljs magic happening that was briefly mentioned above ignore this...

It looks like with the current setup @highlightjs/vue-plugin has a dependency of "highlight.js": "^10.7.1". If the parent package that imports @highlightjs/vue-plugin had a lock file at 10.7.0 the project would end up structured like this:

my-super-project/
  node_modules/
    highlight.js/ (10.7.0 "singleton")
    @hightlightjs/
      vue-plugin/
        node_modules/
          highlight.js/ (10.7.1 "singleton")

Here the docco example hljs.registerLanguage('javascript', javascript) would not be seen by the hljs instance in the @highlightjs/vue-plugin module.

Barring the parent package injecting hljs into the plugin already suggested above, you start needing peer dependencies which have their own issues.

It's a bit of a wat moment when you do run into the "not quite a singleton" issue in the wild, so personally I only rely on module singletons within a single module/package. The super esoteric version of this I've seen is when running on case insensitive file systems and one file in your project does an import on a differently cased name of a file : /

Originally posted by @mhio in https://github.com/highlightjs/highlight.js/issues/2815#issuecomment-813751467

Putnam3145 commented 3 years ago

For the record, as of (I assume) 8 days ago, module singleton does not work with the vue2 plugin whatsoever, even if I finagle with versioning to try to get everything to use the same version.

joshgoebel commented 3 years ago

Does your project have multiple versions of the library?

HelixHexagon commented 3 years ago

I've been having a hard time registering any languages using Vue 3. Results looks the same as in https://github.com/highlightjs/vue-plugin/issues/8 so possibly the same underlying cause. ~I could only see one version of highlight.js in my node_modules though 🤔~ EDIT: think I was mistaken here.

I ended up forking this and making Vue.ts return a function to construct the plugin, which we can pass an instance of hljs in to. Looks like:

import { ref, h, computed, defineComponent, Plugin, watch } from 'vue'
import { escapeHtml } from './lib/utils'

function createPlugin (hljs: any): Plugin {
    const component = defineComponent({...})

    return {
        install(app) {
            app.component('highlightjs', component)
        },
    }
}

export default createPlugin

Usage looks almost identical to current:

import 'highlight.js/styles/default.css'
import hljs from 'highlight.js/lib/core'
import javascript from 'highlight.js/lib/languages/javascript'
import hljsVuePlugin from "@highlightjs/vue-plugin"

hljs.registerLanguage('javascript', javascript);

const app = createApp(App)
app.use(hljsVuePlugin(hljs))
app.mount('#app')

Would you consider moving to dependency injection instead of relying on singletons? I realise it would be breaking API change but this way you don't even need to register highlight.js as a dependency; people can just bring their own.

joshgoebel commented 3 years ago

We may end up going that route... but I'd be very curious for anyone to track down WHY this isn't working as expected. Is one place using a different import? Is one place using require vs import? There has got to be a reason the behavior changed.

HelixHexagon commented 3 years ago

Thanks for the speedy reply! 🙂 I'm not sure what I did wrong yesterday but from testing today things are starting to make a bit more sense. So, as you already know, the problem comes from installing @highlightjs/vue-plugin@next and highlight.js at a version other than ^10.7.1 (e.g. highlight.js@9, highlight.js@10.7.0, or highlight.js@11.)

In any of these cases, NPM will also install a separate copy of highlight.js@^10.7.1 within the vue plugin folder and hljs will not work as a singleton. This structure will be saved in the package-lock.json and I'm not sure how to fix it without deleting the lock file and node_modules then starting afresh with npm install highlight.js@10 @highlightjs/vue-plugin@next.

I'm still in favour of using injection to allow people to bring whatever version they please, but for the time being we could update the readme to specify ^10.7.1 should be used? I think I got in to this mess by allowing highlight.js to install @ latest, i.e. 11.0.1.

mhio commented 3 years ago

I'm still in favour of using injection

Me too, I'd like the option app.use(hljsVuePlugin, { hljs })

joshgoebel commented 3 years ago

So if we went that route... should we deprecate slightly using the bundled Highlight.js or change how it works? I think (because there are so few talking here) that it must work great as-is for a LOT of people... so I'd hate to make it more complex for everyone just to solve what is more of an edge case for some with more complex apps... So what about:

Thoughts?

joshgoebel commented 3 years ago

If someone wants to contribute small PRs (eventually we'd need for both v1 and v2 I think?) to allow injecting a different HLJS instance, makes sense. Tagging accordingly.

longshihui commented 3 years ago

i have same problem. The plugin is use difference highlight,js instance when difference version highlight.js be installed in one project.

Trinovantes commented 3 years ago

Opened a PR but working through this problem, I realized there are other ways to fix this problem:

joshgoebel commented 3 years ago

I think this is closed with #17 also and 2.1.0.