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

Remove need for Oy Components #33

Open
revivek opened this issue Feb 19, 2016 · 3 comments
Open

Remove need for Oy Components #33

revivek opened this issue Feb 19, 2016 · 3 comments
Milestone

Comments

@revivek
Copy link
Owner

revivek commented Feb 19, 2016

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

@revivek
Copy link
Owner Author

revivek commented Mar 10, 2016

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 and its children are mine to do with what I’d like. Don’t bother.”) with validations, past what seem like slight ergonomics?

@revivek revivek changed the title Possible to allow ReactElements instead of requiring Oy Components? Remove need for Oy Components Mar 10, 2016
@revivek revivek modified the milestones: 0.6.0, 1.0.0 Mar 14, 2016
@jackcallister
Copy link

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?

@revivek
Copy link
Owner Author

revivek commented Apr 29, 2016

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

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

2 participants