Skip to content

Conversation

@j-clark
Copy link
Contributor

@j-clark j-clark commented Aug 30, 2016

Hi there,

This isn't a complete working/tested implementation. I'm just testing the waters to see if you guys would be amenable to a PR looking something like this.

An example problem

Imagine I've got 2 different widgets libraries being managed by 2 different teams. widgets-core is managed by 1 team, and is full of tried and true widgets built with React. widgets-experimental is full of the latest widgets being developed by a different team, also with React. Both teams deliver a .js file globally exposing their widgets and are separate from the Rails team.

I'm on the Rails team and everything is going great in my Rails app using react-rails until I start to use widgets form both widgets-core and widgets-experimental on the same page. They use different versions of React. The components are already unhappy with being rendered by a different version of React than they were defined with, but to make it worse, widgets-core is still pre-0.14 and widgets-experimental is using React 15.x, meaning that these versions of React have different rendering interfaces! I'm stuck!

Hopefully that example wasn't too contrived. It's very similar to a problem that I'm experiencing. This PR represents a potential way to solve the problem. Imagine a Rails partial that looks like this

= react_component('Widgets.Core.PrimaryWidget', 
  props, 
  { react_name: 'Widgets.Core.React',
    react_dom_name: 'Widgets.Core.React' })

= react_component('Widgets.Experimental.WizzBang', 
  props, 
  { react_name: 'Widgets.Experimental.React',
    react_dom_name: 'Widgets.Experimental.ReactDOM' })

This allows us our React components to be developed independently from the Rails app and to package up the React versions they need in order to render correctly.

Thoughts?

Totally open to suggestions and, of course, to renaming / shifting code around / whatever to conform to codebase style guides.

@j-clark j-clark changed the title Allow configurability of React/ReactDOM instance Allow configurability of React/ReactDOM instance per component Aug 30, 2016
@rmosolgo
Copy link
Member

rmosolgo commented Aug 31, 2016

Wow, that's a tight spot!

I'd like to support your case, but in a way that minimizes maintenance overhead for the library.

In #503, one little section of mountComponents was broken out into a separate function, getConstructor. This way, people could override that function if they wanted to lookup constructors in a new way. Could we apply a similar approach here?

For example, perhaps this line could be extracted to a separate function:

  • from

    ReactDOM.render(React.createElement(constructor, props), node);
  • to

    ReactRailsUJS.renderComponent(node, constructor, props)
    
    // .... then ... 
    renderComponent: function(node, constructor, props) { 
      ReactDOM.render(React.createElement(constructor, props), node);
    },

Then, in your case, you could provide a custom renderComponent function:

ReactRailsUJS.renderComponent = function(node, constructor, props) { 
  if (node.getAttribute("data-react-experimental")) {
    Widgets.Experimental.React // do whatever 
  } else {
    Widgets.Core.React // do whatever 
  }
}

And I think the view helper will pass any "unknown" properties on to the DOM element so

<%= react_component("MyComponent", {}, {data: {react_experimental: true}}) %>
<!-- I think this will become: -->
<div ... data-react-experimental="true" />

Would an extension point like that work in this case?

@facebook-github-bot
Copy link

Thank you for your pull request and welcome to our community. We require contributors to sign our Contributor License Agreement, and we don't seem to have you on file. In order for us to review and merge your code, please sign up at https://code.facebook.com/cla - and if you have received this in error or have any questions, please drop us a line at cla@fb.com. Thanks!

If you are contributing on behalf of someone else (eg your employer): the individual CLA is not sufficient - use https://developers.facebook.com/opensource/cla?type=company instead. Contact cla@fb.com if you have any questions.

@rmosolgo
Copy link
Member

rmosolgo commented Dec 1, 2016

I don't think this approach is a good fit for the gem in general, but if you get a chance to extract those UJS functions, please reopen!

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

Successfully merging this pull request may close these issues.

3 participants