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

RFC - Consistent path comparison #2324

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open

RFC - Consistent path comparison #2324

wants to merge 2 commits into from

Conversation

mastermatt
Copy link
Member

@mastermatt mastermatt commented Mar 28, 2022

BREAKING CHANGE: The query/search portion of the path is now always excluding during path matching.

Previously, the pathname and search string were included during path comparison by default, however, the search string was removed during the matching step if the query method had been called or a search string was provided as part of a string to the first argument of a "verb" method.
This change makes the experience consistent by making query/search matching exclusively opt-in.

I find this little diagram useful, to clarify terminology.
https://nodejs.org/api/url.html#url-strings-and-url-objects

By example:

nock('https://example.com')
  .get('/foo')
  .reply(200, 'OK')

GET https://example.com/foo?name=alice
Previously, Nock would not have matched the request because the search portion of the path on the request did not match what was provided to Nock.

This change removes the search portion during that matching step and will now match the request.

If you're Nocking a request where matching the query/search portion is desirable, then using the .query() method or passing a search string as part of the path will continue to work as it did before. ei .get('/foo?name=alice') or .get('/foo').query({name: 'alice}) etc.

The noticeable breaking change will be those using a RegExp or callback function on the path.
.get(/foo.*alice/) nor .get(uri => uri.endsWith('alice') will work going forward as only "/foo" will be supplied to the match or callback.

Reasoning

It is believed that this behavior is more intuitive to consumers. The idea that the search parameters are considered a separate chunk of data on the request, similar to headers and the body. And therefore should follow the same pattern where Nock should match the request unless the caller opts-in to comparing the search params.

BREAKING CHANGE: The query/search portion of the path is now always excluding during path matching.

Previously, the pathname and search string  were included during path comparison by default, however, the search string was removed during the matching step if the `query` method had been called or a search string was provided as part of a string to the first argument of a "verb" method.
This change makes the experience consistent by making query/search matching exclusively opt-in.
@mastermatt mastermatt changed the title WIP - Consistant path comparision RFC - Consistent path comparison Mar 28, 2022
@mastermatt mastermatt marked this pull request as ready for review March 28, 2022 19:07
@mastermatt mastermatt requested a review from a team March 28, 2022 19:07
@mastermatt
Copy link
Member Author

The most recent example of the current behavior causing confusion #2322
fyi @ebisbe

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.

None yet

1 participant