pubnub / javascript

PubNub JavaScript SDK docs https://www.pubnub.com/docs/sdks/javascript
Other
553 stars 403 forks source link

.ready() not defined using PUBNUB.secure #5

Closed brettveenstra closed 10 years ago

brettveenstra commented 11 years ago

When initializing with PUBNUB.secure() call instead of PUBNUB.init(), the .ready() function is not provided.

Two questions:

  1. Isn't .ready() needed when including pubnub in <head>? (I can't move it to last element in body for my solution.)
  2. Can I expect .secure() to be a pure extension of what init() creates?

example code:

    if (Messaging.Settings.Cipher) {
      provider = PUBNUB.secure({
        subscribe_key: Messaging.Settings.SubscriptionKey,
        ssl: Messaging.Settings.Secure || false,
        origin: Messaging.Settings.Origin,
        cipher_key: Messaging.Settings.Cipher
      });
    }
    else {
      provider = PUBNUB.init({
        subscribe_key: Messaging.Settings.SubscriptionKey,
        ssl: Messaging.Settings.Secure || false,
        origin: Messaging.Settings.Origin
      });
    }

    provider.ready();   // must use this since we are using pubnub in head
stephenlb commented 11 years ago

Hi @brettveenstra Good question let me see if there is an interface between Secure and Init for the .ready() function.

stephenlb commented 11 years ago

@brettveenstra this is not available for PUBNUB.secure(...) as you have noted. The best option is to move the PubNub javascript resources to the bottom of the <body>. This option will provide the best performance for all browsers.

<script ...></script>
<script ...></script>
</body> <!-- Closing Body Tag -->

This method allows you to skip the .ready() function altogether. It is much better to do this!

geremyCohen commented 11 years ago

I will also file a ticket to expose it via secure as well. @brettveenstra does @stephenlb's workaround work for you, or you are stuck with PubNub scripts in the head?

brettveenstra commented 11 years ago

@geremyCohen Definitely need support for it so can use this in head. Unfortunately cannot register them all in the body.

stephenlb commented 11 years ago

@geremyCohen can you add this? it is a single line addition to /core/crypto/encrypt-pubnub.js here:

https://github.com/pubnub/javascript/blob/master/core/crypto/encrypt-pubnub.js#L34

And the change is simply exposing:

    ready       : pubnub.ready, // < --- ADD
    raw_encrypt : encrypt,
    raw_decrypt : decrypt,
    ...
geremyCohen commented 11 years ago

@stephenlb @brettveenstra will do.

geremyCohen commented 11 years ago

Adding now to master.

On 05/13/2013 07:48 PM, Stephen L. Blum wrote:

|ready: pubnub.ready,|