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

Initial route update doesn't happen if ngView in a template loaded by ngInclude #1213

Closed
jswright opened this Issue Jul 31, 2012 · 26 comments

Comments

@jswright
Copy link

jswright commented Jul 31, 2012

I can't seem to reproduce this on jsFiddle (mhevery says it's because you need real async with delay to reproduce, but the only way I know to include templates for ngInclude in jsFiddle are inline script tags), but here's how to set it up with two local files:

index.html

<html>
  <head>
    <script src="http://docs.angularjs.org/angular-1.0.1.min.js"></script>
    <script type="text/javascript">
      angular.module('myApp', [], function($routeProvider) {
        $routeProvider.when('/topic', {
          controller: TopicsCtrl,
          template: 'Controller: {{name}}'
        });
        $routeProvider.when('/topic/:topicId', {
          controller: TopicCtrl,
          template: 'Controller: {{name}}<br />Topic ID: {{params.topicId}}'
        });
        $routeProvider.otherwise({redirectTo: '/topic'});
      });

      function TopicsCtrl($scope) {
        $scope.name = 'TopicsCtrl';
      };

      function TopicCtrl($scope) {
        $scope.name = 'TopicCtrl';
        $scope.params = $routeParams;
      };
    </script>
  </head>
  <body ng-app="myApp">
    <div ng-include src="'app.html'"></div>
  </body>
</html>

app.html

<a href="#/topic">All topics</a>
<a href="#/topic/1">Topic 1</a>
<a href="#/topic/2">Topic 2</a>
<a href="#/topic/3">Topic 3</a>
<hr />
<div ng-view></div>

Expected results:

Page loads, redirects to #/topic, and shows 'Controller: TopicCtrl' in the view.

Actual results:

Page loads, does not redirect. If you click any link, the proper view is loaded. However, if you then reload, the view will not be properly loaded.

If you move the ng-view into index.html it works fine.

@jswright

This comment has been minimized.

Copy link
Author

jswright commented Jul 31, 2012

Miško writes:

if ng-view instantiation is delayed (through ng-include) then the $route instantiation is delayed as well. This means that $route will miss the location change event.

Fix:
Run $route update function on instantiation (not just on the location change event)

@saidimu

This comment has been minimized.

Copy link

saidimu commented Oct 4, 2012

After a bunch of false starts, here's how I did it (using the proposed fix):

If this is my app definition:

var myApp =  angular.module( ... );

This is where I update the $route update function:

myApp.run(['$route', function($route)  {
  $route.reload();
}]);

Now ng-view can be reliably nested inside ng-include.

myApp.run() is the place for application initialization: http://docs.angularjs.org/api/angular.Module

@fredrikbonander

This comment has been minimized.

Copy link
Contributor

fredrikbonander commented Mar 12, 2013

+1

Thanks for the fix.

@btford btford closed this Aug 24, 2013

@btford

This comment has been minimized.

Copy link
Contributor

btford commented Aug 24, 2013

As part of our effort to clean out old issues, this issue is being automatically closed since it has been inactivite for over two months.

Please try the newest versions of Angular (1.0.8 and 1.2.0-rc.1), and if the issue persists, comment below so we can discuss it.

Thanks!

@tobyn

This comment has been minimized.

Copy link

tobyn commented Sep 6, 2013

This issue still exists, but saidimu's fix still works. You don't have to call $route.reload() though, including it as a dependency to a run function is enough. I wound up with this:

myApp.run(['$route', angular.noop]);
@cmkarlsson

This comment has been minimized.

Copy link

cmkarlsson commented Nov 5, 2013

This is still an issue in 1.2.0-rc.3

The workarounds mentioned above work but if you inject $route into a run block all unit tests depending on the main module and using $scope.apply() fails (the app is doing $http calls to get the template when the route reloads or something). Don't know if that is a separate issue or not but certainly related to the workaround.

@raitucarp

This comment has been minimized.

Copy link

raitucarp commented Feb 24, 2014

Bump..

this issue is still there, it is call controller twice when I inject $route to app.run. Why?

@morgan-germain

This comment has been minimized.

Copy link

morgan-germain commented May 5, 2014

does anyone know if this annoying bug will be fixed in 1.3 ?

@watat83

This comment has been minimized.

Copy link

watat83 commented Jul 2, 2014

hello I am trying to load a template located on a seperate file, but when I look at the console, the template doesnt load up. Can someone help me here?
Thanks

@Narretz Narretz added this to the Backlog milestone Jul 2, 2014

@Narretz

This comment has been minimized.

Copy link
Contributor

Narretz commented Jul 2, 2014

I just checked it locally, and this still happens in the latest snapshot. Reopen.

@Narretz Narretz reopened this Jul 2, 2014

@rozza2058

This comment has been minimized.

Copy link

rozza2058 commented Jul 5, 2014

I am also experiencing this issue (in 1.2.18), but haven't seen it in other projects where I'm using a different (I think older) version - is it fixed in some versions?

@zackarychapple

This comment has been minimized.

Copy link

zackarychapple commented Jul 29, 2014

Had to add $route.reload() again with 1.3.0-beta.17

@Foxandxss

This comment has been minimized.

Copy link
Member

Foxandxss commented Sep 13, 2014

Still happen on rc.0

@caitp

This comment has been minimized.

Copy link
Contributor

caitp commented Sep 13, 2014

So, we tried this a while ago --- the thing is it was reverted: 3085987

Basically the issue is, ngView will instantiate a route, but won't update immediately --- the ngView's template content is empty until the route updates with a template.

It does work if the route update happens, however (http://plnkr.co/edit/MoqyggpCnCoSAAq97359?p=preview)

So, the reason this fix was reverted was, it means we end up emitting events which were maybe not expected to be emitted, or something. I don't have the details on the specific failures that caused the revert, but we might be able to re-land it, because that's essentially the only thing that breaks this.

edit

This pull request broke some of our code due to a change in the ordering of the onLocationSuccess event and the route resolve logic. In the past, onLocationSuccess was always fired before the route's resolve method was called. As of this change, onLocationSuccess fired after the resolve method when navigating to a new page (though no change was observed on initial page load).

@luckylooke

This comment has been minimized.

Copy link
Contributor

luckylooke commented Jan 10, 2015

ng 1.3.8, reload fix worked for me :)

@wcjr

This comment has been minimized.

Copy link

wcjr commented Mar 20, 2015

I believe this is also requiring me to do some silly stuff in my unit tests as a workaround:

http://stackoverflow.com/q/29170970/1259275

@dvdknaap

This comment has been minimized.

Copy link

dvdknaap commented May 15, 2015

This bug still exists in 1.3.15

by adding the following code it's fixed
app.run(['$route', function($route) {
$route.reload();
}]);

@blackbeltdev

This comment has been minimized.

Copy link

blackbeltdev commented Jun 18, 2015

The bug still exists in 1.4.1 as well. We have deeply nested ng-include that has a 'page' directive which contains ng-view in the template.

It seems just the act of injecting $route into the run block does the trick for us. Someone else pointed that out earlier in thread, "tobyn commented on Sep 6, 2013" but the noop isn't necessary just the injection.

// doesn't work!
app.run(['$log', '$rootScope', function ($log, $rootScope) { 
  // nothing
}
// works!
app.run(['$log', '$rootScope', '$route', function ($log, $rootScope, $route) { 
  // nothing
}
@mstroming

This comment has been minimized.

Copy link

mstroming commented Jun 18, 2015

+1 to blackbeltdev's comment.

@blackbeltdev

This comment has been minimized.

Copy link

blackbeltdev commented Jun 19, 2015

There's an interesting sentence in here which might explain why injecting the $route fixes the problem with ng-include.

http://www.bennadel.com/blog/2770-route-must-be-injected-in-order-to-enable-the-routechangesuccess-event-in-angularjs.htm

"Instead, I have another, optional, sub-controller which does nothing but inject the $route service (which forces AngularJS to call the $routeProvider)."

@erikcode

This comment has been minimized.

Copy link

erikcode commented Feb 18, 2016

1.5.0 has the same behavior. Requires you to inject $route into run(..)

gkalpak added a commit to gkalpak/angular.js that referenced this issue Feb 19, 2016

fix(ngRoute): allow `ngView` to be included in an asynchronously load…
…ed template

During it's linking phase, `ngView` relies on the info provided in `$route.current` for
instantiating the initial view. `$route.current` is set in the callback of a listener to
`$locationChangeSuccess`, which is registered during the instantiation of the `$route` service.

Thus, it is crucial that the `$route` service is instantiated before the initial
`$locationChangeSuccess` is fired. Since `ngView` declares `$route` as a dependency, the service is
instantiated in time if `ngView` is present during the initial load of the page.

Yet, in cases where `ngView` is included in a template that is loaded asynchronously (e.g. in
another directive's template), the directive factory might not be called soon enough for `$route`
to be instantiated before the initial `$locationChangeSuccess` event is fired.

This commit fixes it, by always instantiating `$route` up front, during the initialization phase.

Fixes angular#1213
Fixes angular#6812

gkalpak added a commit to gkalpak/angular.js that referenced this issue Feb 19, 2016

fix(ngRoute): allow `ngView` to be included in an asynchronously load…
…ed template

During it's linking phase, `ngView` relies on the info provided in `$route.current` for
instantiating the initial view. `$route.current` is set in the callback of a listener to
`$locationChangeSuccess`, which is registered during the instantiation of the `$route` service.

Thus, it is crucial that the `$route` service is instantiated before the initial
`$locationChangeSuccess` is fired. Since `ngView` declares `$route` as a dependency, the service is
instantiated in time if `ngView` is present during the initial load of the page.

Yet, in cases where `ngView` is included in a template that is loaded asynchronously (e.g. in
another directive's template), the directive factory might not be called soon enough for `$route`
to be instantiated before the initial `$locationChangeSuccess` event is fired.

This commit fixes it, by always instantiating `$route` up front, during the initialization phase.

Fixes angular#1213
Fixes angular#6812

gkalpak added a commit that referenced this issue Feb 25, 2016

fix(ngRoute): allow `ngView` to be included in an asynchronously load…
…ed template

During it's linking phase, `ngView` relies on the info provided in `$route.current` for
instantiating the initial view. `$route.current` is set in the callback of a listener to
`$locationChangeSuccess`, which is registered during the instantiation of the `$route` service.

Thus, it is crucial that the `$route` service is instantiated before the initial
`$locationChangeSuccess` is fired. Since `ngView` declares `$route` as a dependency, the service is
instantiated in time if `ngView` is present during the initial load of the page.

Yet, in cases where `ngView` is included in a template that is loaded asynchronously (e.g. in
another directive's template), the directive factory might not be called soon enough for `$route`
to be instantiated before the initial `$locationChangeSuccess` event is fired.

This commit fixes it, by always instantiating `$route` up front, during the initialization phase.

Fixes #1213
Fixes #6812

Closes #14088

@gkalpak gkalpak closed this in 5e37b2a Feb 25, 2016

gkalpak added a commit that referenced this issue Feb 25, 2016

fix(ngRoute): allow `ngView` to be included in an asynchronously load…
…ed template

During it's linking phase, `ngView` relies on the info provided in `$route.current` for
instantiating the initial view. `$route.current` is set in the callback of a listener to
`$locationChangeSuccess`, which is registered during the instantiation of the `$route` service.

Thus, it is crucial that the `$route` service is instantiated before the initial
`$locationChangeSuccess` is fired. Since `ngView` declares `$route` as a dependency, the service is
instantiated in time if `ngView` is present during the initial load of the page.

Yet, in cases where `ngView` is included in a template that is loaded asynchronously (e.g. in
another directive's template), the directive factory might not be called soon enough for `$route`
to be instantiated before the initial `$locationChangeSuccess` event is fired.

This commit fixes it, by always instantiating `$route` up front, during the initialization phase.

Fixes #1213
Fixes #6812

Closes #14088
@davidsandoz

This comment has been minimized.

Copy link

davidsandoz commented Apr 21, 2016

The fix was reverted in 75bf807

Then I suppose the issue should be re-opened.

@Narretz

This comment has been minimized.

Copy link
Contributor

Narretz commented Apr 21, 2016

It will be re-applied for v1.6.x, with a breaking change notice and possibly a way to disable
the feature is tests.

We just have to make sure the BC notice is added to the CL when the time comes.

@goddabuzz

This comment has been minimized.

Copy link

goddabuzz commented May 2, 2016

+1 to blackbeltdev's comment. =) Works for 1.5.x

@gkalpak gkalpak reopened this Jul 10, 2016

gkalpak added a commit to gkalpak/angular.js that referenced this issue Jul 11, 2016

feat(ngRoute): allow `ngView` to be included in an asynchronously loa…
…ded template

During its linking phase, `ngView` relies on the info provided in `$route.current` for
instantiating the initial view. `$route.current` is set in the callback of a listener to
`$locationChangeSuccess`, which is registered during the instantiation of the `$route` service.

Thus, it is crucial that the `$route` service is instantiated _before_ the initial
`$locationChangeSuccess` event is fired. Since `ngView` declares `$route` as a dependency, the
service is instantiated in time, if `ngView` is present during the initial load of the page.

Yet, in cases where `ngView` is included in a template that is loaded asynchronously (e.g. in
another directive's template), the directive factory might not be called soon enough for `$route`
to be instantiated before the initial `$locationChangeSuccess` event is fired.

This commit fixes it, by enabling eager instantiation of `$route` (during the initialization phase).
Eager instantiation can be disabled (restoring the old behavior), but is on by default.

Fixes angular#1213

BREAKING CHANGE:

In cases where `ngView` was loaded asynchronously, `$route` (and its dependencies; e.g. `$location`)
might also have been instantiated asynchronously. After this change, `$route` (and its dependencies)
will - by default - be instantiated early on.

Although this is not expected to have unwanted side-effects in normal application bebavior, it may
affect your unit tests: When testing a module that (directly or indirectly) depends on `ngRoute`, a
request will be made for the default route's template. If not properly "trained", `$httpBackend`
will complain about this unexpected request.

You can restore the previous behavior (and avoid unexpected requests in tests), by using
`$routeProvider.eagerInstantiationEnabled(false)`.

@gkalpak gkalpak closed this in c13c666 Jul 18, 2016

@HaroldStruwig

This comment has been minimized.

Copy link

HaroldStruwig commented Sep 12, 2016

this is still not fixed in 1.5.8

@gkalpak

This comment has been minimized.

Copy link
Member

gkalpak commented Sep 12, 2016

@HaroldStruwig, this was fixed on master with #14893 (and will be included in the upcoming v1.6.0 release. The fix can't be backported as-is, due to a slight breaking change.

Although we could backport it, but change the default to to non-eager $route instantiation (to avoid the breaking change), it isn't worth it. Manually fixing this is easier than opting-into eager-instantiation. E.g.:

// IF we had backported the fix (which we won't)
app.config(['$routeProvider', function ($routeProvider) {
  $routeProvider.eagerInstantiationEnabled(true);
}]);

// vs

// Manually fixing it in v1.5.x (much sorter and recommended)
app.run(['$route', angular.noop]);

The work-around/fix for v1.5.x is even document here.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.