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

Allow html conditional comments and doctype #1035

Closed
natew opened this Issue Feb 6, 2014 · 54 comments

Comments

Projects
None yet
@natew
Copy link

natew commented Feb 6, 2014

I'm hacking on an fully server and client compatible app using React. One obstactle I'm running into is it seems react doesn't like exclamation marks in your jsx-style code.

Here's an example of a pretty standard boilerplate for HTML:

<!doctype html>
  <!--[if IE 7]>    <html class="no-js ie ie7 ltie8 ltie9"><![endif]-->
  <!--[if IE 8]>    <html class="no-js ie ie8 ltie9"><![endif]-->
  <!--[if gt IE 8]> <html class="no-js ie gtie8"><![endif]-->
  <!--[if !IE]><!--><html class="no-js" xmlns:og="http://opengraphprotocol.org/schema/" xmlns:fb="http://www.facebook.com/2008/fbml" ><!--<![endif]-->
    <head>
      <meta charset="utf-8">
      <meta http-equiv="X-UA-Compatible" content="IE=edge">
      <title></title>
      <meta name="description" content="">
      <meta name="viewport" content="width=device-width, initial-scale=1">

      <link rel="stylesheet" href="/css/app.css" type="text/css" media="all" />
    </head>
    <body>
      <!--[if lt IE 8]>
        <p class="browsehappy">You are using an <strong>outdated</strong> browser. Please <a href="http://browsehappy.com/">upgrade your browser</a> to improve your experience.</p>
      <![endif]-->

      {this.props.page}

      <!-- <script src="/js/vendor.js"></script> -->
      <script src="/js/app.js"></script>
    </body>
  </html>

I think it's getting stuck on the conditional comments in this example.

I would prefer to keep this code in the HTML style, and to render it from the server without making major changes, it needs to be inside a react class. I'd be open to trying to hack this together, I may take a stab sometime this coming week.

@natew

This comment has been minimized.

Copy link
Author

natew commented Feb 6, 2014

After checking further, it did seem to break on the doctype (!) as well.

@jhiswin

This comment has been minimized.

Copy link
Contributor

jhiswin commented Feb 6, 2014

JSX is not HTML. You'll want to look at the HTML to JSX compiler. http://facebook.github.io/react/html-jsx.html

You won't be able to generate doctypes or conditional comments with React. This is technically impossible by design. If you want functionality like this, you might consider creating a script or addon to html-jsx to generate a javascript (JSX) equivalent.

@petehunt

This comment has been minimized.

Copy link
Contributor

petehunt commented Feb 6, 2014

While this is true it sucks for people who want to do full-page server rendering. I think if we claim to support full-page server rendering we should at least support doctype.

@natew

This comment has been minimized.

Copy link
Author

natew commented Feb 7, 2014

I can see a good reason for leaving out these portions of the HTML spec. I ended up adding on to this branch of react-app-middleware some changes to allow pulling in a static HTML template initially and then rendering components results into it.

See an example of it being used in this repo (see server.js).

@zackify

This comment has been minimized.

Copy link

zackify commented Feb 7, 2014

@petehunt I agree with you but I also think it's not really that practical to render full pages inside of react.

@natew

This comment has been minimized.

Copy link
Author

natew commented Feb 7, 2014

I think a good reason against is that React is used by many as a client side library, and therefore should be kept as light as possible.

Perhaps there could be an extension made that implements the "rest" of the HTML5 spec, but could be mixed in only on the server side as client's shouldn't need to render stuff like doctype once the server has.

@jhiswin

This comment has been minimized.

Copy link
Contributor

jhiswin commented Feb 7, 2014

I guess you could use React.DOM.injection and modify the JSX transform to read <!doctype.
Plain conditional comments could probably be done similarly.

@jhiswin

This comment has been minimized.

Copy link
Contributor

jhiswin commented Feb 7, 2014

@natew
How do you deal with void tags like <img> and <meta> right now? Do you self-close them before embedding them?
Would you use a tool like html-jsx to transform the html to syntax that the jsx transformer understands?

@natew

This comment has been minimized.

Copy link
Author

natew commented Feb 7, 2014

I have images working like this: <img src={this.imagePath + image.thumb} /> with no problems.

I have played with the html to jsx transformer thats on React's site currently, but haven't needed it yet personally. Is there a document that outlines all the supported tags? I haven't checked, but I also haven't gotten too deep into things yet.

Edit: As for the self-closing, yea I've always felt more accustomed to using them anyway.

@jhiswin

This comment has been minimized.

Copy link
Contributor

jhiswin commented Feb 7, 2014

On second thought, I think I may have missed the point above. Adding something like React.DOM._doctype would be useful, and would have very little impact on React's code.
Same could probably be done with conditional comments (React.DOM._ConditionalCommentHide ,React.DOM._ConditionalCommentShow). Inserting conditional comments using innerHTML might work? I'm not sure how updating them will work though.

I don't think there's a document for it (@Daniel15 )
A few issues I've seen with the html-jsx transformer:
strips script tags
strips html head body tags
<style> tag content isn't transformed
namespaced attributes (xmlns:fb or data:myattr) don't work
There's a few issues with css styles and attribute case-sensitivity in another github issue.
Since it uses the browser dom (you can use jsdom so you don't have to use a browser), all tags supported by browsers should work. Has worked pretty well for me.

html-jsx transformer does things like turn class -> className and style strings into objects and turn void tags (<img>) into self-closing tags (<img/>).
I'm creating tools that transform user-provided html/data (as-is) into components, so I've had to make use it.

@wyuenho

This comment has been minimized.

Copy link

wyuenho commented Feb 17, 2014

I too would like to see JSX support HTML conditional comments and not do the things @jhiswin described. I was going to implement a JSX preprendering tool until I found out I couldn't even write HTML comments and doctype...

@petehunt

This comment has been minimized.

Copy link
Contributor

petehunt commented Feb 17, 2014

I'm not sure if html comments are a deal breaking use case -- are conditional comments the only solution to these problems? -- and doctype can be prepended by your web server request handler or static build system.

@wyuenho

This comment has been minimized.

Copy link

wyuenho commented Feb 17, 2014

Prepending doctype is easy, conditional comments are harder to because that's the easiest way to conditionally load IE shims, short of this option I'll have to write a bunch of JS pre-shims for browser detection or fiddle with the webserver config, which I may not always have access to. IE8 users are still ~25% of the market. Is it difficult to elevate HTML comments to have same status as JS comments in your Esprima fork?

@petehunt

This comment has been minimized.

Copy link
Contributor

petehunt commented Feb 17, 2014

@jhiswin

This comment has been minimized.

Copy link
Contributor

jhiswin commented Feb 17, 2014

@wyuenho As another take on it, you could have a preprocessor (like html-jsx-lib) turn <!x to <_x (or something like ConditionalComment) and then add React.DOM._x

My view on it is that you will need a preprocessor to deal with differences between HTML and JSX anyways.

@wyuenho

This comment has been minimized.

Copy link

wyuenho commented Feb 17, 2014

@jhiswin Or better yet, preprocess to JSX. As a hack it'll work, just have to make sure to spit out the right string what renderComponentToString is called. The extra setup is a hassle tho. This also denies the claim that React is designed to be easily rendered on the server.

@wyuenho

This comment has been minimized.

Copy link

wyuenho commented Feb 17, 2014

I still think it's best to have JSX support as many HTML5 constructs as possible. Just fewer surprises.

@petehunt

This comment has been minimized.

Copy link
Contributor

petehunt commented Feb 17, 2014

BTW: rendering on the server doesn't necessarily mean that we need to be able to render full pages, especially since browser support for updating <html> and <head> is so sketchy. We've gone back and forth as to whether we want to actually support this or not for this reason.

We could probably get comments into core with a little work (mostly just cloning ReactTextComponent to work with comments and figuring out how to look it up by ID), not sure about the jsx transform. Not sure if we can prioritize this relative to other things (especially since this can be worked around), but I think a PR would definitely be considered.

@wyuenho

This comment has been minimized.

Copy link

wyuenho commented Feb 17, 2014

Will look into it this weekend. What's the release schedule for 0.9?

@petehunt

This comment has been minimized.

Copy link
Contributor

petehunt commented Feb 17, 2014

Sometime between Tuesday and Friday

@wyuenho

This comment has been minimized.

Copy link

wyuenho commented Feb 17, 2014

No rush then :)

@natew

This comment has been minimized.

Copy link
Author

natew commented Feb 19, 2014

Comments will be interesting because the typical setup is to have a number of them all in a row, without a closing tag. Not sure the syntax or tricks you'd have to do to build a root level component that has all the opening tags in conditionals.

@syranide

This comment has been minimized.

Copy link
Contributor

syranide commented Feb 19, 2014

Just a heads-up, there are a bunch of bugs with comments in IE8 in particular I think, where they get merged or get lost IIRC. Shouldn't be a problem for server-rendering, but it may mess up client-side _mountIndex etc.

@balanceiskey

This comment has been minimized.

Copy link
Contributor

balanceiskey commented Jul 14, 2014

Has there been any further progress on bringing doctypes into React?

@syranide

This comment has been minimized.

Copy link
Contributor

syranide commented Jul 14, 2014

@balanceiskey React can't mount higher than body client-side, so I doubt there is much pressure to get it done. And I'm not sure if I understand the issue, can't you just prepend a fixed doctype before the React output?

@balanceiskey

This comment has been minimized.

Copy link
Contributor

balanceiskey commented Jul 15, 2014

@syranide Prepending doctype just feels a touch hacky. That said, you're right, it's probably not a huge issue.

@zpao zpao added the wishlist label Jul 26, 2014

@GetContented

This comment has been minimized.

Copy link

GetContented commented Oct 10, 2014

I think React's rendering model needs to be separate from the DOM. There are React components for the html, body and iframe tags, so the idea of the DOM being separate is a good one, IMHO.

Prime use case is iFrames as a component, which has a DOM itself and therefore INCLUDES doctype, HTML, HEAD, BODY and other top-level tags, so I think if iframe is actually officially being supported as a component then it's fundamental to support these other components.

@adjavaherian

This comment has been minimized.

Copy link

adjavaherian commented Jan 15, 2015

Should I not be using React to render complete documents before serving?

@zackify

This comment has been minimized.

Copy link

zackify commented Jan 15, 2015

@adjavaherian You shouldn't need to.

@xcatliu

This comment has been minimized.

Copy link

xcatliu commented Jun 25, 2015

I've found a hack way to insert comments to html:
https://nemisj.com/conditional-ie-comments-in-react-js/

@nuweb

This comment has been minimized.

Copy link

nuweb commented Sep 11, 2015

+1 doctype for full-page server rendering.

@vizath

This comment has been minimized.

Copy link

vizath commented Sep 18, 2015

For my usecase, I use React to render emails using React.renderToStaticMarkup().
It would be nice to add conditionnal comments for Outlook.

@zackify

This comment has been minimized.

Copy link

zackify commented Sep 23, 2015

In React 0.14, rendering the <body> tag (at least with the test utils) raises an invariant violation. Not sure if I should be mentioning that here, or make a new issue. I figured it applies to this.

@zpao

This comment has been minimized.

Copy link
Member

zpao commented Sep 23, 2015

@zackify That's unrelated, please file a new issue with steps to repro

@schickling

This comment has been minimized.

Copy link

schickling commented Nov 11, 2015

Any update on this?

@zpao

This comment has been minimized.

Copy link
Member

zpao commented Nov 11, 2015

No update.

@baygeldin

This comment has been minimized.

Copy link

baygeldin commented Feb 8, 2016

I've just ran into this. The ability to render full page with just react can help to deal with SEO, title and many other things with much flexibility. And you won't even need something like helmet or react-document-title. And it simply seems logical to me to handle whole application state within react. While prepending doctype is easy, other things are really hard or impossible sigh.

@baygeldin

This comment has been minimized.

Copy link

baygeldin commented Feb 10, 2016

Well, I found out that some people say that changing head with virtual DOM can push some browsers to go crazy, which is definitely worse than not having whole app within react. Maybe, it wasn't a good idea after all...

@jimfb

This comment has been minimized.

Copy link
Contributor

jimfb commented Feb 10, 2016

@baygeldin Specifically, can you define "go crazy"? Which browsers, changing which nodes?

@baygeldin

This comment has been minimized.

Copy link

baygeldin commented Feb 10, 2016

@jimfb I guess I should have said "an unexpected behavior". I couldn't find a good example, but from what I understood the main arguments are: 1) Browsers do not expect replacing the whole nodes in the head, so their behavior is not standardized 2) It's a common practice for browser-extensions to inject external resources to the head, react will override those if it handles the head tag. Also, according to MDN, document.head is read-only. Sorry, I don't have concrete examples and time to investigate it :( I decided to go the standard way of doing things.

@syrnick

This comment has been minimized.

Copy link

syrnick commented May 17, 2016

+1 for this. It's quite a bit unexpected that server-side react can't actually render the whole page.

@optimalisatie

This comment has been minimized.

Copy link

optimalisatie commented Jul 8, 2016

Hi!

I have found a new method to include HTML comments (e.g. conditional IE comments) in JSX and React components using a Web Component.

https://github.com/optimalisatie/react-jsx-html-comments

<react-comment>[if lt IE 8]</react-comment>
<p class="browsehappy">You are using an <strong>outdated</strong> browser. Please <a href="http://browsehappy.com/">upgrade your browser</a> to improve your experience.</p>
<react-comment>[endif]</react-comment>
@sumbad

This comment has been minimized.

Copy link

sumbad commented Oct 8, 2016

Hello, I added additional component Head in my server-side to resolve this situation:

class Head extends React.Component{
    render() {
        let dangerousInnerHTML: string[] = this.props.children.map(value => {
            if (typeof value !== 'string')
                return ReactDOMServer.renderToStaticMarkup(value);
            else
                return value;
        });

        return (
            <head dangerouslySetInnerHTML={{ __html: dangerousInnerHTML.join('') }}>
            </head>
        );
    }
}

and now I can just use it:

        return (
            <html lang="ru">
                <Head>
                    <meta charset="utf-8" />
                    <title>Example</title>
                    <meta name="viewport" content="width=device-width, initial-scale=1" />
                    <meta http-equiv="X-UA-Compatible" content="IE=edge" />
                    <link rel="stylesheet" href="stylesheets/views/index/index.css" />
                    <script src="javascript/index/bundle.js"></script>
                    {`
                        <!--[if lte IE 8]><link rel="stylesheet" href="javascript/css/ie8.css" /><![endif]-->
                        <!--[if lte IE 9]><link rel="stylesheet" href="javascript/css/ie9.css" /><![endif]-->
                    `}
                 </Head>
...

Maybe this is not perfect solution and requires some additional code. Please, review and say what do you think about it. Thanks.

@psixdev

This comment has been minimized.

Copy link

psixdev commented May 15, 2017

@sumbad The children can contain an array, this requires a separate check, because ReactDOMServer.renderToStaticMarkup can not render arrays.

@dominikwilkowski

This comment has been minimized.

Copy link

dominikwilkowski commented Jun 7, 2017

This is definitely something needed for server side rendering. For right now we have to inject it after we call renderToStaticMarkup. An official way would be much better.

wmfgerrit pushed a commit to wikimedia/marvin that referenced this issue Sep 7, 2017

Chore: convert page template to (P)React
Make the server page template just another component for parity with the
client. `doctype` is specified by the server itself since this is
independent of the component and not practical to include before
rendering. Additionally, fix chunk scripts in the Page component and
update the Page tests. Tests could be made less brittle using Domino and
rendering to and asserting a real DOM.

https://github.com/developit/preact-render-to-string/blob/e1d7f47/README.md#render-jsx--preact--whatever-via-express
facebook/react#1035

Change-Id: Ibe1d18ca2c0e1466146578b50df1afd4fad6c9ab
@khilnani

This comment has been minimized.

Copy link

khilnani commented Sep 10, 2017

+1

@gaearon

This comment has been minimized.

Copy link
Member

gaearon commented Oct 1, 2017

Closing as there is an easy workaround (string manipulation before sending the markup for doctype, and dangerouslySetInnerHTML for comments). React also doesn’t support IE8 anymore, and the only remaining browser for which conditional comments are relevant is IE9.

Adding support for comments or doctype would have to span multiple projects (React, Babel, ESLint, etc) and yields little benefit, so it is unlikely we’ll be working on this.

@dominikwilkowski

This comment has been minimized.

Copy link

dominikwilkowski commented Jan 10, 2018

For everyone still finding this issue and wondering just how to use dangerouslySetInnerHTML in your code, have a look at this article: https://nemisj.com/conditional-ie-comments-in-react-js/

TL;DR:

renderHead() {
  return (
    var comment = '<!--[if lte IE 9]><script src="/public/media.match.js"></script><![endif]-->';
    <head>
      <title>Website title</title>
      <meta name="react-comment-hack" 
          dangerouslySetInnerHTML={{__html: comment}}>
      </meta>
    </head>
  );
}
@shaodahong

This comment has been minimized.

Copy link

shaodahong commented Dec 29, 2018

still error

Invariant Violation: meta is a void element tag and must neither have `children` nor use `dangerouslySetInnerHTML`
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.