encodeUriQuery differs in Angular.js and ngResource/resource.js & it's duplicated

[jrencz]

Please compare those 2 pieces of code:

angular.js@L1301

/**
 * This method is intended for encoding *key* or *value* parts of query component. We need a custom
 * method because encodeURIComponent is too aggressive and encodes stuff that doesn't have to be
 * encoded per http://tools.ietf.org/html/rfc3986:
 *    query       = *( pchar / "/" / "?" )
 *    pchar         = unreserved / pct-encoded / sub-delims / ":" / "@"
 *    unreserved    = ALPHA / DIGIT / "-" / "." / "_" / "~"
 *    pct-encoded   = "%" HEXDIG HEXDIG
 *    sub-delims    = "!" / "$" / "&" / "'" / "(" / ")"
 *                     / "*" / "+" / "," / ";" / "="
 */
function encodeUriQuery(val, pctEncodeSpaces) {
  return encodeURIComponent(val).
    replace(/%40/gi, '@').
    replace(/%3A/gi, ':').
    replace(/%24/g, '$').
    replace(/%2C/gi, ',').
    replace(/%3B/gi, ';').
    replace(/%20/g, (pctEncodeSpaces ? '%20' : '+'));
}

and in ngResource/resource.js@L405

/**
 * This method is intended for encoding *key* or *value* parts of query component. We need a
 * custom method because encodeURIComponent is too aggressive and encodes stuff that doesn't
 * have to be encoded per http://tools.ietf.org/html/rfc3986:
 *    query       = *( pchar / "/" / "?" )
 *    pchar         = unreserved / pct-encoded / sub-delims / ":" / "@"
 *    unreserved    = ALPHA / DIGIT / "-" / "." / "_" / "~"
 *    pct-encoded   = "%" HEXDIG HEXDIG
 *    sub-delims    = "!" / "$" / "&" / "'" / "(" / ")"
 *                     / "*" / "+" / "," / ";" / "="
 */
function encodeUriQuery(val, pctEncodeSpaces) {
  return encodeURIComponent(val).
    replace(/%40/gi, '@').
    replace(/%3A/gi, ':').
    replace(/%24/g, '$').
    replace(/%2C/gi, ',').
    replace(/%20/g, (pctEncodeSpaces ? '%20' : '+'));
}

Please notice the replace(/%3B/gi, ';'). which is present in Angular.js but it's not in ngResource/resource.js

angular.js/src/Angular.js

Line 1307 in 33f7f26

replace(/%3B/gi, ';').

The line was introduced in #8397 and released in 1.3.0-beta.18.

Apparently the ngResource code has to be updated as well.
Though this is a trivial change to make I'm not filing a PR yet. I'd like to discuss the need to DRY this a bit: even though it's fixed in ngResource those functions stay susceptible to inconsistency.

[lgalfaso]

Given that ngResources is not part of the core and given that it does not make any sense for Angular to expose the method encodeUriQuery, then I think that just a comment that specifies that these two methods need to be kept in sync, should be enough.