Repository navigation
url: parsed .search should always be a string. #9600
Description
Activity
- addedurlIssues and PRs related to the legacy built-in url module.Issues and PRs related to the legacy built-in url module.
on Nov 14, 2016 I guess the same would go for
hash,portand probably others...I guess the same would go for hash, port and probably others...
Yes.
Does anyone mind if I take on this issue?
@jessicaquynh this may be a good issue to dig into but @jasnell should be able to confirm if this is something that will be an easy fix, or something we rely on the WhatWG implementation for
Sounds good to me! Thanks @thealphanerd !
@jessicaquynh If you want, this can be broken up into two contributions: First, write tests for these and put them in
test/known_issues(which is where we put tests that we expect to fail but that we one day hope to move to the regular test suite when we fix issues in the code such that they pass).Then you can contribute a fix that would also move those tests out of
test/known_issuesand intotest/parallel.Doing it in two parts like that is not required, but it's an option.
@Trott Thanks! I'm interested in keeping with due diligence to the process. So I will break it up into two!
- added a commit that references this issue
on Nov 25, 2016 I don't think we should change url, existing behaviour is justifiable, and @jasnell is working on a
url.URLthat will meet browser specifications. Why break people's existing code, when we still wouldn't get a browser compliant url lib?The existing behavior seems like a 🐛 to me.
I agree with @sam-github:
existing behaviour is justifiable, and @jasnell is working on a
url.URLthat will meet browser specificationsInterpreting
nullas "nonexistent" is a reasonable justification of the current behavior. Changing this behavior would likely break a lot of code, and since v7.0.0 the docs explicitely say:While the Legacy API has not been deprecated, it is maintained solely for backwards compatibility with existing applications. New application code should use the WHATWG API.
The new API provides the desired behavior and changes to the old API will most probably do more harm than good at this point.
Since there are unlikely to be changes like this on the old
parseAPI and there hasn't been any movement on this in a long time (in fact we've gone in the opposite direction with some bug fixes), I'm going to go ahead and close this. That said, please feel free to reopen if you believe this is still an issue that needs to be addressed — I'm just trying to keep things tidy and not acting on any strong opinions. :)
Node v7.1.0.
I parsed a url with the
require('url')module using.parseand ran into a gotcha.I had
path.join('vendor', url.pathname, url.search)and it errored because.searchwasnull.I had assumed it followed the browser behavior of returning an empty string when no search query is found.