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

chore($resource): Use shallow copy instead of angular.copy #5252

Closed
wants to merge 20 commits into from

Conversation

kseamon
Copy link
Contributor

@kseamon kseamon commented Dec 3, 2013

chore($resource): Use shallow copy instead of angular.copy

Replace calls to angular.copy with calls to a new function, shallowClearAndCopy.
Add calls to copy for cache access in $http in order to prevent modification of cached data.
Results in a measurable improvement to the startup time of complex apps within Google.

Replace calls to angular.copy with calls to a new function, shallowClearAndCopy.
Add calls to copy for cache access in $http in order to prevent modification of cached data.
Results in a measurable improvement to the startup time of complex apps within Google.
@mary-poppins
Copy link

Thanks for the PR!

  • Contributor signed CLA now or in the past
    • If you just signed, leave a comment here with your real name
  • PR's commit messages follow the commit message format

If you need to make changes to your pull request, you can update the commit with git commit --amend.
Then, update the pull request with git push -f.

Thanks again for your help!

@kseamon
Copy link
Contributor Author

kseamon commented Dec 3, 2013

Not sure if this change warrants any new unit tests.

Also, the $http change is a slight change in behavior for cases where it is not being used by way of $resource (Previously $resource was doing all of the copying).

I can revert the changes to $http if that's preferrable.

@IgorMinar
Copy link
Contributor

is this a breaking change in any way?

@@ -465,7 +484,7 @@ angular.module('ngResource', ['ng']).
if (data) {
// Need to convert action.isArray to boolean in case it is undefined
// jshint -W018
if ( angular.isArray(data) !== (!!action.isArray) ) {
if (angular.isArray(data) !== (!!action.isArray) ) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

also remove the trailing space while you are at it :)

@kseamon
Copy link
Contributor Author

kseamon commented Dec 3, 2013

The resource part should not be breaking.

The $http part would break code that depends on being able to modify the cache by way of reading from it.

@ghost ghost assigned IgorMinar Dec 5, 2013
@IgorMinar
Copy link
Contributor

this looks good to me (except for not being squashed into a single commit with a good commit message).

I believe that we can't merge this until #5276 lands, otherwise the $resource instance could modify the cache.

Daniel Tabuenca and others added 5 commits December 4, 2013 22:49
…propriate

Due to an earlier change, ngModelWatch() no longer returns a value to the
caller. This means the digest loop has no way to tell if the watch actually
modified anything and so can not schedule another pass.

This means any watches that watch form or model controller changes
(e.g. watches on form.$valid) that are scheduled prior to an ngModelWatch()
will not be able to see any changes made therin.

This commit fixes this behavior by returning the latest evaluated ng-model
value.

Closes angular#5258
Closes angular#5282
If jshint (or any other ci-check) fails, Travis marks the build as "Errored" which I don't think is desider:
https://travis-ci.org/angular/angular.js/builds/14938896
The priority of ngInit is adjusted to occur before ngInclude, and after
ngController. This enables ngInit to initiallize values in a controller's
scope, and also to initiallize values before ngInclude executes.

Closes angular#5167
Closes angular#5208
@kseamon
Copy link
Contributor Author

kseamon commented Dec 5, 2013

I closed #5276 as it actually does nothing. The data going in and out of the $http cache is always a string, so angular.copy has no effect.

petebacondarwin and others added 5 commits December 5, 2013 22:06
Replace calls to angular.copy with calls to a new function, shallowClearAndCopy.
Add calls to copy for cache access in $http in order to prevent modification of cached data.
Results in a measurable improvement to the startup time of complex apps within Google.
@kseamon kseamon closed this Dec 5, 2013
@kseamon kseamon deleted the shallow-copy branch December 5, 2013 23:40
@kseamon
Copy link
Contributor Author

kseamon commented Dec 5, 2013

I've managed to completely ruin this branch with rebase, so once I figure out what's wrong I'll get it re-updated.

@kseamon
Copy link
Contributor Author

kseamon commented Dec 5, 2013

Opened a new PR: #5300 with a cleaned up branch.

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

Successfully merging this pull request may close these issues.