oy icon indicating copy to clipboard operation
oy copied to clipboard

Remove need for Oy Components

Open revivek opened this issue 9 years ago • 3 comments
trafficstars

i.e. Allow <table> versus requiring <Table>. This would cut the API for usage down to one function: Oy.renderTemplate.

revivek avatar Feb 19 '16 21:02 revivek

Thinking out loud here:

  • We could use ReactTestUtils.createRenderer to fork, decorate, and modify the ReactElement tree. This works because we have no state on the server. Yes, this uses an experimental feature in react-addons-test-utils. Also, interesting to note what the React docs say about shallow rendering: “We're releasing this feature early and would appreciate the React community's feedback on how it should evolve.”
  • This would fundamentally change Oy from opt-in to opt-out. This may be desirable given the “best practice” goal, but we would need discoverable escape hatches. We could expose oy-skip and oy-skip={rules} props as a means of skipping validation.
  • This would require Oy.renderTemplate consuming the top-level React component directly, since it needs to be the one to ultimately call ReactDOMServer.renderToStaticMarkup. This is desirable, since (1) we can provide better security by rendering the component directly instead of allowing injection of an arbitrary string, and (2) doing so could move Oy.renderTemplate to a more familiar ReactDOM.render(<Foo />, element) API like Oy.renderTemplate(<Template />, templateOptions).
  • Implementing this would most certainly be a 1.0 breaking change.

Open questions:

  • Are there glaring performance penalties from using ReactTestUtils.createRenderer? Email rendering performance, in my experience, has never been a performance-critical task. Usually rendering is done as a batch job or in development.
  • An additional escape hatch: Is there a common use case in not decorating a certain subtree (i.e. “This <Header oy-skip> and its children are mine to do with what I’d like. Don’t bother.”) with validations, past what seem like slight ergonomics?

revivek avatar Mar 10 '16 05:03 revivek

Nice work on removal of the Table components, I definitely agree that's the way to go by focusing on rendering. Just out of curiosity, what's wrong with the current renderer you've built over using ReactTestUtils.createRenderer?

jackcallister avatar Apr 27 '16 08:04 jackcallister

Thanks for the feedback. I'm still wary about the switch from opt-out to opt-in validation, but I guess that's what a major release tick is for :) I am using ReactTestUtils.createRenderer: https://github.com/revivek/oy/blob/next/src/utils/Tree.js#L21

revivek avatar Apr 29 '16 03:04 revivek