ngRouter: Multiple controller initialization when search changed #16632

fifonik · 2018-07-18T05:23:54Z

I'm submitting a ...

bug report
feature request
other

Current behavior:

Controller is initializing multiple times while search changed with ngRouter.

Expected / new behavior:

Single search change should only trigger controller initialization once.

Minimal reproduction of the problem with instructions:

index.htm

<!DOCTYPE html>
<html lang="en" ng-app="app">
<head>
	<meta charset="utf-8" />
	<meta http-equiv="X-UA-Compatible" content="IE=edge" />
	<meta name="viewport" content="width=device-width, initial-scale=1" />
	<base href="/" />
	<title>ngRoute + links with #</title>
	<style>
		ul, p   { font-size: 200%;  }
		h2      { margin-bottom: 0; }
		ul, pre { margin-top: 0;    }
	</style>

	<script src="/angular/1.6.9/angular.min.js"></script>
	<script src="/angular/1.6.9/angular-route.min.js"></script>
	<script src="app.js"></script>
</head>

<body>
	<p>Click on <b>main/#a1</b> and <b>main/#a2</b> links a few times and notice that every time when address is changing 'Ctrl.init' record added to the log once.
	<br />Then click on <b>main/</b> or <b>main/#</b> link once and notice that the single click just added 3 records to the log (rouring occured 3 times and every time the controller is initialized).
	<br />There are no such issue with AngularJS 1.6.1 and below: every click to the link initializes the controller only once.</p>

	<h2>Links</h2>
	<ul>
		<li><a href="main/">main/</a></li>
		<li><a href="main/#">main/#</a></li>
		<li><a href="main/#a1">main/#a1</a></li>
		<li><a href="main/#a2">main/#a2</a></li>
	</ul>

	<h2>View</h2>
	<section id="view" ng-view=""></section>

	<h2>Log <button type="button" ng-click="$root.log.length = 0">Clear log</button></h2>
	<pre class="log" ng-bind="$root.log|json"></pre>
</body>
</html>

app.js

var log = [];

angular
	.module('app', ['ngRoute'])
	.config(Config)
	.controller('Ctrl', Ctrl)
	.run(Run)
;

Run.$inject = ['$rootScope', '$location'];
function Run($rootScope, $location){
	log.push('Run');
	$rootScope.log = log;
}

Config.$inject = ['$locationProvider', '$routeProvider'];
function Config($locationProvider, $routeProvider){
	log.push('Config');
	$locationProvider.html5Mode(true);
	$routeProvider
		.when('/main/', {
			  template: '{{ctrl.url}}'
			, controller: 'Ctrl as ctrl'
		})
		.otherwise({
			redirectTo: '/main/'
		})
	;
}

Ctrl.$inject = ['$location'];
function Ctrl($location){
	log.push('Ctrl.init');
	var vm = this;
	vm.url  = $location.url();
}

http://plnkr.co/ZjIq1gb6hKgqPrNjsKZ0

AngularJS version: 1.6.2 and above (checked with 1.6.2, 1.6.9, 1.7.1, 1.7.2). The last version that is not affected -- 1.6.1

Browser: [all]

The text was updated successfully, but these errors were encountered:

gkalpak · 2018-07-18T11:50:26Z

I think it is b607618 😇 Interesting that nobody complained for almost 1.5 years. This is clearly broken 😁

…pty hash Fixes angular#16632

gkalpak · 2018-07-19T16:40:14Z

Should be fixed by #16636. In the meantime, you can hack around the issue like this:

myApp.decorator('$browser', $delegate => {
  const originalUrl = $delegate.url;
  $delegate.url = (...args) => {
    const result = originalUrl.apply($delegate, args);
    return angular.isString(result) ? result.replace(/#$/, '') : result;
  };
  return $delegate;
});

_{DISCLAIMER: This is a total hack, relies on private APIs and is not recommended. Use at your own risk 😁}

fifonik · 2018-07-19T22:20:53Z

Many thanks for prompt fix. Have not expected this.
I should have reported it half a year ago when first discovered %-)

gkalpak · 2018-07-20T13:21:04Z

You definitely should have 😠
I usually feel the urge to fix things that I have broken myself 😁

This one was quite serious (combined with routing and non-trivial route initialization logic).
Thx for reporting it.

…pty hash Fixes angular#16632

…pty hash Fixes #16632 Closes #16636

gkalpak added type: bug severity: regression component: $location labels Jul 18, 2018

gkalpak added this to the 1.7.x milestone Jul 18, 2018

gkalpak self-assigned this Jul 18, 2018

gkalpak added a commit to gkalpak/angular.js that referenced this issue Jul 19, 2018

fix($location): avoid unnecessary $locationChange* events due to em…

198c90c

…pty hash Fixes angular#16632

gkalpak mentioned this issue Jul 19, 2018

fix($location): avoid unnecessary $locationChange* events due to empty hash #16636

Closed

3 tasks

gkalpak added a commit to gkalpak/angular.js that referenced this issue Jul 25, 2018

fix($location): avoid unnecessary $locationChange* events due to em…

e6fa24f

…pty hash Fixes angular#16632

gkalpak added a commit to gkalpak/angular.js that referenced this issue Jul 26, 2018

fix($location): avoid unnecessary $locationChange* events due to em…

b085893

…pty hash Fixes angular#16632

gkalpak closed this as completed in 864c7f0 Jul 30, 2018

gkalpak added a commit that referenced this issue Jul 30, 2018

fix($location): avoid unnecessary $locationChange* events due to em…

1144b1e

…pty hash Fixes #16632 Closes #16636

dependencies bot mentioned this issue Aug 9, 2018

Update 6 dependencies from npm JetBrains/ring-ui#349

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

ngRouter: Multiple controller initialization when search changed #16632

ngRouter: Multiple controller initialization when search changed #16632

fifonik commented Jul 18, 2018 •

edited

Loading

gkalpak commented Jul 18, 2018 •

edited

Loading

gkalpak commented Jul 19, 2018 •

edited

Loading

fifonik commented Jul 19, 2018

gkalpak commented Jul 20, 2018

ngRouter: Multiple controller initialization when search changed #16632

ngRouter: Multiple controller initialization when search changed #16632

Comments

fifonik commented Jul 18, 2018 • edited Loading

gkalpak commented Jul 18, 2018 • edited Loading

gkalpak commented Jul 19, 2018 • edited Loading

fifonik commented Jul 19, 2018

gkalpak commented Jul 20, 2018

fifonik commented Jul 18, 2018 •

edited

Loading

gkalpak commented Jul 18, 2018 •

edited

Loading

gkalpak commented Jul 19, 2018 •

edited

Loading