Skip to content

bug(recorder): replace qs lib with native querystring#1653

Merged
mastermatt merged 1 commit intonock:betafrom
mastermatt:fix-qs-in-recorder
Aug 6, 2019
Merged

bug(recorder): replace qs lib with native querystring#1653
mastermatt merged 1 commit intonock:betafrom
mastermatt:fix-qs-in-recorder

Conversation

@mastermatt
Copy link
Copy Markdown
Member

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

Note that the output of generateRequestAndResponse will change for requests with search params, however, the net effect when that output is used will be the same.

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
@paulmelnikow
Copy link
Copy Markdown
Member

Wouldn't this break recordings created in the old version?

@mastermatt
Copy link
Copy Markdown
Member Author

I don't think so, but I'll add a test using a recording from v10.

@mastermatt
Copy link
Copy Markdown
Member Author

This change doesn't affect the way recordings are processed, just recorded when output_objects is not set to true.

example:

got('https://example.com/todos/1?foo[bar]=123&baz[0]=a&baz[1]=b')

// output before change
nock('https://example.com:443', {"encodedQueryParams":true})
  .get('/todos/1')
  .query({"foo":{"bar":"123"},"baz":["a","b"]})
  ...

// output after change
nock('https://example.com:443', {"encodedQueryParams":true})
  .get('/todos/1')
  .query({"foo%5Bbar%5D":"123","baz%5B0%5D":"a","baz%5B1%5D":"b"})
  ...

And we already have tests in test_complex_querystring.js showing that both styles are able to be processed.

@paulmelnikow
Copy link
Copy Markdown
Member

Thanks for looking into that!

@mastermatt mastermatt merged commit e661d0d into nock:beta Aug 6, 2019
@mastermatt mastermatt deleted the fix-qs-in-recorder branch August 6, 2019 12:09
@nockbot
Copy link
Copy Markdown
Collaborator

nockbot commented Aug 12, 2019

🎉 This PR is included in version 11.0.0 🎉

The release is available on:

Your semantic-release bot 📦🚀

gr2m pushed a commit that referenced this pull request Sep 4, 2019
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 pull request Sep 4, 2019
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 pull request Sep 5, 2019
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
juninmd pushed a commit to juninmd/nock that referenced this pull request Mar 21, 2026
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
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.

3 participants