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

fix(copy): angular copy should throw exception when source and destin… #15462

Closed
wants to merge 1 commit into from

Conversation

m-amr
Copy link
Contributor

@m-amr m-amr commented Dec 1, 2016

What kind of change does this PR introduce? (Bug fix, feature, docs update, ...)

bug

What is the current behavior? (You can also link to an open issue here)
#15444

What is the new behavior (if this is a feature change)?
when angular copy source and destination are not the same type, it throw an exception

Does this PR introduce a breaking change?

Please check if the PR fulfills these requirements

Other information:

…ation are not the same type

Closes #15444

@m-amr
Copy link
Contributor Author

m-amr commented Dec 1, 2016

This will require removing some test cases like

 it('should clear destination arrays correctly when source is non-array', function() {
  expect(copy(null, [1,2,3])).toEqual([]);
  expect(copy(undefined, [1,2,3])).toEqual([]);
  expect(copy({0: 1, 1: 2}, [1,2,3])).toEqual([1,2]);
  expect(copy(new Date(), [1,2,3])).toEqual([]);
  expect(copy(/a/, [1,2,3])).toEqual([]);
  expect(copy(true, [1,2,3])).toEqual([]);
});

it('should clear destination objects correctly when source is non-array', function() {
  expect(copy(null, {0:1,1:2,2:3})).toEqual({});
  expect(copy(undefined, {0:1,1:2,2:3})).toEqual({});
  expect(copy(new Date(), {0:1,1:2,2:3})).toEqual({});
  expect(copy(/a/, {0:1,1:2,2:3})).toEqual({});
  expect(copy(true, {0:1,1:2,2:3})).toEqual({});
});

Since that these cases will never happened because the source and destination must be the same type.
Is there is a reason to keep these cases ? they are useless now.

@gkalpak
Copy link
Member

gkalpak commented Dec 2, 2016

I am putting this in the Ice Box, as I don't think this change is appropriate (see #15444 (comment)).
I could get cnvinced otherwise thougg (if there is a compelling usecase).

Narretz added a commit to Narretz/angular.js that referenced this pull request Jan 14, 2019
Narretz added a commit to Narretz/angular.js that referenced this pull request Jan 14, 2019
Narretz added a commit to Narretz/angular.js that referenced this pull request Jan 21, 2019
@Narretz Narretz closed this in 2d8b346 Jan 21, 2019
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.

angular copy should throw exception when source and destination are not the same type.
3 participants