fix(angular.toJson): only strip properties beginning with $$, not $

[rodyhaddad]

This PR makes angular.toJson strip properties beginning with $$, instead of $

Rationale behind this:

2 years ago, angular switched from using it's own json serializer to the native JSON.stringify in 35125d2.
In the old serializer, angular was stripping the following properties: this, $parent and anything starting with $$

When we switched to the native JSON.stringify, this got changed to stripping anything starting with $

I don't know if this greedier stripping resulted from an internal discussion, but it seems to be causing problems to those who use properties beginning with $ in their model (see #1463)
So I reverted to having angular.toJson only strip props that start with $$

There's the issue that ngResource now defines the $promise and $resolved props on any resource instance: two properties that we don't want serialized and sent to the server on $save.
I worked around this by having resources utilize the toJSON method to not include both these properties on serialization.

I didn't go with the whitelist approach mentioned in #1463 because it would require a lookup in an array/hash for every property of an object being serialized, and it would be an app-wide config potentially requiring a lot of whitelisting. A better alternative would be a blacklist of [$$hashKey, $promise, $resolved], or what I'm doing here.

[mary-poppins]

Thanks for the PR! Please check the items below to help us merge this faster. See the contributing docs for more information.

• Uses the issue template (#6253)

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!

[caitp]

this is a feature, unrelated to the commit message

[rodyhaddad]

Got it. I'll break this PR down to 2 commits then

I'd have:

1. feat($resource): allow props beginning with $ to be used on resources
2. fix(angular.toJson): only strip properties beginning with $$, not $

Sounds good?

Proposing 2. without 1. would mean a PR that fails the tests

---

Done: I split the PR into 2 commits. I'm not 100% confident about my commit naming and ordering. If anyone has any issues with it, feel free to comment and I'll have some fun with git magic.

[btford]

The names of the commits seem fine. The ordering doesn't particularly matter, but I'd probably put the fix first.

[rodyhaddad]

@btford oops, my rebasing removed your comment?

btford: I would add a test showing that $-prefixed properties should be serialized as well.

---

I addressed your comments: added a test for $- and switched the commits

Let me know if you want me to change anything else, or if this strategy isn't the way we want to go to solve the problem.

---

Edit: worth clarifying I believe: the tests that are failing in travis are failing without my commits, so this PR has nothing to do with them.

[btford]

@rodyhaddad np, this is a known thing.

[quantizor]

+1 This makes a lot of sense. The developer can always do reassignments with ngInit in the event of a naming collision with a single $.

[btford]

Code-wise, this LGTM.

I might mark this as a breaking change in the commit message. Someone might be relying on the $-stripping behavior.

@IgorMinar thoughts? I know you favored a whitelist approach, but regardless I'd like to get this fix in soon, since it leads to surprising/confusing behavior for many developers and it's relatively low-hanging fruit.

[caitp]

+1 for shipping this version of the fix

[IgorMinar]

sounds good. can we mark this as a breaking change though so nobody is surprised?

[btford]

Agreed. I'll update the commit and merge this.

[btford]

@IgorMinar done.

Landed as c054288 and d3c50c8.

[sssilver]

This is still causing a problem, and is an ugly hack. Why not track exactly what properties are being added by Angular and then only remove those?

[caitp]

what problem is it causing for you?

[sssilver]

Same as #1463 was causing for other people. If I have my own properties in my own object prefixed with a '$$', I expect them to be there and stay there at all times, unless I remove them myself.

[sssilver]

And no, the solution isn't to add another dollar sign.

[caitp]

The $ prefix caused issues in cases where the object was used as a mongodb query object, for instance. Is there a similar issue for $$-prefixed properties? Because this is far and away the most performant way to keep private properties out. A blacklist has maintenance overhead, and is really just slower, so that kind of sucks :( I mean, it would be cool in an ideal world where we remembered to update the blacklist every time a private property was added or removed, but I'm not sure that's realistic :(

[sssilver]

I understand.

The proprietary service I work with uses double dollar sign for some parameters.

What if the $$-properties are all moved under a single special angularjs property, and namespaced that way?

[kavuri]

@btford would this fix be backported to 1.2.x? This is a fix I really require, but cannot upgrade my angular to 1.3.x yet.

[bitliner]

same of @kavuri , would be great to send keys including $ characters inside body request in the 1.2 too