-
Notifications
You must be signed in to change notification settings - Fork 124
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
fix: check url before request #172
Conversation
Current coverage is 98.19%@@ master #172 diff @@
==========================================
Files 1 1
Lines 382 386 +4
Methods 31 31
Messages 0 0
Branches 112 113 +1
==========================================
+ Hits 375 379 +4
Misses 7 7
Partials 0 0
|
每次都多了一次字符串相加… |
还好吧…损耗不大 |
if 判断,然后抛错? Sent from my iPhone
|
@@ -173,6 +174,8 @@ exports.requestThunk = function (url, args) { | |||
|
|||
exports.requestWithCallback = function (url, args, callback) { | |||
// requestWithCallback(url, callback) | |||
assert(url && (typeof url === 'string' || url.constructor.name === 'Url'), |
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.
if (!url || (typeof url !== 'string' && url.constructor.name !== 'Url')) {
throw new TypeError('expect request url to be a string or an instance of Url, but got '+ url);
}
考虑到这个 request 会大量调用,能省即省。
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.
好了
+1 |
@dead-horse 测试用例挂了 |
@@ -210,7 +216,7 @@ exports.requestWithCallback = function (url, args, callback) { | |||
parsedUrl = url; | |||
} | |||
|
|||
var method = (args.type || args.method || parsedUrl.method || 'GET').toUpperCase(); | |||
var method = (args.type || args.method || 'GET').toUpperCase(); |
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.
规范一下,第一个参数只允许 string 或者 Url
对象可能会有不兼容
只判断 string 或者 object 了 |
感觉可以当做是一个 fix 了 |
+1 |
2.9.1 |
否则后面会报错,让人没有头绪