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.

Sign up for GitHub

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

Jump to bottom

11.0.0-beta.30 imports qs but does not require it #1651

Closed
paulmelnikow opened this issue Aug 1, 2019 · 3 comments
Closed

11.0.0-beta.30 imports qs but does not require it #1651

paulmelnikow opened this issue Aug 1, 2019 · 3 comments
Assignees
@mastermatt
Labels
bug released

Comments

@paulmelnikow
Copy link
Member

#1632 removed qs, but recorder.js still imports qs (in beta) here:

nock/lib/recorder.js

Line 4 in 35221ce

const qs = require('qs')

Here is an example of a failing build for 11.0.0-beta.30:
@mastermatt
Copy link
Member

whoops, I wonder how I missed that. PR incoming today probably.

@mastermatt mastermatt self-assigned this Aug 1, 2019
@mastermatt
Copy link
Member

qs is a sub-dep of a dev-dep (request), which means it's available for "requiring" when the tests are run.

Good news is, the usage of qs in recorder.js does not take advantage of the "unpacking" feature of JSON path parameter names. So switching it out for the native querystring will work.

mastermatt added a commit to mastermatt/nock that referenced this issue Aug 1, 2019
@mastermatt
bug(recorder): replace qs lib with native querystring.
bc81323 
The `qs` dependency was removed, however, recorder.js retained usage
of it. Replacing it with `querystring` from the std lib is sufficient.

Introduced: nock#1632
Fixes: nock#1651
@mastermatt mastermatt mentioned this issue Aug 1, 2019
Merged
@weekly-digest weekly-digest bot mentioned this issue Aug 4, 2019
Closed
@paulmelnikow paulmelnikow mentioned this issue Aug 6, 2019
Closed
mastermatt added a commit that referenced this issue Aug 6, 2019
@mastermatt
bug(recorder): replace qs lib with native querystring. (#1653)
e661d0d 
The `qs` dependency was removed, however, recorder.js retained usage
of it. Replacing it with `querystring` from the std lib is sufficient.

Introduced: #1632
Fixes: #1651
@weekly-digest weekly-digest bot mentioned this issue Aug 11, 2019
Closed
@nockbot
Copy link
Collaborator

nockbot commented Aug 13, 2019

🎉 This issue has been resolved in version 11.0.0 🎉

The release is available on:

Your semantic-release bot 📦🚀

gr2m pushed a commit that referenced this issue Sep 4, 2019
@mastermatt @gr2m
bug(recorder): replace qs lib with native querystring. (#1653)
fdf89b2 
The `qs` dependency was removed, however, recorder.js retained usage
of it. Replacing it with `querystring` from the std lib is sufficient.

Introduced: #1632
Fixes: #1651
gr2m pushed a commit that referenced this issue Sep 4, 2019
@mastermatt @gr2m
bug(recorder): replace qs lib with native querystring. (#1653)
ad856d3 
The `qs` dependency was removed, however, recorder.js retained usage
of it. Replacing it with `querystring` from the std lib is sufficient.

Introduced: #1632
Fixes: #1651
gr2m pushed a commit that referenced this issue Sep 5, 2019
@mastermatt @gr2m
bug(recorder): replace qs lib with native querystring. (#1653)
5ce2a21 
The `qs` dependency was removed, however, recorder.js retained usage
of it. Replacing it with `querystring` from the std lib is sufficient.

Introduced: #1632
Fixes: #1651
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@mastermatt mastermatt

Labels
bug released
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
3 participants
@paulmelnikow @mastermatt @nockbot