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 host-chsk-url-fn #136

Closed
wants to merge 5 commits into from
Closed

Add host-chsk-url-fn #136

wants to merge 5 commits into from

Conversation

danielcompton
Copy link
Collaborator

It would be helpful for people using cross domain channel sockets to have a simplified chsk-url-fn for making connections to different hosts. This PR adds the function, and documentation on how to use it.

@danielcompton
Copy link
Collaborator Author

If you'd prefer fewer functions in the main Sente ns, the other option I thought of was making default-chsk-fn take a map of config and return a function. The function would override any config provided by window-location, with the initial config provided. This would be a breaking change though, and more complex to use.

@ptaoussanis
Copy link
Member

Hi Daniel, thanks for this.

As I mentioned last time this came up (in #50), I'm not convinced it makes sense to add any specialised code for the use case you're describing.

An appropriate chsk-url-fn can usually be expressed more flexibly and transparently as a small inline fn, no?

What problem are you actually trying to solve here? Is it that you feel users won't understand how to write an appropriate chsk-url-fn? Could that not better be solved by adding a short cross-domain example to the default-chsk-url-fn docstring? (Just trying to understand your motivation better).

Cheers :-)

@danielcompton
Copy link
Collaborator Author

I thought that users may not understand how to write the chsk-url-fn correctly. A docstring would be fine too if you'd prefer that.

ptaoussanis added a commit that referenced this pull request Jun 26, 2015
@ptaoussanis
Copy link
Member

Okay, going to simplify this. Will keep :chsk-url-fn for the moment as an advanced/undocumented feature.

For the simpler common-case that you're describing, have added a :host key to the client-side make-channel-socket! options. Does that handle your case okay?

ptaoussanis added a commit that referenced this pull request Jun 26, 2015
@danielcompton
Copy link
Collaborator Author

Sure, if you prefer that, I'm happy with any of the options.

@ptaoussanis
Copy link
Member

Great, closing this then. Change is up on the dev branch and [com.taoensso/sente "1.6.0-alpha2"] on Clojars. Cheers :-)

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

Successfully merging this pull request may close these issues.

2 participants