New Feature: Protocol support #131
Conversation
| let(:_options) { { :url_links => "//example.com" } } | ||
| it "should generate url links with correct protocol" do | ||
| expect(evaljs("Routes.new_inbox_url()")).to eq("//example.com#{routes.new_inbox_path}") | ||
| expect(evaljs("Routes.new_session_url()")).to eq("//example.com#{routes.new_session_path}") |
There was a problem hiding this comment.
As I understand if the url_links is abstract from protocol than protocol route option is ignored, correct? I don't think this is right.
new_session route forces protocol to be https and it can only be ignored when url_links specified protocol explicitly.
So I think that //example.com and example.com should work in the same way.
There was a problem hiding this comment.
Yes, @bogdan is right. //example.com use protocol from which web page was loaded.
There was a problem hiding this comment.
@bogdan Depends on how you think of it. Some people might expect a protocol relative base URI to stay protocol relative since it would be used in HTML. If you are on https and you want to stay on https then using protocol relative URI would work. Pre-generating the routes would have no way of knowing if you were on http or https. We could add some JavaScript to detect that when the JavaScript function is called, but I don't think that is the best way to do it. I designed this to support backwards compatibility (minor version bump) while providing the ability to add a specific protocol at runtime, but I think it would be better to not support protocol relative base URIs at all, add a default_host config, and have the routes definition be the authority on which protocol/host to use. This would require a major version bump since it would change the behavior/expectations of the config values, but I think it would be better in the long run. If you agree, I will make the changes to this pull request.
|
@bogdan @le0pard I just realized that js-routes already provides a Also, do you agree that the |
|
I'm going to close this PR since it has been superseded by #132. |
New Features:
:url_linksconfig.Changes:
generate_url_linkno longer validates that the:url_linksconfig value contains the protocol.This PR fixes #127.