Allow localhost database URL

[rynobax]

This change was discussed in #116.

[googlebot]

Thanks for your pull request. It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

📝 Please visit https://cla.developers.google.com/ to sign.

Once you've signed, please reply here (e.g. I signed it!) and we'll verify. Thanks.

---

• If you've already signed a CLA, it's possible we don't have your GitHub username or you're using a different email address on your commit. Check your existing CLA data and verify that your email is set on your git commits.
• If your company signed a CLA, they designated a Point of Contact who decides which employees are authorized to participate. You may need to contact the Point of Contact for your company and ask to be added to the group of authorized contributors. If you don't know who your Point of Contact is, direct the project maintainer to go/cla#troubleshoot. The email used to register you as an authorized contributor must be the email used for the Git commit.
• In order to pass this check, please resolve this problem and have the pull request author add another comment and the bot will run again. If the bot doesn't comment, it means it doesn't think anything has changed.

[rynobax]

I signed it!

[googlebot]

CLAs look good, thanks!

[schmidt-sebastian]

Can you move the port extraction logic from line 136 up so that it happens before this if-statement? Then we won't have to double parse the port number.

Then I would also suggest that instead of assigning 'localhost' to subdomain you should assign it to domain. A subdomain would be something like 'foo' in 'foo.localhost'.

[rynobax]

I believe I made the change you requested about parsing the port number, let me know if that is not what you had in mind.

I also changed it to set the domain to localhost. However, the subdomain also needs to be set to something, as it is used for the 'namespace' variable, which cannot be empty. For now it also sets the subdomain to 'localhost', as I am not entirely sure of what the namespace is used for.

[schmidt-sebastian]

I also changed it to set the domain to localhost. However, the subdomain also needs to be set to something, as it is used for the 'namespace' variable, which cannot be empty. For now it also sets the subdomain to 'localhost', as I am not entirely sure of what the namespace is used for.

Thanks, the changes look mostly good. The namespace is used by our backend if you are not connecting the intended host. For mocking, this should not be needed. It seems like you changed the assert to no longer fire AND you set the subdomain to localhost? Can you explain why we need to do both - otherwise I would just remove the setting of the subdomain and we should be good to submit this PR.

[rynobax]

If the subdomain is not set, the test fails with the error Error: Invalid Firebase Database URL failed: First argument must be a valid firebase URL and the path can't contain ".", "#", "$", "[", or "]".

The error seems to be triggered by this line, because the namespace is an empty string:

firebase-js-sdk/packages/database/src/core/util/validation.ts

Line 420 in 73a586c

!isValidKey(parsedUrl.repoInfo.namespace) ||

If I remove that line all the tests pass.

The two options that come to my mind are:

1. Do not set the subdomain on localhost domains
2. In validation.ts, skip the namepace validity check if the host is localhost

Or:

1. Set the subdomain on localhost domains
2. Remove the extra check I added to the assert

[schmidt-sebastian]

Do not set the subdomain on localhost domains
In validation.ts, skip the namespace validity check if the host is localhost

Please go ahead and do this. It seems cleaner.

[rynobax]

I made the changes.

[schmidt-sebastian]

LGTM. Note that this won't work for localhost URLs that don't have a port set. I will send you a follow up PR to add support for this.