Skip to content
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

subdomain() doesn't parse localhost subdomains properly #375

Open
sebastian-zarzycki-apzumi opened this issue Jan 24, 2019 · 3 comments
Open

Comments

@sebastian-zarzycki-apzumi

subdomain("http://my-subdomain.localhost") -> empty
domain("http://my-subdomain.localhost") -> my-subdomain.localhost

@rodneyrehm
Copy link
Member

I see how this can be confusing… .localhost is considered the TLD (Top Level Domain, e.g. .com), which makes my-subdomain.localhost the domain, and there is no subdomain.

here's the origin of that: https://github.com/medialize/URI.js/blob/gh-pages/src/URI.js#L1529-L1533

@sebastian-zarzycki-apzumi
Copy link
Author

I see. Well, in my mind, in x.localhost, x is always considred to be a subdomain and localhost is domain. I have no earthly idea if it is correct with the general understanding of how domains work for internet addresses. Do you think you could implement a workaround for localhosts in particular? If need be, maybe driven by some additional parameter passed to subdomain()?

@rodneyrehm
Copy link
Member

PRs welcome ;)

If you're interested in tackling this allow me to give a few pointers:

  • the change needs to be backward compatible
  • the change may not change a function's signature to diverge from how other function signatures look
  • the change needs to be configurable, as the next person may need the same for lolcat (because they were bored by localhost or something - what do I know, humans and domains - funny business ;))

So I'd probably add something like URI.forceDomains = [ 'localhost' ] and then make .tld() and .domain() work with that. that would solve the second and third point. In order to solve the first point we'd consider URI.forceDomains = [] the default config.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants