Skip to content
This repository has been archived by the owner on Jul 28, 2019. It is now read-only.

How do you get angular code to compile within the rendered react components? #80

Closed
seanwestfall opened this issue Jun 15, 2015 · 26 comments

Comments

@seanwestfall
Copy link

First off guys, I love what you're doing. Bravo.

But how do you $compile anglarjs code within react components? Say you render a reactComponent within a reactDirective, provided by the ngReact module, that contains angular attributes -- such as ng-click="". How do you get those attributes activated and compiled to work with angular?

Here's some example code that I've been trying to work through that doesn't work. The ng-click never becomes activated, probably because the render is getting executed after the angular controller code is being executed...

Sorry, but here comes a lot of code.

<html>
    <slab-table table="table" />
    <div id=''btntest"></div>
</html>
<script type="text/jsx">
    var num = 4;
        var ngbutton = React.createClass({
            render: function() {
                return (
                    <ng-button type={this.props.type} className={this.props.class} data-ng-click={this.props.ngclick}>
                        {this.props.text}
                    <ng-button>
                );
            }
        });
        var dumpBtn = React.createElement(ngbutton, _.extend({}, this.props, { type: 'button', class: 'btn btn-success', ngclick: 'getTest()', text: 'Dump'}));
        React.render(dumpBtn, document.getElementById('btntest'));
</script>
<script>
var app = angular.module('app', ['react']);
app.value( 'SlabTable', React.createClass( {
    ....
});
app.directive( 'slabTable', function( reactDirective ) {
    return reactDirective( 'SlabTable' );
});
app.directive('ngButton', function($compile) {
    return {
        restrict: 'CAE',
        replace: true,
        link: function(scope, element, attrs) {
            console.log('I was called');
            var html = element.html();
            $compile(element.contents())(scope);
        }
    }
})

app.controller('mainCtrl', function($scope) {
    $http.get('address').then(function(data) {
        var thistable = {
            cols: [ '', 'name', 'chuck_size', 'chunks_per_page', 'get_hits', 'mem_requested', 'total_chunks', 'total_pages', 'used_chunks' ],
            rows: rows
        };
        React.render(React.createElement(SlabTable, _.extend({}, this.props, { table: thistable })),document.getElementById('react'));
    });

    $scope.getTest = function() {
        alert("ng-click worked!");
    }
});
</script>

I think, next I'll try modifying ngReact.js. From lines 143 to 168, within reactComponent, through the link function (https://github.com/davidchang/ngReact/blob/master/ngReact.js#L143-L168). Do you think it would possibly work if I put something in there to replace the given html code with $compileed code?

@kasperp
Copy link
Collaborator

kasperp commented Jun 15, 2015

@seanwestfall thanks for your question. I think we gotten this question before (see #49), using a directive from a React Component. I'm not sure how that would work, also I'm not sure if it is a good idea.

You can use angular services from react components and you can pass in callbacks from the angular controller to the React Components as discussed here #71. So another way to go might be to extract common code from the directives that you wish to use from React and rebuild the view part as a React Component. if I'm reading the example code correct, that would mean turning your ngButton into a React Component.

@seanwestfall
Copy link
Author

k, thanks @kasperp. I'm working on it right now. I got $compile injected into reactDirective, but now I'm having trouble transferring a data-* prop without JSX. I might need to use JSX for some of these parts.

I'll keep hacking away at this, and I'll let you know if I succeed at any of it.

@seanwestfall
Copy link
Author

Hey @kasperp, I got it to work.
It's possible to use an angular directive from within the React Component.

The only problem is is the renderComponent function is called too many times, so the event is attached too many times to the controller's scope, in the example below -- so I'll have to figure out how to make sure it only runs once per component:

The example bellow uses the jsx-transformer example (https://github.com/davidchang/ngReact/tree/master/examples/jsx-transformer):

// JSX
var Hello = React.createClass( {
  propTypes: {
    fname: React.PropTypes.string.isRequired,
    lname: React.PropTypes.string.isRequired
  },

  render: function() {
    return <span data-ng-click='test()'>Hello {this.props.fname} {this.props.lname}</span>;
  }
} );
setTimeout( function() {
  var app = angular.module('app', ['react']);

  app.controller('mainCtrl', function($scope) {
    $scope.person = {fname: 'Clark', lname: 'Kent'};

    // test function
    $scope.test = function() {
        alert("I worked!");
    }
  });

  app.directive('hello', function(reactDirective) {
    return reactDirective(Hello);
  } );

  angular.bootstrap(document, ['app']);
}, 1000 );
// https://github.com/davidchang/ngReact/blob/master/ngReact.js#L198-L242
function renderComponent(component, props, $timeout, elem, scope, $compile) {
    $timeout(function() {
      var rendered = React.render(React.createElement(component, props), elem[0]);
        $compile(rendered.getDOMNode())(scope);;
    });
  }

...
// https://github.com/davidchang/ngReact/blob/master/ngReact.js#L198-L242
var reactDirective = function($timeout, $injector, $compile) {
    return function(reactComponentName, propNames, conf) {
      var directive = {
        restrict: 'E',
        replace: true,
        link: function(scope, elem, attrs) {
          var reactComponent = getReactComponent(reactComponentName, $injector);

          // if propNames is not defined, fall back to use the React component's propTypes if present
          propNames = propNames || Object.keys(reactComponent.propTypes || {});

          // for each of the properties, get their scope value and set it to scope.props
          var renderMyComponent = function() {
            var props = {};
            propNames.forEach(function(propName) {
              props[propName] = scope.$eval(attrs[propName]);
            });
            renderComponent(reactComponent, applyFunctions(props, scope), $timeout, elem, scope, $compile);
          };

          // watch each property name and trigger an update whenever something changes,
          // to update scope.props with new values
          var propExpressions = propNames.map(function(k){
            return attrs[k];
          });

          watchProps(attrs.watchDepth, scope, propExpressions, renderMyComponent);

          renderMyComponent();

          //
          // cleanup when scope is destroyed
          scope.$on('$destroy', function() {
            React.unmountComponentAtNode(elem[0]);
          });
        }
      };
      return angular.extend(directive, conf);
    };
  };

  // create the end module without any dependencies, including reactComponent and reactDirective
  return angular.module('react', [])
    .directive('reactComponent', ['$timeout', '$injector' reactComponent])
    .factory('reactDirective', ['$timeout','$injector', '$compile', reactDirective]);
})

I think that you guys should really consider allowing the use of angular directives within the react component. The only reason why I've been considering using a combination of reactjs and angular -- and I believe others are also thinking along the same lines -- is because I want to be able to use an angular controller while rendering components with ReactJS. It's pretty pointless to use angular controllers, if their functions can't bind to elements in the DOM.

The only conditions would be that you need to use JSX, because ReactJS does not include anyway to transfer the ng-* props. You have to include them in JSX as data-ng-*, like in the example above.

If you and @davidchang allow for it, I'll do all the work -- I can write the test cases and documentation -- and submit via pull request -- if I can be included as a contributor.

@kasperp
Copy link
Collaborator

kasperp commented Jun 21, 2015

@seanwestfall thanks it look interesting and thanks for putting in the effort.

I'm however not convinced about the use cases for using directives inside react components.

is because I want to be able to use an angular controller while rendering components with ReactJS. It's pretty pointless to use angular controllers, if their functions can't bind to elements in the DOM

To do this I have been "passing" functions from angular controllers to React components through props. So In the example I would to pass in the test function as a prop and call it from a React event handler like so:

// React
var Hello = React.createClass( {
  propTypes: {
    fname: React.PropTypes.string.isRequired,
    lname: React.PropTypes.string.isRequired,
    test: React.PropTypes.func.isRequired
  },

  handleClick() {
     this.props.test();
  },

  render: function() {
    return <span onClick={this.handleClick}>Hello {this.props.fname} {this.props.lname}</span>;
  }
} );

// angular
var app = angular.module('app', ['react']);

app.controller('mainCtrl', function($scope) {

  function test() { 
     alert("I worked!");
  }

  $scope.person = {fname: 'Clark', lname: 'Kent', test: test };
});

app.directive('hello', function(reactDirective) {
  return reactDirective(Hello);
});

@davidchang any thoughts ?

@hasdavidc
Copy link
Collaborator

First off, thanks @seanwestfall for opening the issue and looking into the code and figuring out what's going on. Also, sorry for this delayed response. (Also, anyone who gets code merged in will definitely be considered a Contributor, but we still need to make sure only the right things get into the library.)

Like @kasperp, I'm not sure about your premise about needing to trigger Angular directives from within React. It seems like Kasper's code above should be preferable.

If you disagree, could you elaborate on:

  1. what your use case is?
  2. how intrusive you think the code change would be, and if it would negatively affect the performance of our common case

@seanwestfall
Copy link
Author

@hasdavidc
Let me do a little more research and I'll get back to you. I guess the appeal would be to allow for a more angular way of building web applications, where as @kasperp approach follows the react way. I'm not sure if it would be fair to say one is better than the other -- and if performance is being called in to question, much in the vain of thought of this entire project, it's possible to fit it inside and allow for it to be turned on and off. But, like I said, let me do more research first.

@zpratt
Copy link
Contributor

zpratt commented Aug 10, 2015

One use case is:

  • you are using the ui-router
  • you have a react component that contains an anchor that you want to trigger a state change
  • you can pass the $state service yes, but it would be slightly more elegant to use the ui-sref attribute directive
  • it seems the ui-sref-active directive is completely unavailable as well, which provides some convenience. Without it, it would likely require you to manage some cascading props through the root component bound to the route by again injecting $state and determining if the rendered anchor is the current state

Is it a killer feature? No. Is it useful? Yes. Thoughts?

@kandizzy
Copy link

Hi, are you all still considering this?
@seanwestfall I started with your code and modified the renderMyComponent function to destroy previous scope, and that stopped the multiple alert windows.

link: function(scope, elem, attrs) {
          var prevScope;
var renderMyComponent = function() {
            if (prevScope) {
              prevScope.$destroy();
              prevScope = null;
            }
            var props = {};
            propNames.forEach(function(propName) {
              props[propName] = scope.$eval(attrs[propName]);
            });
            prevScope = scope.$new();
            renderComponent(reactComponent, applyFunctions(props, prevScope), $timeout, elem, prevScope, $compile);
          };

My use case is using an angular-material directive in my React component, an md-button specifically. I still have an ngTransclude error with that though. Has anyone else tried using the two together?

@seanwestfall
Copy link
Author

@kandizzy I took a bit of a break from it, but I could definitely return to working on it. It looks like there are quite a few people out there who'd like to see this happen. Though, David Chang and Kasper are going to take some convincing.

@kasperp
Copy link
Collaborator

kasperp commented Sep 25, 2015

@seanwestfall perhaps it could be worth taking another look at it, there seems to be some demand for it.

I do get the point that it allowing it would allow you to develop apps in more of an "angular way". To be honest what drove me to this project was a wish to escape that, perhaps that is where my apprehension comes from. But you're absolutely right it is not fair to say what to say which is better, because as with all things it depends.

Are you still up for giving it a go?

I think we're still interested in knowing how intrusive you think the code change would be?

FYI @hasdavidc

@kandizzy
Copy link

@kasperp @seanwestfall i got this working without the ngTransclude errors on the angular-material components, we're testing it on our project. If you guys want to take a look at the changes they're here:
kandizzy@8a42245

@seanwestfall
Copy link
Author

Looks good @kandizzy, though I think it might be better to find out why it being attached to the scope so many extra times than just removing it or keeping track of it with a variable. Let me see if I can't find out why it's being attached to the scope so many times.

@kandizzy
Copy link

@seanwestfall cool sounds good. What i noticed in my use case is that it would be attached to scope as many times as there were properties being watched. My other ngTransclude error happened every time I called $compile after the first time, even after destroying prevScope and sending scope.$new instead.

@seanwestfall
Copy link
Author

It gets attached every time function renderComponent() is called. So it's attached even when the values change for the watch statement.

@kandizzy
Copy link

and then $compile walks the tree and associates everything to scope, so I think we can only call it once.

@seanwestfall
Copy link
Author

hey @kandizzy,
So here's how to do it without having to keep track of prevScope. Just use a property on the scope. It's not a big difference, but it saves a few lines of code. renderComponent has to be called for every property listed on the react component, and every time those properties are updated.

// https://github.com/davidchang/ngReact/blob/master/ngReact.js#L119-L123
function renderComponent(component, props, $timeout, elem) {
    $timeout(function() {
      var rendered = React.render(React.createElement(component, props), elem[0]);
      if(angular.isUndefined(scope.compiled)) {
          $compile(rendered.getDOMNode())(scope);
          scope.compiled = true;
      }
   });
}

I'll be working on the benchmarking tests, which is where I left off before I took a break from working on this.

@vigneshtr-ionic
Copy link

@seanwestfall, I am testing ngReact for my angular application to be integrate with the react component. But I am failing that I am using lot of angular injectors in htmls. For ex: filters- $filter('date'). is it possible to use this in jsx file? Please help me on this..

@kandizzy
Copy link

@seanwestfall nice! much cleaner.

@kasperp
Copy link
Collaborator

kasperp commented Sep 30, 2015

@vigneshtr-ionic thanks for you question. It should be poosible to use angular filters and services in a React components already. Did you check the section about reusing angular injectables in the README ?

This is issue is about also supporting directives inside React components.

@JalalAlbasri
Copy link

@kandizzy Thank you! kandizzy/ngReact@8a42245 is exactly what I needed to get angular-material directives working in react components. Cheers.

Update: Actually spoke too soon. It doesn't update when my prop values change, I know I need to call renderComponent but I'm not sure how :/

I'm also trying to use an md-button if the button is created initially it works fine and the md-button turns into a button but if I add a button after the initial load by changing the props of my react component it remains an md-button like it was before I started using your modified ngReact.js

@kandizzy
Copy link

@cavemanninja $compile only gets called once the first time the react component is rendered so adding a second directive like md-button after the initial load won't work. You could try ng-if to show a button conditionally instead of adding.

This was all working on our project, however we made the decision to remove React entirely to save load time/file size of the vendor scripts. Angular / Angular Material / and React was pretty big.

@JalalAlbasri
Copy link

@kandizzy I was really hoping that wouldn't be your answer.

No go on the ng-if, I'm generating a list from query results so there's no way for me to know how many buttons I'll need.

Do you reckon I could explicitly call $compile from my controller or rerender my react component, after I get my results and update my props?

@kandizzy
Copy link

@cavemanninja Ah I see, well I'm not sure how your app is structured but the first thought that comes to mind is to make each result it's own component so it will be rendered with it's button when you loop through the results.

@JalalAlbasri
Copy link

@kandizzy gotcha, so that means directives within components will be properly compiled the first time they are added to the DOM even if that is after the first load, but not if the prop values change and that causes new angular directives to be added.

I can work with that :) I'll just ng-repeat over the components, I think the benefits of using react for the DOM updates will still be there, Thanks!

@onemenny
Copy link

I'm having the same issue, $compile works for me as well, but I'm afraid this could lead to performance degradation on each render...
More use cases: is using angular bootstrap's tooltip, dropdown, ui-select which the whole app already use and you don't want to re-implement just for react.

@bryzaguy
Copy link

Is it possible that the $timeout is triggering multiple digest cycles and you just need to add the invokeApply argument to short circuit? Ex: $timeout(function() { ... }, 0, false)

@kasperp kasperp closed this as completed Feb 15, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

9 participants