Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Better support for <script> #654

Closed
vjeux opened this issue Dec 11, 2013 · 11 comments
Closed

Better support for <script> #654

vjeux opened this issue Dec 11, 2013 · 11 comments

Comments

@vjeux
Copy link
Contributor

vjeux commented Dec 11, 2013

&lt;script> is a bit hard to use right now.

  • If you have a {...} then it's going to start interpolating. And you are bound to have whenever you use if, function ... Workaround: you can wrap it inside of an interpolated string {'if (true) { ... }'}
  • If you have any ' or " it's going to output the html encoded version and Javascript is going to throw a parsing exception.

In order to workaround those two issues the best way I found is to use dangerouslySetInnerHTML and use ES6 backtick in order to have multi-line strings.

<script dangerouslySetInnerHTML={{__html: `
  (function(i,s,o,g,r,a,m){i['GoogleAnalyticsObject']=r;i[r]=i[r]||function(){
  (i[r].q=i[r].q||[]).push(arguments)},i[r].l=1*new Date();a=s.createElement(o),
  m=s.getElementsByTagName(o)[0];a.async=1;a.src=g;m.parentNode.insertBefore(a,m)
  })(window,document,'script','//www.google-analytics.com/analytics.js','ga');
  ga('send', 'pageview');
`}} />

We should probably make it easier, it's quite a pain to use right now.

@sophiebits
Copy link
Collaborator

What should it even mean to include a <script> tag? If you change the contents, does it get rerun? This seems fairly react-page-specific.

@vjeux
Copy link
Contributor Author

vjeux commented Dec 11, 2013

Yeah, i'm not really sure that it is really useful in a normal React app but for react-page it's definitively useful. @zpao suggested that we change the <script> tag to behave like in the DOM. It takes a single string child that is not escaped like we do for spans

@syranide
Copy link
Contributor

@vjeux @zpao

I could look into fixing this... from what I understand, what needs to be done is to create a ReactDOMScript-component that should have an invariant that checks that it's either empty or has only 1 string as child (or possibly many strings?) and of course works like it should.

What should happen if the string changes? Simply update the script-element and defer to the browser (I'm assuming they all do nothing) or recreate the script-element?

Seems about right?

@sophiebits
Copy link
Collaborator

We're getting rid of full-page rendering anyway (see #515, #585). I don't think it's worth doing anything special here.

@syranide
Copy link
Contributor

@spicyj It stills seems like being able to render into body will be supported, and even head. Even if only rendering into body was supported, this could still be useful. But yeah, perhaps not worth the extra bytes.

@vjeux vjeux closed this as completed Jan 6, 2014
@matthewwithanm
Copy link
Member

Just ran into this. IMO, it's very surprising behavior that adding a <script> to your virtual dom doesn't result in the script's evaluation (as appendChild would, for example). I created this fiddle to demonstrate the issue but, based on this thread, it seems like this is expected/desired?

@sophiebits
Copy link
Collaborator

@matthewwithanm Can you say a little more about your use case? I think we'd like to do the least surprising thing here but it's hard to know what that is; my guess is any behavior we have would be surprising in some way.

@marten
Copy link

marten commented Aug 26, 2014

@spicyj If I may answer for @matthewwithanm, since I just ran into the same issue, we still have some server-side rendered pages that we would like to open inside a modal dialog.

I made a Dialog component that gets a URL via props, and on componentDidMount fetches HTML from the server, and renders it inside the dialog using dangerouslySetInnerHTML. I was quite surprised when I found out that the code inside the script tag that was also in the fetched HTML was not executed.

@syranide
Copy link
Contributor

@marten dangerouslySetInnerHTML is just another name for innerHTML exposed by browsers, they do not execute <script> when using innerHTML. I believe jQuery has code that explicitly goes in and evals code inside <script>, if that's what you were expecting.

@marten
Copy link

marten commented Aug 27, 2014

@syranide I guess I was just spoiled by jQuery and never realized that. I've already implemented this same behaviour myself, and it's for a layer that should be converted to actual React components anyway, so the workaround should disappear from our code at some point. It's not a big deal, just surprised me, but I guess it's just a lack of knowledge on my part.

@bchenSyd
Copy link

bchenSyd commented Jun 1, 2017

@syranide thanks for sharing that! i didn't know it before

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

6 participants