These are chat archives for redfin/react-server

24th
May 2016
Douglas Wade
@doug-wade
May 24 2016 17:46
Some backstory for non-Redfinnians: We use a patched version of React 0.14 in production that catches render errors. We’re trying to move to React 15.
So I was thinking about the gulp module loader and source transforms and how to get off our patched React, and I think I have a crazy idea that I want to float by the gang. I’d like to use higher order components to get the effect of handling errors in production, but I’m not sure how to get the higher order component on every React component we create
Adding a export decoratorFactory.decorate(myComponent) might get tedious to add to every component, plus you would want to change the implementation of decoratorFactorybased on environment
Bo Borgerson
@gigabo
May 24 2016 17:49
:grimacing:
IMO the default error behavior is actually appropriate at the framework level. A render error is contained to the <RootElement> it arose within.
I think the reason we chase finer-grained error containment is because we have some really big root elements for historical reasons.
OTOH an error-containing higher-order component sounds really cool. Seems like it could be applied in a targeted way, even independent of react-server.
Douglas Wade
@doug-wade
May 24 2016 17:53
I agree; I don’t want to change the way that the framework handles errors, but I think there’s value in making it easier to apply higher order components.
Think also, for instance, of our timing data; it would be great to be able to use a timing higher order component that logged lifecycle times for every react component
I think there is something the framework could do to enable a higher-order component pattern:
  • Add a “higher order component factory” (name to be determined) key to .reactserverrc that points to a file (”factory”: “/relative/path/to/file”)
  • Do a source transform that adds an import of that file and adds a call to decorate around every React.createComponent() (var factory = require(“/relative/path/to/file”);… export factory.decorate(React.createComponent(myComponent));)
I think it would add a useful extension point for react-server that could be useful for lots of behaviors
Douglas Wade
@doug-wade
May 24 2016 17:59
To be clear, I’m not expecting the framework to address our problem directly; I’m proposing adding an extension point to make it easier for us to address our problem tersely (without editing every react component directly) and to allow it to be configured based on environment (maybe inject a logger or a sham based on environment, or add a supplementary timing HOC).
Bo Borgerson
@gigabo
May 24 2016 18:00
It's a cool idea. I'd really like us to be off of our hacked up React. :hurtrealbad:
Jason Benesch
@jbenesch
May 24 2016 19:43
@doug-wade please excuse me if I am a bit ignorant on the issue being a noob, but instead of using a tranform to wrap all components, couldn’t you traverse the children?
Douglas Wade
@doug-wade
May 24 2016 20:11
@jbenesch woah
Mmmmm… let me think about this for a second
Looking at the example
const replaceDivsWithSpans = (node) => traverse(node, {
  DOMElement(path) {
    if(path.node.type === 'div') {
      return React.createElement(
        'span',
        path.node.props,
        ...path.traverseChildren(),
      );
    }
    return React.cloneElement(
      path.node,
      path.node.props,
      ...path.traverseChildren(),
    );
  },
});

replaceDivsWithSpans(<div>This is a span.</div>)
// will render as:
<span>This is a div.</span>
It seems to be that this walks the tree after it is rendered?
What I need to do is something like this:
class ErrorObscuringComponent extends React.Component { 
  constructor(decorated) {
    this.decorated = decorated;
  },
  render() {
    try {
      return this.decorated.render();
    } catch (e) {
      return <noscript></noscript>;
    }
  }
}
Sasha Aickin
@aickin
May 24 2016 20:18
@doug-wade is this for handling errors thrown in the render method?
Douglas Wade
@doug-wade
May 24 2016 20:20
Or something like this:
export TimingComponent(decorated) => {
  let timer = logger.timer('some_work');
  let result = decorated.render();
  timer.stop();
  return result;
}
Sasha Aickin
@aickin
May 24 2016 20:21
have you seen unstable_handleError?
Douglas Wade
@doug-wade
May 24 2016 20:22
@aickin I haven’t seen unstable_handleError, and yes, that is the (a?) goal
Sasha Aickin
@aickin
May 24 2016 20:22
I'm not sure it's officially supported, and I'm trying to get it removed from React before it's officially supported so that React can support streaming, but it's an interesting thing
Douglas Wade
@doug-wade
May 24 2016 20:28
Looking at the unit test code, it looks like it works essentially like a try/catch for errors in children
    class Boundary extends React.Component {
      constructor(props) {
        super(props);
        this.state = {error: false};
      }
      render() {
        if (!this.state.error) {
          return (<div><button onClick={this.onClick}>ClickMe</button><Angry /></div>);
        } else {
          return (<div>Happy Birthday!</div>);
        }
      }
      onClick() {
        /* do nothing */
      }
      unstable_handleError() {
        this.setState({error: true});
      }
    }
Sasha Aickin
@aickin
May 24 2016 20:29
exactly. having an unstable_handleError method is essentially a catch block for descendant render methods
that said, I'm starting a campaign for it to not be officially supported, because it's fundamentally at odds with streaming support
Douglas Wade
@doug-wade
May 24 2016 20:30
But I think I would still have to edit every single react component?
And you’re campaigning against it?
Sasha Aickin
@aickin
May 24 2016 20:30
because obviously, if you've already streamed out part of the render tree and then you hit a render that throws, you can't go back to the stream and say "wait, delete the last 15KB"
Douglas Wade
@doug-wade
May 24 2016 20:31
So, sounds like let’s not use that?
Sasha Aickin
@aickin
May 24 2016 20:31
well, I may not win ;)
and what's your goal? if it's just to show an error page when a render throws, then you would only need to put a unstable_handleError method on the outermost component, right?
Douglas Wade
@doug-wade
May 24 2016 20:35
The proximate goal is to catch arbitrary errors in a render function, and return a noscript tag for the smallest block of affected React components possible
The broader goal is to do it in such a way that you could instrument arbitrary augmentations to React components using the same mechanism
Such as timing how long each component takes to complete each lifecycle method, for instance
Sasha Aickin
@aickin
May 24 2016 20:36
ah, then yes, would need to modify every component.