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

fix(merge): regExp should not be treated as a objects when merging. #12419

Closed
wants to merge 1 commit into from
Closed

Conversation

sreeramu
Copy link
Contributor

While merging two objects RegExp should not be treated same as the object.

Closes : #12409


expect(dst.regexp).not.toBe(src.regexp);
expect(isRegExp(dst.regexp)).toBeTruthy();
expect(dst.regexp.toString() === src.regexp.toString()).toBeTruthy();
Copy link
Contributor

Choose a reason for hiding this comment

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

I think we just want toBeTrue here. Looks ok to me

Copy link
Contributor

Choose a reason for hiding this comment

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

Or toBe(otherString)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks @caitp, i had updated with expect(dst.regexp.toString()).toBe(src.regexp.toString());

merge(dst, src);

expect(dst.regexp).not.toBe(src.regexp);
expect(isRegExp(dst.regexp)).toBeTruthy();
Copy link
Contributor

Choose a reason for hiding this comment

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

expect(isRegExp(dst.regexp)).toBeTrue();

@caitp
Copy link
Contributor

caitp commented Jul 24, 2015

i'll fix it myself, lgtm

@caitp caitp closed this in a5221f3 Jul 24, 2015
@caitp
Copy link
Contributor

caitp commented Jul 24, 2015

Thanks!

@sreeramu
Copy link
Contributor Author

Thanks @caitp

@sreeramu sreeramu deleted the patch-4 branch July 25, 2015 01:05
netman92 pushed a commit to netman92/angular.js that referenced this pull request Aug 8, 2015
angular.merge({ key: /regexp/ }) now works the way you'd expect it to.

Horray!

Closes angular#12419
Closes angular#12409
ggershoni pushed a commit to ggershoni/angular.js that referenced this pull request Sep 29, 2015
angular.merge({ key: /regexp/ }) now works the way you'd expect it to.

Horray!

Closes angular#12419
Closes angular#12409
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants