API Discussion: Why isn't template just a render function? #47

aickin · 2017-03-02T02:24:41Z

First off, congratulations on your launch! Nice job. 🚀🌈🔥
Also: thanks for having good docs!

It's great that you included static templates, but I'm a little confused about your choice to use template strings rather than just making another render function with plain old React elements. It seems to me that using template strings requires the developer to learn a second API (in addition to render), but that's not really required. If instead you made it a plain old function (called, say, `renderTemplate``), I think Rapscallion might have a more compact, learnable API.

To get a sense for what this would look like, the example in the template section would become:

import { render, renderTemplate } from "rapscallion";

// ...

app.get('/example', function(req, res){
  // ...

  const store = createStore(/* ... */);
  const componentRenderer = render(<MyComponent store={store} />);

  const responseRenderer = renderTemplate (
    <html>
    <body>
      {componentRenderer}
      <MyOtherComponent />
      <script>
        // Expose initial state to client store bootstrap code.
        window._initialState = {JSON.stringify(store.getState())};
        // Attach checksum to the component's root element.
        document.querySelector("#id-for-component-root").setAttribute("data-react-checksum", "{componentRenderer.checksum()}")
        // Bootstrap your application here...
      </script>
    </body>
    </html>
  );

  responseRenderer.toStream().pipe(res);
});

This also gets rid of the need to support callbacks in the templated strings, I think. Thoughts?

The text was updated successfully, but these errors were encountered:

divmain · 2017-03-02T06:40:58Z

There are a couple of reasons for this. React doesn't support conditional comments or doctype tags, and it looks like that's not something that will change. If you need to inject <script> tags for analytics purposes, or really for anything aside from your React components, it gets a bit clunky.

In all of our sizable projects, we always end needing these things, and the template tag approach was the most ergonomic I came up with. I'd be open to a lower-level API that the template tag relied upon, but that you could also invoke directly. Definitely interested in your thoughts and ideas here.

cescoferraro · 2017-03-03T14:30:37Z

@divmain First off, thanks for the worderful library! I have been waiting for this in a long time!

I was about to open the same issue. Because the first instinct when finding out about the library was to search for renderToStaticMarkup function. The analogous non streaming react-dom/server module exposes a single object with just two methods on it. It would be nice if we could follow the Principle_of_least_astonishment, and make the API's match

Similar to renderToString, except this doesn't create extra DOM attributes such as data-reactid, that React uses internally. This is useful if you want to use React as a simple static page generator, as stripping away the extra attributes can save lots of bytes.

https://facebook.github.io/react/docs/react-dom-server.html

I ended up doing the following on the server

        withAsyncComponents(ServerApp)
            .then((result) => {
                //  Instead of
                // response.send("<!DOCTYPE html>" +
                //     ReactDOMServer.renderToStaticMarkup(
                //         <HTML userAgent={userAgent} css={css} result={result}/>
                //     ));
                // I am doing
                   template`${<HTML userAgent={userAgent} css={css} result={result}/>}`
                    .toStream().pipe(response);  
                // Proposed
                   renderToStaticMarkup(<HTML userAgent={userAgent} css={css} result={result}/>)
                    .toStream().pipe(response);  
        });

Which makes me wonder. why not just alias the templete function so the API actually match.

It seems to me that creating another templating language instead of using react itself does not actually follow the Unix_philosophy . But I am probably overlooking things. I have never tried to add analytics to react.

There are a couple of reasons for this. React doesn't support conditional comments or doctype tags, and it looks like that's not something that will change. If you need to inject <script> tags for analytics purposes, or really for anything aside from your React components, it gets a bit clunky.

People seem to solve this with either this
https://github.com/optimalisatie/react-jsx-html-comments
and with dangerousInnerHTML, but I have no clue about how dangerous this is

The below just works. Breaks the interface, but conveniently set html5 as default, which is probably true if one is doing ssr.

export const renderToStaticMarkup =
    (Component: JSX.Element, doctype: string = "<!DOCTYPE html>") =>
        template`${doctype}${Component}`;

aickin · 2017-03-03T18:12:19Z

@cescoferraro Thanks for the contribution. I was thinking of a similar wrapper as well. The one thing I'm not clear about is if template will render a React Element that has a Rapscallion renderer as a descendant. For example, it's clear that this works:

const renderer = template`
 <div>
  ${render(<span>)}
 </div>
`

but it's not totally clear from the docs if this will work or not:

const renderer = template`${
 <div>
  {render(<span>)}
 </div>
}`

The difference being that in the first case, the Rapscallion renderer is a passed directly to the template, whereas in the second case, the Rapscallion renderer is a child of a React Element that is passed to the template. If it can correctly render the latter, then it's absolutely possible to build renderToStaticMarkup on top of template.

I also agree with you that the doctype thing seems really easily fixable as a second argument to the render method, or just outside of the API, as it's trivial to stream out a doctype before streaming out content.

I agree with @divmain that the current solutions for html conditional comments are not great. In my experience, conditional comments aren't particularly important to support, as IE9 is the only browser that supports both conditional comments and React, and IE9 is only about a quarter of a percent of global browser share and falling.

If you really, really wanted to support conditional comments in server rendering, you could make a <ConditionalComment> React Component that is specially understood by Rapscallion's template renderer to create conditional comments. In the normal renderer it would have to throw an error, as client-side React won't understand it and be able to reconnect to the markup. This type of solution would probably cover 80% of uses of conditional comments, like cases where script and CSS includes are only included on certain browsers. Since <ConditionalComment> would be part of the React tree, though, it would not be able to work with situations where the conditional comment wraps an opening tag but not a closing tag of a particular element.

As for script tags, it seems to me pretty trivial to make a <EscapedScript> React Component that just reads out its lone string child using dangerouslySetInnerHtml, but I may be misunderstanding the problem.

divmain · 2017-03-03T18:56:59Z

@aickin, re this:

but it's not totally clear from the docs if this will work or not:

const renderer = template`${
 <div>
  {render(<span>)}
 </div>
}`

I doubt that'll work as things stand now. However, if you wanted something like that, you could always do:

const renderer = render(
  <div>
    <span />
    <MyComponent />
  <div>
);

I may be missing what you're going for here, though...

If you want the equivalent of renderToStaticMarkup, this should be supported by includeDataReactAttrs . It's not an identical API to React's, but it seemed better than bloating the API with toStaticContentPromise and toStaticContentStream methods.

So there'd be nothing stopping you from doing

const renderer = render(
  <div>
    <span />
    <MyComponent />
  <div>
);
renderer.includeDataReactAttrs(false);

or

const renderer = template`
 <div>
  ${render(<span>)}
 </div>
`;
renderer.includeDataReactAttrs(false);

That should give you clean markup.

I'll think on this a bit more, and probably solicit some opinions from others who set up the SSR boilerplate regularly. Thanks for all the input!

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

API Discussion: Why isn't template just a render function? #47

API Discussion: Why isn't template just a render function? #47

aickin commented Mar 2, 2017

divmain commented Mar 2, 2017

cescoferraro commented Mar 3, 2017 •

edited

Loading

aickin commented Mar 3, 2017

divmain commented Mar 3, 2017

API Discussion: Why isn't template just a render function? #47

API Discussion: Why isn't template just a render function? #47

Comments

aickin commented Mar 2, 2017

divmain commented Mar 2, 2017

cescoferraro commented Mar 3, 2017 • edited Loading

aickin commented Mar 3, 2017

divmain commented Mar 3, 2017

cescoferraro commented Mar 3, 2017 •

edited

Loading