Fixed bug #1, and added unit test for bug scenario. #2
Conversation
|
I'm not sure if you will like my second commit, but I utilized Node's built-in url resolver to rewrite your url resolver. A lot of lines removed, but I think Node's url resolving is probably more dependable. You can see documentation here: https://nodejs.org/api/url.html#url_url_resolve_from_to I have only ran unit testing to ensure resolveURL behaves the same. It may be worth a look-over to see if any functionality was removed. This pull request is mainly to fix bugs we've found in broken-link-checker. Which is great by the way! |
|
Please remove your second commit. I wrote a custom url resolver for a reason (node#1435 for example). |
| // "?var1=a&var1=b" would've been parsed to { var1: ["a","b"] } | ||
| if (Array.isArray(value1) !== Array.isArray(value2)) return false; | ||
| if (!Array.isArray(value1) || !Array.isArray(value2)) return false; |
There was a problem hiding this comment.
Please replace with:
if (Array.isArray(value1)===false || Array.isArray(value2)===false) return false;
|
I didn't realize there was a bug with Node's url resolver. Huh. You've put a lot of detailed work into this! Nice job. |
|
Thank you. There is still more work to do and I have been sidetracked by other projects for now. I'll be getting back to this one as I hope to propose this package as a replacement for Node's url. Thanks for catching this bug! I'll release a new version tonight for broken-link-checker to use. |
Fixed bug #1, and added unit test for bug scenario.
No description provided.