Skip to content
This repository has been archived by the owner on Apr 12, 2024. It is now read-only.

$routeProvider deep-copies its route objects before using them, which is probably not necessary #14478

Closed
rjamet opened this issue Apr 20, 2016 · 3 comments

Comments

@rjamet
Copy link
Contributor

rjamet commented Apr 20, 2016

Do you want to request a feature or report a bug?
Bug, I guess ?

What is the current behavior?
$routeProvider deep-copies its route object in the beginning of the when('...', route) call, which is a heavy way of solving #8181 and #9731.

What is the expected behavior?
Handle routes without needing a deep-copy.

What is the motivation / use case for changing the behavior?
I'm trying to plumb the Closure goog.html types in Angular as an alternative to the $sce types. These rely on a private marker that is passed to each instance by builders to ensure other classes can't fake them, and the deep copy breaks that link when passing an instance of that type in templateUrl.

Which versions of Angular, and which browser / OS are affected by this issue? Did this work in previous versions of Angular? Please also test with the latest stable and snapshot (https://code.angularjs.org/snapshot/) versions.
Recent ones ? That was initially fixed in b477058 .

Other information (e.g. stacktraces, related issues, suggestions how to fix)
I think this issue can be worked around by replacing templateUrl : safeTypeInstance with templateUrl : function(){return safeTypeInstance;}, but that's ugly. Martin Probst asked us to write an issue, saying "deep copying the entire object seems overkill if all you wanted was to (shallow) copy the fields in the angular.extend call.".

@mprobst
Copy link
Contributor

mprobst commented Apr 20, 2016

I guess a repro would be

var obj = {};
$routeProvider.when('foo/bar', {
  someProp: obj
});
// route change somehow, then:
expect($route.current.someProp_.toBe(obj);

@Narretz
Copy link
Contributor

Narretz commented Apr 21, 2016

Thanks for the report. The issue seems pretty minor, but if there's an easy solution that fixes it, we can merge it in. Do you want to open a PR for this?

@Narretz Narretz added this to the Backlog milestone Apr 21, 2016
rjamet added a commit to rjamet/angular.js that referenced this issue May 31, 2016
This seems to fix angular#14478. No need for a deep copy, just a plain one
is enough as ngRoute does not modify the fields in the route object.
gkalpak added a commit that referenced this issue Jun 10, 2016
Deep-copying route definition objects can break specific custom implementations of `$sce` (used to
trust a `templateUrl` as RESOURCE_URL). The purpose of copying route definition objects was to guard
against the user's modifying the route definition object after route registration, while still
capturing inherited properties.
As suggested by @IgorMinar in #14699 (comment),
we can achieve both _and_ support custom `$sce` implementations, by shallow-copying instead.

This is an alternative implementation for #14699, which avoids the breaking change.

Fixes #14478
Closes #14699

Closes #14750
@rjamet
Copy link
Contributor Author

rjamet commented Jun 10, 2016

Cool, thanks :) I was a little out of my depth on this one.

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