Repository: auth0/auth0.js
Pull request #382 (2017-03-09 17:12:55)

Replace querystring implementation with qs module
__Rationale__: Writing a proper querystring parse implementation can be hard, so we should leverage existing modules to be more robust. Also there is an issue with the current implementation described in https://github.com/auth0/lock/issues/913.

__Scope__: Replaces all occurences of `helpers/qs.js` with the `qs` module

__Notes__
- I removed [this test](https://github.com/auth0/auth0.js/blob/master/test/helper/popup-handler.test.js#L83), because it was actually testing the querystring implementation
- I had to adapt [this test](https://github.com/auth0/auth0.js/blob/master/test/helper/iframe-handler.test.js#L160) as pluses in urls are decoded as spaces, the new expected value seems more correct to me, but correct me if i am wrong

luisrudge: Can you share the bundle size difference with this module as a dependency?
codecov-io: # [Codecov](https://codecov.io/gh/auth0/auth0.js/pull/382?src=pr&el=h1) Report
> Merging [#382](https://codecov.io/gh/auth0/auth0.js/pull/382?src=pr&el=desc) into [master](https://codecov.io/gh/auth0/auth0.js/commit/48dc331d576f60c1598708e3877390e8a8a680e1?src=pr&el=desc) will **decrease** coverage by `0.02%`.
> The diff coverage is `100%`.


```diff
@@            Coverage Diff             @@
##           master     #382      +/-   ##
==========================================
- Coverage   97.54%   97.51%   -0.03%     
==========================================
  Files          35       34       -1     
  Lines        1179     1166      -13     
  Branches      198      195       -3     
==========================================
- Hits         1150     1137      -13     
  Misses         29       29
```


| [Impacted Files](https://codecov.io/gh/auth0/auth0.js/pull/382?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [src/helper/popup-handler.js](https://codecov.io/gh/auth0/auth0.js/compare/48dc331d576f60c1598708e3877390e8a8a680e1...9d949ba24ca4d2e94d2cea2b8bb374afa499fd94?src=pr&el=tree#diff-c3JjL2hlbHBlci9wb3B1cC1oYW5kbGVyLmpz) | `100% <100%> (ø)` | :white_check_mark: |
| [src/authentication/passwordless-authentication.js](https://codecov.io/gh/auth0/auth0.js/compare/48dc331d576f60c1598708e3877390e8a8a680e1...9d949ba24ca4d2e94d2cea2b8bb374afa499fd94?src=pr&el=tree#diff-c3JjL2F1dGhlbnRpY2F0aW9uL3Bhc3N3b3JkbGVzcy1hdXRoZW50aWNhdGlvbi5qcw==) | `100% <100%> (ø)` | :white_check_mark: |
| [plugins/cordova/popup-handler.js](https://codecov.io/gh/auth0/auth0.js/compare/48dc331d576f60c1598708e3877390e8a8a680e1...9d949ba24ca4d2e94d2cea2b8bb374afa499fd94?src=pr&el=tree#diff-cGx1Z2lucy9jb3Jkb3ZhL3BvcHVwLWhhbmRsZXIuanM=) | `91.89% <100%> (ø)` | :white_check_mark: |
| [src/web-auth/index.js](https://codecov.io/gh/auth0/auth0.js/compare/48dc331d576f60c1598708e3877390e8a8a680e1...9d949ba24ca4d2e94d2cea2b8bb374afa499fd94?src=pr&el=tree#diff-c3JjL3dlYi1hdXRoL2luZGV4Lmpz) | `97.9% <100%> (ø)` | :white_check_mark: |
| [src/authentication/index.js](https://codecov.io/gh/auth0/auth0.js/compare/48dc331d576f60c1598708e3877390e8a8a680e1...9d949ba24ca4d2e94d2cea2b8bb374afa499fd94?src=pr&el=tree#diff-c3JjL2F1dGhlbnRpY2F0aW9uL2luZGV4Lmpz) | `100% <100%> (ø)` | :white_check_mark: |

------

[Continue to review full report at Codecov](https://codecov.io/gh/auth0/auth0.js/pull/382?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/auth0/auth0.js/pull/382?src=pr&el=footer). Last update [48dc331...9d949ba](https://codecov.io/gh/auth0/auth0.js/compare/48dc331d576f60c1598708e3877390e8a8a680e1...9d949ba24ca4d2e94d2cea2b8bb374afa499fd94?src=pr&el=footer&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).

Patch:
diff --git a/plugins/cordova/popup-handler.js b/plugins/cordova/popup-handler.js
index f347846..a43bd98 100644
--- a/plugins/cordova/popup-handler.js
+++ b/plugins/cordova/popup-handler.js
@@ -1,5 +1,5 @@
 var windowHandler = require('../../src/helper/window');
-var qs = require('../../src/helper/qs');
+var qs = require('qs');
 var urljoin = require('url-join');
 
 function PopupHandler(webAuth) {
@@ -19,7 +19,10 @@ PopupHandler.prototype.preload = function (options) {
   delete popupOptions.width;
   delete popupOptions.height;
 
-  var windowFeatures = qs.build(popupOptions, ',', false);
+  var windowFeatures = qs.stringify(popupOptions, {
+    encode: false,
+    delimiter: ','
+  });
 
   if (this._current_popup && !this._current_popup.closed) {
     return this._current_popup;
diff --git a/src/authentication/index.js b/src/authentication/index.js
index 507c55d..9abfb11 100644
--- a/src/authentication/index.js
+++ b/src/authentication/index.js
@@ -1,7 +1,7 @@
 var urljoin = require('url-join');
 
 var RequestBuilder = require('../helper/request-builder');
-var qs = require('../helper/qs');
+var qs = require('qs');
 var objectHelper = require('../helper/object');
 var assert = require('../helper/assert');
 var responseHandler = require('../helper/response-handler');
@@ -103,7 +103,7 @@ Authentication.prototype.buildAuthorizeUrl = function (options) {
   params = objectHelper.toSnakeCase(params, ['auth0Client']);
   params = parametersWhitelist.oauthAuthorizeParams(this.warn, params);
 
-  qString = qs.build(params);
+  qString = qs.stringify(params);
 
   return urljoin(this.baseOptions.rootUrl, 'authorize', '?' + qString);
 };
@@ -134,7 +134,7 @@ Authentication.prototype.buildLogoutUrl = function (options) {
 
   params = objectHelper.toSnakeCase(params, ['auth0Client', 'returnTo']);
 
-  qString = qs.build(params);
+  qString = qs.stringify(params);
 
   return urljoin(this.baseOptions.rootUrl, 'v2', 'logout', '?' + qString);
 };
@@ -297,7 +297,7 @@ Authentication.prototype.getSSOData = function (withActiveDirectories, cb) {
   assert.check(cb, { type: 'function', message: 'cb parameter is not valid' });
 
   if (withActiveDirectories) {
-    params = '?' + qs.build({
+    params = '?' + qs.stringify({
       ldaps: 1,
       client_id: this.baseOptions.clientID
     });
diff --git a/src/authentication/passwordless-authentication.js b/src/authentication/passwordless-authentication.js
index d88b83f..48362c7 100644
--- a/src/authentication/passwordless-authentication.js
+++ b/src/authentication/passwordless-authentication.js
@@ -2,7 +2,7 @@ var urljoin = require('url-join');
 
 var objectHelper = require('../helper/object');
 var assert = require('../helper/assert');
-var qs = require('../helper/qs');
+var qs = require('qs');
 var responseHandler = require('../helper/response-handler');
 
 function PasswordlessAuthentication(request, options) {
@@ -48,7 +48,7 @@ PasswordlessAuthentication.prototype.buildVerifyUrl = function (options) {
 
   params = objectHelper.toSnakeCase(params, ['auth0Client']);
 
-  qString = qs.build(params);
+  qString = qs.stringify(params);
 
   return urljoin(this.baseOptions.rootUrl, 'passwordless', 'verify_redirect', '?' + qString);
 };
diff --git a/src/helper/popup-handler.js b/src/helper/popup-handler.js
index 32c361f..0c96241 100644
--- a/src/helper/popup-handler.js
+++ b/src/helper/popup-handler.js
@@ -4,7 +4,7 @@ var WinChan = require('winchan');
 
 var windowHandler = require('./window');
 var objectHelper = require('./object');
-var qs = require('./qs');
+var qs = require('qs');
 
 function PopupHandler() {
   this._current_popup = null;
@@ -38,7 +38,10 @@ PopupHandler.prototype.preload = function (options) {
   var popupPosition = this.calculatePosition(options.popupOptions || {});
   var popupOptions = objectHelper.merge(popupPosition).with(options.popupOptions);
   var url = options.url || 'about:blank';
-  var windowFeatures = qs.build(popupOptions, ',', false);
+  var windowFeatures = qs.stringify(popupOptions, {
+    encode: false,
+    delimiter: ','
+  });
 
   if (this._current_popup && !this._current_popup.closed) {
     return this._current_popup;
@@ -62,7 +65,10 @@ PopupHandler.prototype.load = function (url, relayUrl, options, cb) {
   var winchanOptions = objectHelper.merge({
     url: url,
     relay_url: relayUrl,
-    window_features: qs.build(popupOptions, ',', false),
+    window_features: qs.stringify(popupOptions, {
+        delimiter: ',',
+        encode: false
+    }),
     popup: this._current_popup
   }).with(options);
 
diff --git a/src/helper/qs.js b/src/helper/qs.js
deleted file mode 100644
index dd3cef6..0000000
--- a/src/helper/qs.js
+++ /dev/null
@@ -1,24 +0,0 @@
-function build(params, glue, uriEncodedValues) {
-  glue = glue || '&';
-  uriEncodedValues = uriEncodedValues !== false;
-
-  return Object.keys(params).reduce(function (arr, key) {
-    if (typeof params[key] !== 'undefined') {
-      arr.push(key + '=' + (uriEncodedValues ? encodeURIComponent(params[key]) : params[key]));
-    }
-    return arr;
-  }, []).join(glue);
-}
-
-function parse(qs) {
-  return qs.split('&').reduce(function (prev, curr) {
-    var param = curr.split('=');
-    prev[param[0]] = param[1];
-    return prev;
-  }, {});
-}
-
-module.exports = {
-  build: build,
-  parse: parse
-};
diff --git a/src/web-auth/index.js b/src/web-auth/index.js
index 2c5e1b3..450344d 100644
--- a/src/web-auth/index.js
+++ b/src/web-auth/index.js
@@ -2,7 +2,7 @@ var IdTokenVerifier = require('idtoken-verifier');
 
 var assert = require('../helper/assert');
 var error = require('../helper/error');
-var qs = require('../helper/qs');
+var qs = require('qs');
 var PluginHandler = require('../helper/plugins');
 var windowHelper = require('../helper/window');
 var objectHelper = require('../helper/object');


Test patch:
diff --git a/test/helper/iframe-handler.test.js b/test/helper/iframe-handler.test.js
index a93e2d9..d965cf6 100644
--- a/test/helper/iframe-handler.test.js
+++ b/test/helper/iframe-handler.test.js
@@ -171,7 +171,7 @@ describe('helpers iframeHandler', function () {
 
           expect(err).to.eql({
             error:'some_error',
-            errorDescription: 'the+error+description'
+            errorDescription: 'the error description'
           });
 
           setTimeout(done, 100);
diff --git a/test/helper/popup-handler.test.js b/test/helper/popup-handler.test.js
index 3de1187..a078cf8 100644
--- a/test/helper/popup-handler.test.js
+++ b/test/helper/popup-handler.test.js
@@ -2,7 +2,7 @@ var expect = require('expect.js');
 var stub = require('sinon').stub;
 var WinChan = require('winchan');
 
-var qs = require('../../src/helper/qs');
+var qs = require('qs');
 var PopupHandler = require('../../src/helper/popup-handler');
 
 describe('helpers popupHandler', function () {
@@ -80,19 +80,6 @@ describe('helpers popupHandler', function () {
     });
   });
 
-  describe('stringifies the window properties', function () {
-
-    it('using the options received', function () {
-      var str = qs.build({
-        attr1: 'attribute 1',
-        attr2: 'attribute 2',
-        attr3: 'attribute 3'
-      }, ',', false);
-      expect(str).to.eql('attr1=attribute 1,attr2=attribute 2,attr3=attribute 3');
-    });
-
-  });
-
   describe('should open the popup', function () {
     before(function(){
       global.window = {};
diff --git a/test/helper/qs.test.js b/test/helper/qs.test.js
deleted file mode 100644
index 09d712a..0000000
--- a/test/helper/qs.test.js
+++ /dev/null
@@ -1,51 +0,0 @@
-var expect = require('expect.js');
-
-var qs = require('../../src/helper/qs');
-
-describe('helpers qs', function () {
-  describe('build', function () {
-    it('return the formated query string', function () {
-      var object = {
-        attr1: 'attribute_1',
-        attr2: 'attribute_2',
-        attr3: 'attribute_3'
-      };
-
-      var qString = qs.build(object);
-
-      expect(qString).to.eql('attr1=attribute_1&attr2=attribute_2&attr3=attribute_3');
-    });
-  });
-
-  describe('parse', function () {
-    it('return the formated query string', function () {
-      var qString = 'attr1=attribute_1&attr2=attribute_2&attr3=attribute_3';
-
-      var object = qs.parse(qString);
-
-      expect(object).to.eql({
-        attr1: 'attribute_1',
-        attr2: 'attribute_2',
-        attr3: 'attribute_3'
-      });
-    });
-  });
-
-  describe('build and parse', function () {
-    it('return the formated query string', function () {
-      var object = {
-        attr1: 'attribute_1',
-        attr2: 'attribute_2',
-        attr3: 'attribute_3'
-      };
-
-      var new_object = qs.parse(qs.build(object));
-
-      expect(new_object).to.eql({
-        attr1: 'attribute_1',
-        attr2: 'attribute_2',
-        attr3: 'attribute_3'
-      });
-    });
-  });
-});