-
Notifications
You must be signed in to change notification settings - Fork 46
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
Put query data into the data parameter of the fixture function. #79
base: master
Are you sure you want to change the base?
Conversation
I think if look through |
@elisherer there is. It's in the NB: I removed accidentally added commit |
src/superagent-mock.js
Outdated
return data; | ||
} | ||
|
||
const parts = url.split('?'); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
wouldn't it be simpler to check includes
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
parts[1]
is used further
src/superagent-mock.js
Outdated
@@ -1,3 +1,6 @@ | |||
import qs from 'qs'; | |||
import {isEmpty, isNil} from 'lodash'; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think lodash is a big dependency here, will require consumers to have some lind of lodash optimization. better make those checks part of the code or change the imports to direct ones.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I really think there are totally all projects have lodash) But ok, I removed it)
@elisherer could you already merge or reject it?) |
Hi, thanks for your contribution @mtnt. I wouldn't be exclusive between post data and query string because it is possible to have query params in a POST url 😄. If I well understand what you need, you can use Regexp capturing groups in the pattern. // Example 1, the pattern is stricted and the query params order is known
{
pattern: 'https://domain.example?limit=(\d+)&offset=(\d+)',
fixtures: match => {
const [, limit, offset] = match;
// ...
}
}
// Example 2, the pattern matches every requests with query params
[{
pattern: 'https://domain.example?(.*)',
fixtures: match => {
const { limit, offset } = qs(match[1]);
// ...
// may handle 404 somewhere
}
}] |
@DevSide hmm, I got your point... Yeah, I did not think about usage query params in a POST... Maybe the data should be an object with PS yeah, the regexp is that what I currently use for this purpose. |
Usage of a regexp is not convenient: actual params order may be different with a pattern. Much more useful way is to get somehow a package of parsed data. UPD: it`s possible to use "?(.*)" and parse it - in this case each user should reverse engineer the |
path-to-regex could be better choice I guess but not sure it handles query params. |
Oh, I`m sorry, I forgot one more thing: passing complex params such as objects and arrays are converted not like json: |
No description provided.