Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Replace querystring implementation with qs module #382

Conversation

selaux
Copy link
Contributor

@selaux selaux commented Mar 9, 2017

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 auth0/lock#913.

Scope: Replaces all occurences of helpers/qs.js with the qs module

Notes

  • I removed this test, because it was actually testing the querystring implementation
  • I had to adapt this test 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
Copy link
Contributor

Can you share the bundle size difference with this module as a dependency?

@codecov-io
Copy link

codecov-io commented Mar 9, 2017

Codecov Report

Merging #382 into master will decrease coverage by 0.02%.
The diff coverage is 100%.

@@            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 Coverage Δ
src/authentication/passwordless-authentication.js 100% <100%> (ø)
plugins/cordova/popup-handler.js 91.89% <100%> (ø)
src/web-auth/index.js 97.9% <100%> (ø)
src/helper/popup-handler.js 100% <100%> (ø)
src/authentication/index.js 100% <100%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 48dc331...71ac30d. Read the comment docs.

@hzalaz hzalaz added this to the v8-Next milestone Mar 9, 2017
@luisrudge luisrudge merged commit 7b7f43a into auth0:master Mar 9, 2017
@hzalaz hzalaz modified the milestones: v8-Next, v8.4.0 Mar 10, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants