Controller was executed twice when navigated through ngLink #204

Open
shaunxu opened this Issue Mar 23, 2015 · 14 comments

Comments

Projects
None yet
@shaunxu

shaunxu commented Mar 23, 2015

I found when I clicked a link generated by ngLink, the targeting controller was being invoked twice. This is not happened when I visit that page directly (type URL or refresh the page), just when clicking the ngLink.

I just created a small sample to reproduce this issue at https://github.com/shaunxu/ng-new-router-test/ Just clone, npm install and then run node --port [the port number you like] --debug

Don't know if it's because of something I did wrong.

@skusunam

This comment has been minimized.

Show comment
Hide comment
@skusunam

skusunam Mar 23, 2015

Contributor

Is this duplicate of #200?

Contributor

skusunam commented Mar 23, 2015

Is this duplicate of #200?

@shaunxu

This comment has been minimized.

Show comment
Hide comment
@shaunxu

shaunxu Mar 23, 2015

@skusunam Seems not. In my case I only have one ng-viewport in my index.html.

shaunxu commented Mar 23, 2015

@skusunam Seems not. In my case I only have one ng-viewport in my index.html.

@skusunam

This comment has been minimized.

Show comment
Hide comment
@skusunam

skusunam Mar 23, 2015

Contributor

@shaunxu thanks for clarifying. If you can provide a plunker example (or) running gh-pages it will help to run it and see the problem right away rather than cloning your repo and going through whole 9 yards. Also noticed that you did not even check-in your package.json in your test repo :(.

Contributor

skusunam commented Mar 23, 2015

@shaunxu thanks for clarifying. If you can provide a plunker example (or) running gh-pages it will help to run it and see the problem right away rather than cloning your repo and going through whole 9 yards. Also noticed that you did not even check-in your package.json in your test repo :(.

@shaunxu

This comment has been minimized.

Show comment
Hide comment
@shaunxu

shaunxu Mar 24, 2015

@skusunam Sure. Just noticed plunker support folders so I will copy my example.

shaunxu commented Mar 24, 2015

@skusunam Sure. Just noticed plunker support folders so I will copy my example.

@shaunxu

This comment has been minimized.

Show comment
Hide comment
@shaunxu

shaunxu Mar 24, 2015

@skusunam Just created a plunker http://plnkr.co/edit/FAageDG6OlU12Vs26DN2?p=preview Please open browser console, it shows that controller was executed twice when clicked a link.

shaunxu commented Mar 24, 2015

@skusunam Just created a plunker http://plnkr.co/edit/FAageDG6OlU12Vs26DN2?p=preview Please open browser console, it shows that controller was executed twice when clicked a link.

@skusunam

This comment has been minimized.

Show comment
Hide comment
@skusunam

skusunam Mar 25, 2015

Contributor

@shahata I can clearly see this problem with your pluker and also from "examples/angular-1/hello/". @btford once you confirm this as "bug" may be i can dig in and see if i can do a PR on this. Any hints will help me in doing this first code PR :)

Contributor

skusunam commented Mar 25, 2015

@shahata I can clearly see this problem with your pluker and also from "examples/angular-1/hello/". @btford once you confirm this as "bug" may be i can dig in and see if i can do a PR on this. Any hints will help me in doing this first code PR :)

@robertdp

This comment has been minimized.

Show comment
Hide comment
@robertdp

robertdp Apr 9, 2015

It seems to be an issue in routerFactory().

https://github.com/angular/router/blob/master/src/router-directive.es5.js#L85

$$rootRouter.navigate() updates $location.path(), which triggers the $rootScope.$watch, which in turn calls $$rootRouter.navigate() again...

It doesn't just instantiate the controllers twice, it runs through the whole route activation pipeline twice.

robertdp commented Apr 9, 2015

It seems to be an issue in routerFactory().

https://github.com/angular/router/blob/master/src/router-directive.es5.js#L85

$$rootRouter.navigate() updates $location.path(), which triggers the $rootScope.$watch, which in turn calls $$rootRouter.navigate() again...

It doesn't just instantiate the controllers twice, it runs through the whole route activation pipeline twice.

@rohanrehman

This comment has been minimized.

Show comment
Hide comment
@rohanrehman

rohanrehman Apr 16, 2015

@btford @robertdp that was what I found too

@btford @robertdp that was what I found too

@basvandenheuvel

This comment has been minimized.

Show comment
Hide comment
@basvandenheuvel

basvandenheuvel Sep 10, 2015

Is there a solution yet?

Is there a solution yet?

@Jcamilorada

This comment has been minimized.

Show comment
Hide comment
@Jcamilorada

Jcamilorada Sep 12, 2015

is there any workaround?

is there any workaround?

@tomer78

This comment has been minimized.

Show comment
Hide comment
@tomer78

tomer78 Feb 4, 2016

I found a solution.
I don't use the ng-link.
I've used an ng-click and I redirect using the $location service.
this.$location.path('new url')

This way the controller instantiation happens only once!

tomer78 commented Feb 4, 2016

I found a solution.
I don't use the ng-link.
I've used an ng-click and I redirect using the $location service.
this.$location.path('new url')

This way the controller instantiation happens only once!

@wsfriday-sfd

This comment has been minimized.

Show comment
Hide comment
@wsfriday-sfd

wsfriday-sfd Feb 17, 2016

My solution was similar to @tomer78 , but I've modified ngLinkDirective and anchorLinkDirective directly to use $location.path() to change routes instead of using $router.navigate() and triggering this double controller issue

        function ngLinkDirective($router, $location, $parse) {
                     // we can avoid adding a watcher if it's a literal
                     if (routeParamsGetter.constant) {
                         var params = routeParamsGetter();
        -        url = '.' + router.generate(routeName, params);
        +        url = router.generate(routeName, params);
                         elt.attr('href', url);
                     } else {
                         scope.$watch(function() {
                             return routeParamsGetter(scope);
                         }, function(params) {
        -          url = '.' + router.generate(routeName, params);
        +          url = router.generate(routeName, params);
                             elt.attr('href', url);
                         }, true);
                     }
                 } else {
        -      url = '.' + router.generate(routeName);
        +      url = router.generate(routeName);
                     elt.attr('href', url);
                 }
        }
         ngLinkDirective.$inject = ["$router", "$location", "$parse"];


        -function anchorLinkDirective($router) {
        +function anchorLinkDirective($router, $location) {
             return {
                 restrict: 'E',
                 link: function(scope, element) {
                        function anchorLinkDirective($router) {
                             event.preventDefault();
                         }
                         if ($router.recognize(href)) {
        -          $router.navigate(href);
        +          $location.path(href);
                             event.preventDefault();
                         }
                     });
                 }
             }
         }
        -anchorLinkDirective.$inject = ["$router"];
        +anchorLinkDirective.$inject = ["$router", "$location"];

My solution was similar to @tomer78 , but I've modified ngLinkDirective and anchorLinkDirective directly to use $location.path() to change routes instead of using $router.navigate() and triggering this double controller issue

        function ngLinkDirective($router, $location, $parse) {
                     // we can avoid adding a watcher if it's a literal
                     if (routeParamsGetter.constant) {
                         var params = routeParamsGetter();
        -        url = '.' + router.generate(routeName, params);
        +        url = router.generate(routeName, params);
                         elt.attr('href', url);
                     } else {
                         scope.$watch(function() {
                             return routeParamsGetter(scope);
                         }, function(params) {
        -          url = '.' + router.generate(routeName, params);
        +          url = router.generate(routeName, params);
                             elt.attr('href', url);
                         }, true);
                     }
                 } else {
        -      url = '.' + router.generate(routeName);
        +      url = router.generate(routeName);
                     elt.attr('href', url);
                 }
        }
         ngLinkDirective.$inject = ["$router", "$location", "$parse"];


        -function anchorLinkDirective($router) {
        +function anchorLinkDirective($router, $location) {
             return {
                 restrict: 'E',
                 link: function(scope, element) {
                        function anchorLinkDirective($router) {
                             event.preventDefault();
                         }
                         if ($router.recognize(href)) {
        -          $router.navigate(href);
        +          $location.path(href);
                             event.preventDefault();
                         }
                     });
                 }
             }
         }
        -anchorLinkDirective.$inject = ["$router"];
        +anchorLinkDirective.$inject = ["$router", "$location"];
@dteunkenstt

This comment has been minimized.

Show comment
Hide comment
@dteunkenstt

dteunkenstt Mar 11, 2016

The solution @wsfriday-sfd proposed didn't quite do it for me.

The problem I observed was that the url first was e.g. './home' and then '/home' (note the missing dot in the second url)

I modified router.es5.js somewhere else. I didn't modify any lines, I just added a few.

(createClass)(Router, {
  ...
  navigate: function(url) {
      var $__0 = this;
      if (this.navigating) {
        return $q.when();
      }
      this.lastNavigationAttempt = url;
      var instruction = this.recognize(url);
      if (!instruction) {
        return $q.reject();
      }
+     if(this.lastInstructionCanonicalUrl === instruction.canonicalUrl) {
+       return $q.reject();  
+     }
+     this.lastInstructionCanonicalUrl = instruction.canonicalUrl;
      this._startNavigating();
      ...

The instruction.canonicalUrl is the path, without the '.' in front (the one @wsfriday-sfd seems to be removing).

The solution @wsfriday-sfd proposed didn't quite do it for me.

The problem I observed was that the url first was e.g. './home' and then '/home' (note the missing dot in the second url)

I modified router.es5.js somewhere else. I didn't modify any lines, I just added a few.

(createClass)(Router, {
  ...
  navigate: function(url) {
      var $__0 = this;
      if (this.navigating) {
        return $q.when();
      }
      this.lastNavigationAttempt = url;
      var instruction = this.recognize(url);
      if (!instruction) {
        return $q.reject();
      }
+     if(this.lastInstructionCanonicalUrl === instruction.canonicalUrl) {
+       return $q.reject();  
+     }
+     this.lastInstructionCanonicalUrl = instruction.canonicalUrl;
      this._startNavigating();
      ...

The instruction.canonicalUrl is the path, without the '.' in front (the one @wsfriday-sfd seems to be removing).

@smartworld-konoha

This comment has been minimized.

Show comment
Hide comment
@smartworld-konoha

smartworld-konoha Dec 21, 2017

I've got same issue, When using link the activeUser() is called twice, but when I use the direct address, it is called once.

I've got same issue, When using link the activeUser() is called twice, but when I use the direct address, it is called once.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment