This repository has been archived by the owner on Dec 12, 2021. It is now read-only.
private_pub.js changes: only load faye.js if Faye object is not present and use module pattern to prevent pollution of global namespace #51
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Currently PrivatePub does not allow Redis engine to be specified when initializing the Faye server. Developers can now create a private_pub_redis.yml where desired values can be specified as per: http://faye.jcoglan.com/ruby/engines.html This commit does not include checks for required fields or conflicts between using socket or non-socket approach.
@default_options is what we have today for specifying mount point etc. But this can be changed to configurable as well.
…atePub is undefined and client will get an error on not able to accessing faye.js.
…up. Log a console error if it is not. PrivatePub client now no longer throw uncaught exception and upon seeing console log error, client can choose to handle the situation by performing a null check on PrivatePub.fayeClient.
…n it is up. Log a console error if it is not. PrivatePub client now no longer throw uncaught exception and upon seeing console log error, client can choose to handle the situation by performing a null check on PrivatePub.fayeClient." We are violating cross-domain security check by making ajax call to Faye server. This reverts commit ff647fa.
Faye 0.8.0 contains two incompatible changes according to: http://blog.jcoglan.com/2012/02/27/faye-0-8-the-refactoring/ Notably, his check-in allows users to specify one of the 3 support adapters through private_pub.yml. The supported adapters are: thin (default), rainbows, and goliath
…ily to global namespace.
this allows the inclusion of faye-browser.js into the asset pipeline as an optimization.
| @@ -1,4 +1,4 @@ | |||
| function buildPrivatePub(doc) { | |||
| var PrivatePub = (function (doc) { | |||
There was a problem hiding this comment.
+1, the buildPrivatePub function should not be exposed.
this allows specifying a different faye server url for usage on the (rails) server only. Browser clients will still use :server url.
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Allow loading of faye-browser.js directly through asset pipeline, so that only one request is made for .js files in production mode. This also allows easy serving of faye-browser.js through nginx or apache instead of thin.
Also change creation of PrivatePub variable to use module pattern. There is no need to export the buildPrivatePub function into the global namespace.