Subscribe endpoint will send state query parameter now.
#147
Conversation
* Update package-lock.json after npm install - David * Fix eslint and prettier errors and typo - David * Setting the default presence heartbeat to false on subscribe - David * Update ESLint rules and fix others - David * Formatting and fixing broken tests - David * Format subscription_manager.test.js and fix tests - David * Format reconnection_manager.test.js and fix tests - David * Update versions and include the build - David * Fix linting errors and rules for Codacy checks - David * Fix broken tests for no longer specified pubnub.yml version - David * Update the date to now - David
Return 'state' back for subscribe endpoint, so client will maintain state information even when heartbeat disabled.
davidnub
left a comment
There was a problem hiding this comment.
Looks good! Just one change to set default on destructure. 👍
|
|
||
| _startSubscribeLoop() { | ||
| this._stopSubscribeLoop(); | ||
| let presenceState = {}; |
There was a problem hiding this comment.
Technically these can be const but it's fine to keep the same pattern for now.
|
|
||
| export function prepareParams({ config }: ModulesInject, incomingParams: SubscribeArguments): Object { | ||
| let { channelGroups = [], timetoken, filterExpression, region } = incomingParams; | ||
| let { state, channelGroups = [], timetoken, filterExpression, region } = incomingParams; |
There was a problem hiding this comment.
It's probably safer to set state = {} in case it was not passed into the incomingParams this way later you won't get an error when you do Object.keys(state)
There was a problem hiding this comment.
@davidnub it is good and not so good idea to change.
This value is passed from internal SDK logic. Subscriber provides placeholder ({}) if there is no channels with state.
If because of some reasons Object.kets(state) will throw, because of attempt to access undefined - it will mean, what something has been messed up. If we will add default value in destructure - we can miss this potentially bug-prone change.
There was a problem hiding this comment.
Yes I agree with you that the value passed in from the internal SDK logic will include an empty object and therefore we shouldn't need to worry about it. This is more of a "best practice" scenario to treat each function as its own unit and therefore securing it will only prove that this specific function operates as expected.
Detecting an error only when the calling function changes smells more like "testing in real time" which is not great, but I'm fine to let go to ensure we have progress. Specifically since other destructured properties are not being defaulted either.
|
|
||
| export function prepareParams({ config }: ModulesInject, incomingParams: SubscribeArguments): Object { | ||
| let { channelGroups = [], timetoken, filterExpression, region } = incomingParams; | ||
| let { state, channelGroups = [], timetoken, filterExpression, region } = incomingParams; |
There was a problem hiding this comment.
Yes I agree with you that the value passed in from the internal SDK logic will include an empty object and therefore we shouldn't need to worry about it. This is more of a "best practice" scenario to treat each function as its own unit and therefore securing it will only prove that this specific function operates as expected.
Detecting an error only when the calling function changes smells more like "testing in real time" which is not great, but I'm fine to let go to ensure we have progress. Specifically since other destructured properties are not being defaulted either.
* Update package-lock.json after npm install - David * Fix eslint and prettier errors and typo - David * Setting the default presence heartbeat to false on subscribe - David * Update ESLint rules and fix others - David * Formatting and fixing broken tests - David * Format subscription_manager.test.js and fix tests - David * Format reconnection_manager.test.js and fix tests - David * Update versions and include the build - David * Fix linting errors and rules for Codacy checks - David * Fix broken tests for no longer specified pubnub.yml version - David * Update the date to now - David * Missed version update - David * Subscribe endpoint will send `state` query parameter now. (#147) * fix(subscribe): return 'state' query parameter back Return 'state' back for subscribe endpoint, so client will maintain state information even when heartbeat disabled. * Prettier formatting for Codacy happiness - David * Bumped version to 4.24.1 - David
* Update package-lock.json after npm install - David * Fix eslint and prettier errors and typo - David * Setting the default presence heartbeat to false on subscribe - David * Update ESLint rules and fix others - David * Formatting and fixing broken tests - David * Format subscription_manager.test.js and fix tests - David * Format reconnection_manager.test.js and fix tests - David * Update versions and include the build - David * Fix linting errors and rules for Codacy checks - David * Fix broken tests for no longer specified pubnub.yml version - David * Update the date to now - David * Missed version update - David * Subscribe endpoint will send `state` query parameter now. (#147) * fix(subscribe): return 'state' query parameter back Return 'state' back for subscribe endpoint, so client will maintain state information even when heartbeat disabled. * Prettier formatting for Codacy happiness - David * Bumped version to 4.24.1 - David * added try catch * auto gen files * removed category assignment for exception * version update * build files * readme file * updated default origin * minor refactor * build files * refactor per codacy * build files * minor change * build files * build files after conflicts resolved * Fix code formatting/linting and update .pubnub.yml version Includes re-compile - David
* Update package-lock.json after npm install - David * Fix eslint and prettier errors and typo - David * Setting the default presence heartbeat to false on subscribe - David * Update ESLint rules and fix others - David * Formatting and fixing broken tests - David * Format subscription_manager.test.js and fix tests - David * Format reconnection_manager.test.js and fix tests - David * Update versions and include the build - David * Fix linting errors and rules for Codacy checks - David * Fix broken tests for no longer specified pubnub.yml version - David * Update the date to now - David * Missed version update - David * Subscribe endpoint will send `state` query parameter now. (#147) * fix(subscribe): return 'state' query parameter back Return 'state' back for subscribe endpoint, so client will maintain state information even when heartbeat disabled. * Prettier formatting for Codacy happiness - David * Bumped version to 4.24.1 - David * added try catch * auto gen files * removed category assignment for exception * version update * build files * readme file * updated default origin * minor refactor * build files * refactor per codacy * build files * minor change * build files * build files after conflicts resolved * Fix code formatting/linting and update .pubnub.yml version Includes re-compile - David * handle heartbeat when heartbeatInterval is provided * version update * build files * getHeartbeatInterval * build files * rollback getHeartbeatInterval for test * added back getHeartbeatInterval * unit test fix * build files * Removed unused done() No need to comment it out if it's not used, just remove. * Update the compiled lib - David
🛠 subscribe: return 'state' query parameter back
Return 'state' back for subscribe endpoint, so client will maintain state information
even when heartbeat disabled.