denoland / fresh

The next-gen web framework.
https://fresh.deno.dev
MIT License
12.17k stars 621 forks source link

dangerouslySetInnerHTML usage in <head> breaks style plugins #2387

Closed rojvv closed 5 months ago

marvinhagemeister commented 5 months ago

I'm afraid I don't have enough context on how dangerouslySetInnerHTML was used in combination with <head>. Can you share more context?

rojvv commented 5 months ago
<head dangerouslySetInnerHTML={{ __html: "<script>console.log(1);</script>" }} />
marvinhagemeister commented 5 months ago

Thanks for sharing more context. I see, that's the expected behavior. Setting dangerouslySetInnerHTML basically says: "Replace all of this elements content with the HTML string". It's the same thing as if you'd set .innerHTML on an HTML element directly. The way the .innerHTML property works is that it discards any child nodes present before and overwrites it with the passed HTML string.

rojvv commented 5 months ago

What is your recommendation to add arbitrary code to the head based on string-based config? Parse the HTML and convert it to JSX elements on the fly?

CAYdenberg commented 5 months ago

I provided some quick code here: https://github.com/denoland/fresh/discussions/2360#discussioncomment-8813171

You might have to expand to other tags (eg link) depending on your use-case, but I really think it's going to be a better strategy overall to have tight control over the HTML you are writing rather than "dangerous" injection of code.

If you're trying to inject a script, there's probably a better way to achieve your ultimate goal, however. Can you provide more info on the problem you're trying to solve?

rojvv commented 5 months ago

I have multiple things: some comments, a <noscript>, and a <script>. I think I better inject it in the middleware by interfering with the response stream through a custom ReadableStream impl.