wooorm / refractor

Lightweight, robust, elegant virtual syntax highlighting using Prism
MIT License
724 stars 34 forks source link

Backport the prismjs 1.27.0 upgrade to 3.x #58

Closed stof closed 2 years ago

stof commented 2 years ago

react-syntax-highlighter is still using refractor 3 for now. As prismjs 1.27.0 fixes an XSS vulnerability, would you agree to backport that upgrade to a 3.x version of refractor ?

wooorm commented 2 years ago
72636c commented 2 years ago

Here you go:

I think many of us on 3.x haven't upgraded because our packages/tools are not ESM compatible yet. There's little I can add to the existing discourse on this subject except to say that it shouldn't be trivialised when we have major parts of the ecosystem (think Jest, TypeScript) struggling to land support.

wooorm commented 2 years ago

Thanks

wooorm commented 2 years ago

Released!

stof commented 2 years ago

@wooorm I'm not using react-syntax-highlighter myself. I'm using Storybook, which uses react-syntax-highlighter

stof commented 2 years ago

And thanks for this backport.

wooorm commented 2 years ago

Perhaps you can ask Storybook folks, though?

okonet commented 2 years ago

Storybook will most probably switch from react-syntax-highlighter to something else in the future since we ran into other blockers with it: https://github.com/storybookjs/storybook/issues/18090

wooorm commented 2 years ago

I don’t exactly get what’s going on there, but it seems to reference Prism actually running in the browser? I don’t think this package, nor react-syntax-highlighter, cause that? Can you point me to the exact problem there?

okonet commented 2 years ago

Hmm, I don't know where the "real" Prism JS came from but it was a race condition caused by it for sure. I could spend more time investigating the dependency problem for sure.

okonet commented 2 years ago

I see the dependency here: https://github.com/react-syntax-highlighter/react-syntax-highlighter/blob/master/package.json and Storybook is using it here: https://github.com/storybookjs/storybook/blob/ca2441260bdf2eaf29e6cf58d34bde60856ca7e9/lib/components/src/syntaxhighlighter/syntaxhighlighter.tsx. So I'm also not sure how it's possible since https://github.com/react-syntax-highlighter/react-syntax-highlighter/blob/master/src/prism-light.js not importing it.

wooorm commented 2 years ago

There is some code to prevent Prism from doing weird stuff: https://github.com/wooorm/refractor/blob/c5744cb0c8df2309bab35c0505be51a2e37838f9/lib/core.js#L40-L56. It could be that that’s not working (on a past version?)?

okonet commented 2 years ago

Yeah it seem like it. There is another approach which I really like: https://github.com/FormidableLabs/prism-react-renderer/blob/master/patches/prismjs%2B1.26.0.patch

Essentially it removes unused code instead of working around it. It also cuts the bundle size quite significantly.

Maybe this approach could be considered in this repo as well?

wooorm commented 2 years ago

Landed that idea, thanks! react-syntax-highlighter isn’t maintained actively though. You might still want to do that in house (and also potentially switch to lowlight/starry-night/etc)