Fix :host mount option #976
Conversation
AlfonsoUceda
force-pushed
the
fix-mount-host
branch
from
1de56bc
to
91a33e7
Compare
AlfonsoUceda
force-pushed
the
fix-mount-host
branch
from
91a33e7
to
ec691e8
Compare
lib/hanami/app.rb
Outdated
| @@ -62,7 +62,9 @@ def call(env) | |||
| # @api private | |||
| def mount(configuration) | |||
| configuration.mounted.each do |klass, app| | |||
| routes.mount(klass, at: app.path_prefix) | |||
| # TODO: think if this line is in the correct place | |||
| host = [app.host, Components['environment'].host].compact.join('.') if Components['environment'].host != 'localhost' | |||
There was a problem hiding this comment.
@AlfonsoUceda Why we can't just forward the given host?
There was a problem hiding this comment.
I've not added a spec yet but the problem is when you combine option :host in mount DSL and option --host in server command.
There was a problem hiding this comment.
tested this patch without --host param with lvh.me and it works. no need for this special joining of mount host and env host. if something like this is supposed to be a feature i'd introduce something like:
mount HelloWorld::Application, at: "/", subdomain: "hello-world"which would serve requests like https://hello-world.example.com while the host depends on the request's host header if no host: "..." option was given for the mount.
|
Why was this closed and never merged? @jodosha @AlfonsoUceda |
|
@checkbutton Sorry about that! I've restored the branch and I'm re-opening the PR. I'm not sure why it was closed. It might've been a mistake but if it's not @AlfonsoUceda can close it again and give a brief explanation. Also, for your info, Hanami 2.0 will have an entirely new router. This should definitely work on 1.3 though, before that gets released. |
|
@checkbutton this was a proof of concept to understand the problem and if I could make it work. It works fine if I remember well but I never finished tests. I asked @jodosha if he can take a look ;) |
This PR fix #951