nock mixes usage of qs and querystring

[jeffora]

It appears nock mixes usage of native node querystring module and npm installed qs module for parsing querystring-like objects.

In nock/lib/interceptor.js it requires qs (a dependency in package.json), but in nock/lib/match_body.js it requires querystring, the native node library.

These two libraries have different outputs. For example:

var query = require('querystring');
var qs = require('qs');

query.parse('a%5Bb%5D=test'); // => { 'a[b]': 'test' }
qs.parse('a%5Bb%5D=test'); // => { a: { b: 'test' } }

This means interceptors and body matchers have different behavior.

[pgte]

@jeffora Good catch.

A PR would be welcome..

[stale]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. We try to do our best, but nock is maintained by volunteers and there is only so much we can do at a time. Thank you for your contributions.

[AJamesPhillips]

Which dependency would be desirable to keep? 'querystring' or 'qs' ?

[kbakba]

Which dependency would be desirable to keep? 'querystring' or 'qs' ?

I think native 'querystring'

see #563 (comment)

[paulmelnikow]

I think this is a good idea, though it will be a breaking change to body matching. For example, It will break use cases like this one:

nock/tests/test_body_match.js

Lines 193 to 222 in 3d74a15

	test('array like urlencoded form posts are correctly parsed', function(t) {
	nock('http://example.test')
	.post('/', {
	arrayLike: [
	{
	fieldA: '0',
	fieldB: 'data',
	fieldC: 'value',
	},
	],
	})
	.reply(200)
	mikealRequest(
	{
	url: 'http://example.test',
	method: 'post',
	form: {
	'arrayLike[0].fieldA': '0',
	'arrayLike[0].fieldB': 'data',
	'arrayLike[0].fieldC': 'value',
	},
	},
	function(err, res) {
	if (err) throw err
	assert.equal(res.statusCode, 200)
	t.end()
	}
	)
	})

I suggest we add a flag to allow users who use array syntax and qs-style encoding to keep using it. We could make qs an optionalDependency.

[nockbot]

🎉 This issue has been resolved in version 11.0.0-beta.30 🎉

The release is available on:

• npm package (@beta dist-tag)
• GitHub release

Your semantic-release bot 📦🚀

[nockbot]

🎉 This issue has been resolved in version 11.0.0 🎉

The release is available on:

• npm package (@next dist-tag)
• GitHub release

Your semantic-release bot 📦🚀