Added Passable options #32
Merged
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
…ation. This allows someone to define settings immediately upon instantiation instead of after.
It works like so:
```
socket = new ReconnectingWebSocket(url, null, {debug:true, reconnectInterval:10000});
```
The settings object are merged with the passed options object. Non-existent settings will be ignored when merging. So for example, if you passed an option of "hello":"world", it would be ignored, since there is no setting called "hello".
After the merge, the settings are added to the object (this.debug, this.reconnectInterval, etc.). However you can still change these settings after the fact, just like you can now:
```
socket = new ReconnectingWebSocket(...);
socket.debug = true;
```
Thus, backwards compatibility is preserved with this pull request.
Overall this change will allow some more flexibility in the future. This will allow an "automaticOpen" option to be passed, as previously discussed. Some other thigs to consider is this approach would open the door for a change that allows event handlers to be passed through these options as well. So for example, you could eventually do:
```
socket = new ReconnectingWebSocket(url, null, {
debug:true,
reconnectInterval:5000,
onconnecting: function(){
// some code
}
});
```
That approach would fix the previously discussed issue about the socket connecting before the `onconnecting` event handler can be defined. This pull request does not allow you to do this, but it opens the door for this approach.
Moved reconnectAttempts to the read-only section of variables. Not sure why it was with the other settings. It is a read only setting, correct? Optimized the merging and defining of options and settings. Removed an unnecessary loop.
|
This pull request opens the door to address both of the problems discussed in #30. |
|
Nice. Can you add some documentation explaining this to README and the JS header comments. |
… Parameters section with explanations. Added Options section with explanations, accepted and default values.
|
Done. I added quite a bit to the docs. |
|
Thanks! |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
I changed settings to an object, and added passable options on instantiation. This allows someone to define settings immediately upon instantiation instead of after. It works like so:
The settings object compared to the options and then added to the object. Options that don't exist in the settings will be ignored. So for example, if you passed an option of "hello":"world", it would be ignored, since there is no default setting called "hello". The options are completely optional. You can still change these settings after the fact, just like you can now:
Therefore, backwards compatibility is preserved with this pull request.
Overall this change will allow some more flexibility in the future. For example, this will allow an "automaticOpen" option to be passed, as previously discussed. Some other things to consider is this approach would open the door for a change that allows event handlers to be passed through these options as well. So for example, you could eventually do:
That approach would fix the previously discussed issue about the socket connecting before the
onconnectingevent handler can be defined. This pull request does not allow you to do this, but it opens the door for this approach to be implemented at a later time.