Skip to content

feat(interceptor): duplicate query calls throw#1630

Merged
mastermatt merged 2 commits intonock:betafrom
mastermatt:interceptor-query-dup-key2
Jul 16, 2019
Merged

feat(interceptor): duplicate query calls throw#1630
mastermatt merged 2 commits intonock:betafrom
mastermatt:interceptor-query-dup-key2

Conversation

@mastermatt
Copy link
Copy Markdown
Member

Continuation of #1626

BREAKING CHANGE: Attempting to call Interceptor.query twice throws an error.

Continuation of nock#1626

BREAKING CHANGE: Attempting to call `Interceptor.query` twice throws an error.
@mastermatt mastermatt requested review from gr2m and paulmelnikow July 16, 2019 15:46
@mastermatt
Copy link
Copy Markdown
Member Author

Going down the route of properly supporting duplicate keys (which I would consider a bug fix, not a feature) gets into the weeds of the issue that we still use qs when doing the matching on query strings. Ref #507.
This solution gets closer to a complete fix by utilizing qs, but stops short of changing the functionality of query matching until #507 is resolved.

Copy link
Copy Markdown
Member

@paulmelnikow paulmelnikow left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice one!

@@ -122,16 +122,27 @@ test('query() accepts URLSearchParams as input', async t => {
})

test('query() throws for duplicate keys', async t => {
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
test('query() throws for duplicate keys', async t => {
test('query() throws if query params have already been defined', async t => {

)
})

test('query() throws for invalid arguments', async t => {
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
test('query() throws for invalid arguments', async t => {
test('query() throws for invalid arguments', t => {

Will also need a t.end().


nock(`${url}`, { allowUnmocked: true })
.get('/')
.query(false)
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, we're removing support for this undocumented feature, correct? Could you update the test title to remove "query false"?

Copy link
Copy Markdown
Member Author

@mastermatt mastermatt Jul 16, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This test just seems to be a duplicate of the test above now, so I'm going to remove it completely.

@mastermatt mastermatt merged commit 2a54482 into nock:beta Jul 16, 2019
@mastermatt mastermatt deleted the interceptor-query-dup-key2 branch July 16, 2019 20:21
@nockbot
Copy link
Copy Markdown
Collaborator

nockbot commented Jul 16, 2019

🎉 This PR is included in version 11.0.0-beta.27 🎉

The release is available on:

Your semantic-release bot 📦🚀

@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
* feat(interceptor): duplicate query calls throw

Continuation of #1626

BREAKING CHANGE: Attempting to call `Interceptor.query` twice throws an error.
gr2m pushed a commit that referenced this pull request Sep 4, 2019
* feat(interceptor): duplicate query calls throw

Continuation of #1626

BREAKING CHANGE: Attempting to call `Interceptor.query` twice throws an error.
gr2m pushed a commit that referenced this pull request Sep 5, 2019
* feat(interceptor): duplicate query calls throw

Continuation of #1626

BREAKING CHANGE: Attempting to call `Interceptor.query` twice throws an error.
juninmd pushed a commit to juninmd/nock that referenced this pull request Mar 21, 2026
* feat(interceptor): duplicate query calls throw

Continuation of nock#1626

BREAKING CHANGE: Attempting to call `Interceptor.query` twice throws an error.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants