feat(buildFetchUrl): enable optional ids via question mark #13
Conversation
|
Hi @PatNeedham can you also include an update the readme ? |
PatNeedham
force-pushed
the
feature/optional-params
branch
from
ff7f5ce
to
b22a7ae
Compare
PatNeedham
force-pushed
the
feature/optional-params
branch
from
8f4f563
to
1ae6f83
Compare
__tests__/helpers/url.spec.js
Outdated
| query: { userId: 'questionable?' }, | ||
| }, | ||
| }); | ||
| expect(url).toBe('http://api.domain.com/users?userId=questionable?'); |
There was a problem hiding this comment.
What use case would require a trailing ??
There was a problem hiding this comment.
Honestly not sure, I was mainly trying to think of all the possible cases that could cause it to fail, regardless of how realistic they are.
There was a problem hiding this comment.
Gotcha. Okay, Im thinking one more unit test for this case would be good: http://api.domain.com/users/:user?/comments and pass:
opts: {
query: { someModifier: 'questionable?' },
},
without a :user defined and see if:
expect(url).toBe('http://api.domain.com/users/comments?someModifier=questionable?')
There was a problem hiding this comment.
Test fails!
Ampersand is being included instead of the question mark. Well at least initially that was the case. But moving the .replace(/\?\//g, '/'); to the end of the initial builtUrl value (before the addQueryParams call) got it to pass, and all remaining tests pass as well.
|
@PatNeedham you will need to amend your last commit as its causing the build to fail. |
There was a problem hiding this comment.
Add one more test case and rebase and reword commits that dont follow Conventional Commits https://www.conventionalcommits.org/en/v1.0.0/#summary Thanks again!
Since this change from the first commit is no longer taking place (as the replace is now being called on the This one becomes no longer necessary. |
|
@PatNeedham Yeah definitely go ahead and squash commits in this PR and reword the commit to follow convention. That will fix things. Thanks! |
PatNeedham
force-pushed
the
feature/optional-params
branch
from
7277c52
to
dd2aee8
Compare
|
🎉 This PR is included in version 1.9.0 🎉 The release is available on: Your semantic-release bot 📦🚀 |
No description provided.