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

Escaping <style> tag content #3496

Closed
asbjornenge opened this issue Mar 24, 2015 · 6 comments
Closed

Escaping <style> tag content #3496

asbjornenge opened this issue Mar 24, 2015 · 6 comments

Comments

@asbjornenge
Copy link

According to #3399 and #3465 React uses escapeTextContentForBrowser to replace " with " etc. This behavior is sane and awesome!

Problem is that it also happens inside <style> tags, which is not so great. Turns things like url("yolo.png") into url("yolo.png") which of course breaks.

It works fine if applied using dangerouslySetInnerHTML.

@syranide
Copy link
Contributor

Unless I'm mistaken you must use dangerouslySetInnerHTML with style elements, updating style element that uses children will break IIRC (this should be documented/warned for).

@asbjornenge
Copy link
Author

@syranide 👍 As noted it works when using dangerouslySetInnerHTML. I'm just not sure what the correct approach here should be. A style tag should contain css, so one would expect that to be valid?

@jviereck
Copy link
Contributor

Maybe using <style> is not such a good idea, as I had to learn when experimenting with it. Take a look at the following code that uses <style>. When you try to transpile it to JavaScript using babel.js there will be parser errors:

// Just define a simple `<div>` element here. Everything works just fine.
var myDiv = <div>test</div>;

// As everything between two { ... } brackets is interpreted as plain JS, the following
// style definition will have the variable `myDiv` as contnat.
var myStyle = <style>.className { myDiv }</style>

// The following will not parse as `2px` is not valid JavaScript.
var myStyleAgain = <style>.className { padding: 2px }</style>

You can look at the output of the babel-online-repl here: http://bit.ly/19heJyO

The main issue is that in JSX everything between {...} is interpreted as JavaScript. But in the case of CSS the content in the brackets has a different semantics. For sure the JSX parser in babel.js could be adjusted to cope for these cases (in fact, I did this by hacking acron-jsx), but the result looked very confusing to me. Eventually I've ended up defining a <StyleDef content={cssString} /> React element - you can find the code here: https://gist.github.com/jviereck/9a71734afcfe848ddbe2.

Hope this is helpful.

@asbjornenge
Copy link
Author

@jviereck Interesting... Personally I'm not handwriting the css inside the styletag. I'm using a browserify transform (post babelify) to require the styling -> css-string. So I'm not getting any of the babel errors. But that's beside the point.

I guess the issue is whether jsx should support the style tag or not...? If it is supported I think it makes sense to support css inside it? Or...? What do you guys think?

My two cents;

  • I'm experimenting with co-locating everything related to the function of a component. This includes internal layout styling and sometimes even small images. Images we can base64 encode, and styling we can apply in a style tag.
  • There are lots of tooling built up around styling and css; sass, less, stylus, etc. and they all have lots of great plugins and features etc. Seems to me a bit of waste leaving all that behind and doing all the styling with pojo's.

@murashki
Copy link
Contributor

I thikn if would be nice if we could use shorthand to report jsxTrans. if we want to set inner content w/o escaping. May be something like:

var a = <div !>url("yolo.png")<div>;

Yes, it's danger, but can you imagin if all templates engines force us to use long syntax construction for unescaped content.

We have this features in other template engines and i think it is not necessarily to use React only fo complex components with some logic, but a plain template engine! I have no Mustache or Embeddedjs (etc.) in my project cause of React and it's cool and i believe that's some features would be easy accessible.

@zpao
Copy link
Member

zpao commented May 26, 2015

JSX is very much a simple transform and we have no intention of making it special case specific tags. We also don't want to introduce complicated semantics. In this case you should use dangerouslySetInnerHTML. We may be able to make React warn if it gets a style element with children, but that should probably be tracked separately.

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

5 participants