Skip to content

properties injection issue with controllerAs #121

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

Closed
bhagn opened this issue Oct 15, 2016 · 12 comments
Closed

properties injection issue with controllerAs #121

bhagn opened this issue Oct 15, 2016 · 12 comments

Comments

@bhagn
Copy link

bhagn commented Oct 15, 2016

I"m using the ui-scroll in its simplest form -

<div ui-scroll="item in datasource" adapter="listCtrl.adapter">
  <div ng-bind="::item.name"></div>
</div>

The problem I'm facing here is the adapter instance is not getting injected into the right scope/controller. listCtrl.adapter remains undefined.

  • I tried changing the adapter attribute to adapter on listCtrl. It complains saying - Failed to locate target controller 'listCtrl' to inject 'adapter'.
  • I set adapter="adapter". This injected the adapter instance into$scope.$parent` (this is probably as expected).

Looking at the code in ui-scoll.js (lines 517 - 524) -

while (candidate.length) {
  let controller = candidate.attr('ng-controller');
  if (controller === controllerName) {
    scope = candidate.scope();
    break;
  }
  candidate = candidate.parent();
}

Without specifying ng-controller explicitly it looks like the injection doesn't work as expected.

@mfeingold
Copy link
Contributor

mfeingold commented Oct 15, 2016

any chance you can give us a repro? On plunker or something?

@dhilt
Copy link
Member

dhilt commented Oct 30, 2016

@bhagn could you try the latest source code (https://github.com/angular-ui/ui-scroll/blob/master/src/ui-scroll.js)? there are some modifications around adapter assignment... If you still facing the issue and don't want to create a demo, please put your full template code in a comment.

@bhagn
Copy link
Author

bhagn commented Oct 30, 2016

I'm really sorry. I probably missed the mails asking me for a demo. I'm pasting the template code here - will create a demo soon and provide a link.

<!-- template.tpl.html -->
<my-dir>
    <div ui-scroll="item in rtList" adapter="scrollAdapter">
        <div class="item" ng-bind="::item.name" ng-click="rtList.remove(item.identifier)">
    </div>
</my-dir>
// Controller
function myDirController($scope) {
  var vm = this;
  vm.get = function(index, count, success) {
     success(itemsFromAPromise);
  }

  vm.remove = function(id) {
    // delete item
    $scope.$parent.scrollAdapter.reload();    // <--- this is the issue.
  }
}

function myDirective() {
   return {
      restrict: 'E'
      controllerAs: 'rtList',
      controller: myDirController,
      templateUrl: '/path/to/template.tpl.html',
      scope: true
   }
}

angualr.module('myModule', [])
  .directive('myDir', MyDirective);

I'll get back with a jsfiddle demo.

@dhilt
Copy link
Member

dhilt commented Oct 31, 2016

@bhagn I've just thought that this is a good idea to have such public demo, when the ui-scroll directive is encapsulated within some custom directive with it's own controller. So please follow it (especially the source code), I believe this could help. And again, you should try the latest ui-scroll code wich has not released yet and wich will be a part of ui-scroll 1.5.2 release.

@bhagn
Copy link
Author

bhagn commented Oct 31, 2016

@dhilt I tried with the master branch. I'm still seeing the same issue - adapter injected into $scope.$parent.

@dhilt
Copy link
Member

dhilt commented Oct 31, 2016

@bhagn and what about my demo? and what about your demo?

@bhagn
Copy link
Author

bhagn commented Oct 31, 2016

Your demo works fine.

Here's my demo (still not working :) ) - https://github.com/bhagn/ui-scroll/tree/master/demo/insideComponent
b.t.w it uses component. You can see the issue in the console.

Let me know if I need to create a PR for the example.

@dhilt
Copy link
Member

dhilt commented Nov 1, 2016

@bhagn You have no ui-scroll-viewport in your demo. According to the documentation (the part of assignable expressions):

If the viewport is not defined (window viewport), the $rootScope will be used as the target scope.

That are the reasons why the Adapter is being bound to the $scope.$parent.$ctrl (and not to $scope.$parent as you put in a code comment):

  • $scope.$parent in this case is equal to $rootScope;
  • $ctrl from the adapter="$ctrl.scrollAdapter" expression is not an accessible "Controller As" prefix.

Unfortunately the second point is still the problem. Both of cases (Component and Directive) have no pure "Controller As" declaration, I mean the markup. So the scroller has no chance to catch correct scope by DOM traversing... But why the case when we have ui-scroll-viewport does not face this issue? It is so because of $parse("ctrl.adapter").assign executes on the viewport scope wich is under the component/directive's controller scope wich is already existed in the scopes hierarchy with a container object named "ctrl" due to internal angular stuff... And of course this does not work when we have no viewport and trying to $parse "ctrl.adapter" from the $root.

So in your demo you should set the viewport directive in the template or provide an access to the adapter right via root-scope.

@bhagn
Copy link
Author

bhagn commented Nov 1, 2016

@dhilt Thanks for the clarification - I missed that part in the documentation.

@dhilt
Copy link
Member

dhilt commented Nov 18, 2016

@bhagn I've made a fix for no ui-scroll-viewport case too. Please try ui-scroll v1.5.2.

@dhilt dhilt closed this as completed Nov 18, 2016
@bhagn
Copy link
Author

bhagn commented Feb 13, 2017

@dhilt Having ui-scroll-viewport didn't help. I figured out that the ui-scroll directive depends on the DOM element's scope attribute to determine the scope into which the adapter has to be injected.

Unfortunately, when debug info is disabled (https://docs.angularjs.org/guide/production), the DOM elements' dom.scope() and dom.isolateScope() methods don't return a valid value. I think depending on the element's scope might be a bad thing.

@dhilt
Copy link
Member

dhilt commented Feb 13, 2017

@bhagn yes... we've already got this problem some time ago (#132) and now we are thinking of a new solution.

I believe that it will come soon and the next release will contain a new mechanism of the adapter injection!

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

3 participants