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

Add fastify.url to return the current url of the application #4586

Closed
2 tasks done
mcollina opened this issue Feb 17, 2023 · 11 comments · Fixed by #4674
Closed
2 tasks done

Add fastify.url to return the current url of the application #4586

mcollina opened this issue Feb 17, 2023 · 11 comments · Fixed by #4674
Labels
feature request New feature to be added good first issue Good for newcomers

Comments

@mcollina
Copy link
Member

Prerequisites

  • I have written a descriptive issue title
  • I have searched existing issues to ensure the feature has not already been requested

🚀 Feature Proposal

I think we should add fastify.url as it would be a convenient API.
I spend way too much time coding http://localhost:${server.server.address().port, so maybe we could get it done in Fastify itself.

Wdyt?

Motivation

No response

Example

In Platformatic, we addded:

https://github.com/platformatic/platformatic/blob/d8ad6a97e552c4d2c365a27b1a427738aaa6833c/packages/service/index.js#L208-L215

@mcollina mcollina added feature request New feature to be added good first issue Good for newcomers labels Feb 17, 2023
@climba03003
Copy link
Member

We should also provide the correct protocol based on server setting (maybe server instance class would be better).

@mcollina
Copy link
Member Author

Yes, exactly

@jsumners
Copy link
Member

How is this different from the discussion in #387?

@climba03003
Copy link
Member

climba03003 commented Feb 17, 2023

How is this different from the discussion in #387?

In the discussion is used for requset.
In here is used for a friendly shortcut for test-case.

Another main different is one of them depends on header, one of them depends on server startup setting only.

@Uzlopak
Copy link
Contributor

Uzlopak commented Feb 18, 2023

i already see that people report bugs, saying that fastify.url does not respect their top level domain and just report http://127.0.0.1:3000 (instead of http://www.mysecret.tld:3000)

@mcollina
Copy link
Member Author

mcollina commented Feb 19, 2023

Ouch, now that I think about it, it will likely happen. Maybe we should call it localUrl instead?

@climba03003
Copy link
Member

climba03003 commented Feb 19, 2023

Or listenHost / listenOrigin ?

@mcollina
Copy link
Member Author

listenOrigin is likely the more correct one

@Uzlopak
Copy link
Contributor

Uzlopak commented Feb 19, 2023

I would prefer something like originUrl as listen is a Verb indicating a function.

@climba03003
Copy link
Member

I would prefer something like originUrl as listen is a Verb indicating a function.

originUrl does not indicate what origin it is. and it is odd to have both origin and url since origin is part of url.

Would listeningOrigin or currentListeningOrigin be specific enough?

@mcollina
Copy link
Member Author

listeningOrigin would work for me.

@Eomm Eomm linked a pull request Apr 11, 2023 that will close this issue
4 tasks
zsimo added a commit to zsimo/fastify that referenced this issue Apr 26, 2023
… getter) (fastify#4586)

Co-authored-by: James Sumners <james@sumners.email>
zsimo pushed a commit to zsimo/fastify that referenced this issue Apr 26, 2023
zsimo pushed a commit to zsimo/fastify that referenced this issue Apr 27, 2023
zsimo pushed a commit to zsimo/fastify that referenced this issue Apr 27, 2023
zsimo pushed a commit to zsimo/fastify that referenced this issue May 2, 2023
zsimo pushed a commit to zsimo/fastify that referenced this issue May 3, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature request New feature to be added good first issue Good for newcomers
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants