File Highlight: ie crash

[dreammaster82]

IE bug with matches in File Highlight

[RunDevelopment]

Thank you for the fix @dreammaster82!

Please run npm run build and commit all changes to make the CI pass.

[RunDevelopment]

Thank you for contributing @dreammaster82!

[mAAdhaTTah]

In general, I don't love modifying the prototypes like this but if MDN is recommending it, this is fine.

Related to #1578.

[RunDevelopment]

@mAAdhaTTah Btw, we already used this polyfill here before. But I agree that Prism shouldn't modify these prototypes in all other cases.

[joshgoebel]

Is it necessary to have the polyfill in two places? If Prism.js hooks it up why does the plug-in need to do the same thing?

[RunDevelopment]

Prism doesn't really do anything for plugins other than to provide hooks. You can mix and match any combination of plugins, so copying the polyfill is the easiest solution, or is there something I'm missing?

[mAAdhaTTah]

If Prism.js hooks it up why does the plug-in need to do the same thing?

Prism core doesn't. Two separate plugins do.

It is a little annoying that we have the same code twice tho. Maybe it should be in core?

[RunDevelopment]

Maybe it should be in core?

Should core house a polyfill that is only used by 2 plugins? Maybe.
Should core house the polyfills of all plugins? Definitely no.

Core shouldn't contain any code it doesn't need to run IMO. If we wanted to extract the common logic of plugins, then that logic should be a new plugin (library). We already have examples of this with languages like Markup templating.

[joshgoebel]

Ah I just saw prism.js in the changes. But I guess for you all that’s a build asset?

[mAAdhaTTah]

@joshgoebel Yeah, we commit our build assets cuz they power the download page.

@RunDevelopment Makes sense, let's leave it then. It's not a ton of code, and we'll just kill it when we drop IE11 support.