Doesn't work if prism is asynchronously loaded #75

Closed
WickyNilliams opened this Issue Feb 18, 2013 · 20 comments

Projects

None yet

8 participants

@WickyNilliams

If prism is loaded asynchronously then it doesn't highlight any code. It appears because it is being loaded after DOMContentLoaded event has already fired.

Damn shame this does not work out of the box as I would definitely like this kind of thing loaded asynchronously as it's (technically) non-essential and a discrete piece of functionality.

Any help much appreciated

@LeaVerou
Contributor

Oh, good point. Let me look into this. I recall there was a way of testing whether the DOM was loaded (with try...catch and querying <body>) but not sure I remember it correctly.

@WickyNilliams

Can you check readyState on the document?

@LeaVerou
Contributor

I recall there were a few incompatibilities with that, have they been solved?

@WickyNilliams

Not 100% sure, to be honest that suggestion came about after 10 seconds of googling - which usually means there are a million edge cases and things to workaround :) Though i suppose because you have limited browser support you may be able to get away with it?

If not you could always sniff around the jQuery source and pilfer something from there haha

@WickyNilliams

This may inspire: https://github.com/cms/domready/blob/master/domready.js

It has some try catch thing for IE, but I guess this is IE <8?

@LeaVerou
Contributor

Yeah, that’s old...

@WickyNilliams

I must say, it's bloody hard to find info on DOM ready stuff. It's like everyone was baffled by it years back (loads of stuff from that period), then jQuery basically solved the issue, and now there's no info describing the current state of play!

if i make the small alteration:

// in case dom has already loaded
if(document.readyState !== "loading") {
    _.highlightAll();
}
else {
    document.addEventListener('DOMContentLoaded', _.highlightAll);
}

It seems to now call highlightAll() when loaded async, but not highlighting anything. Will delve deeper now. Any progress your end?

@WickyNilliams

Right so the above doesn't work because Prism.languages hasn't been populated yet. I guess this could be populated before the main function is executed? Without reorganising large swathes of code though, you could do this (little hack) which staggers highlightAll enough to have Prism.languages populated:

if(document.readyState !== "loading") {
    setTimeout(_.highlightAll, 0);
}
else {
    document.addEventListener('DOMContentLoaded', _.highlightAll);
}

I'll leave you to decide which is the more favourable approach (reshuffle or a bit of a hack).

EDIT: tested in safari, chrome, opera, firefox. All good. Still feels a bit dirty though...

@WickyNilliams

hey @LeaVerou, did you make any progress on this?

@kyleaffolder

@WickyNilliams Where did you include this hack within the version of Prism from prismjs.com? I would love to use this with Modernizr/yepnope and may have to settle with the hack for now. Any further luck with this WickyNilliams? Any help would be much appreciated. Thanks!

@WickyNilliams

I altered this line to read as above. I've been working with it like that since I discovered the fix and I've had no problems, works perfectly

@LeaVerou would you accept my solution above as a pull request? It's inelegant but functional. I may spend a little time investigating a cleaner solution today.

@arsenetar

When loading Prism as an extension in MediaWiki using the following allowed it to highlight successfully. Granted this is in an external script but if something like this could be integrated into the core.

document.onreadystatechange = function () {
    if (document.readyState == "complete") {
        Prism.highlightAll();
    }
}

Initially the internal ._highlightAll() call happens before the languages are all loaded into the script when loading from separate files at least.

@brendanfalkowski

Guessing this wasn't resolved. Just tried loading with async param and it's not working.

@brendanfalkowski

Bonus: using the defer attribute does work. This is probably best for performance because the HTML parser isn't blocked during download or execution. async does block execution. See: http://www.growingwiththeweb.com/2014/02/async-vs-defer-attributes.html

So use something like:

<script defer src="/path/to/my-bundle-including-prism-etc.js"></script>
@apfelbox
Contributor
apfelbox commented Nov 6, 2014

Maybe we could just include a working & proven domready function (which matches our browser support table)?

@LeaVerou
Contributor
LeaVerou commented Nov 6, 2014

If it doesn't add too much code, no objections :)
We'd need to adapt it a lot though, as they all contain a lot of older browser compat stuff.

@WickyNilliams

One use case that would definitely require proper dom ready handling (rather than using defer): I want to create a bookmarklet to highlight code on the fly (nothing worse than trying to read an article without highlighting). In this scneario, Prism would be injected into the page long after dom ready has passed.

Happy to help where I can.

Also, my ugly hack above still works great - maybe that's the simplest fix for now?

@aykevl
aykevl commented Jun 23, 2015

Can I make a pull request for that? I would really like prism to support async. I don't know how to test it though - is there any documentation how to build the prism.js file?

@Golmote
Contributor
Golmote commented Jun 23, 2015

Sure. To build the prism.js file, you just need to run gulp.

@Strubbl Strubbl added a commit to Strubbl/troll-hugo-theme that referenced this issue Oct 26, 2015
@Strubbl Strubbl load prism.js with defer instead of async due to this bug PrismJS/pri… 9ef2e4a
@Golmote
Contributor
Golmote commented Jul 5, 2016

#959 fixed this issue. First JSFiddle is working as expected.

@Golmote Golmote closed this Jul 5, 2016
@Strubbl Strubbl added a commit to Strubbl/troll-hugo-theme that referenced this issue Sep 13, 2016
@Strubbl Strubbl Revert "load prism.js with defer instead of async due to this bug Pri…
…smJS/prism#75"

This reverts commit 9ef2e4a because it
is fixed now.
f3802b1
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment