refactor: remove context from SharedState#set

src/common/SharedState.js

@@ -33,8 +33,6 @@ export const kSharedStatePromiseStore = Symbol('soundworks:shared-state-promise-
  *  applied to the state.
  * @param {Object} oldValues - Key / value pairs of the updated params before
  *  the updates has been applied to the state.
- * @param {Mixed} [context=null] - Optionnal context object that has been passed
- *  with the values updates in the `set` call.
  */
 
 /**
@@ -136,27 +134,27 @@ ${JSON.stringify(initValues, null, 2)}`);
     this[kSharedStatePromiseStore] = new PromiseStore(this.constructor.name);
 
     // add listener for state updates
-    this.#client.transport.addListener(`${UPDATE_RESPONSE}-${this.#id}-${this.#remoteId}`, async (reqId, updates, context) => {
-      const updated = await this.#commit(updates, context, true, true);
+    this.#client.transport.addListener(`${UPDATE_RESPONSE}-${this.#id}-${this.#remoteId}`, async (reqId, updates) => {
+      const updated = await this.#commit(updates, true, true);
       this[kSharedStatePromiseStore].resolve(reqId, updated);
     });
 
     // retrieve values but do not propagate to subscriptions
-    this.#client.transport.addListener(`${UPDATE_ABORT}-${this.#id}-${this.#remoteId}`, async (reqId, updates, context) => {
-      const updated = await this.#commit(updates, context, false, true);
+    this.#client.transport.addListener(`${UPDATE_ABORT}-${this.#id}-${this.#remoteId}`, async (reqId, updates) => {
+      const updated = await this.#commit(updates, false, true);
       this[kSharedStatePromiseStore].resolve(reqId, updated);
     });
 
-    this.#client.transport.addListener(`${UPDATE_NOTIFICATION}-${this.#id}-${this.#remoteId}`, async (updates, context) => {
+    this.#client.transport.addListener(`${UPDATE_NOTIFICATION}-${this.#id}-${this.#remoteId}`, async (updates) => {
       // https://github.com/collective-soundworks/soundworks/issues/18
       //
       // # note: 2002-10-03
       //
-      // `setTimeout(async () => this.#commit(updates, context, true, false));`
+      // `setTimeout(async () => this.#commit(updates, true, false));`
       // appears to be the only way to push the update commit in the next event
       // cycle so that `attach` can resolve before the update notification is
       // actually dispatched. The alternative:
-      // `Promise.resolve().then(() => this.#commit(updates, context, true, false))``
+      // `Promise.resolve().then(() => this.#commit(updates, true, false))``
       // does not behave as expected...
       //
       // However this breaks the reliability of:
@@ -170,7 +168,7 @@ ${JSON.stringify(initValues, null, 2)}`);
       // ```
       // which is far more important than the edge case reported in the issue
       // therefore this wont be fixed for now
-      this.#commit(updates, context, true, false);
+      this.#commit(updates, true, false);
     });
 
     // ---------------------------------------------
@@ -302,7 +300,7 @@ ${JSON.stringify(initValues, null, 2)}`);
     }
   }
 
-  async #commit(updates, context, propagate = true, initiator = false) {
+  async #commit(updates, propagate = true, initiator = false) {
     const newValues = {};
     const oldValues = {};
 
@@ -336,7 +334,7 @@ ${JSON.stringify(initValues, null, 2)}`);
 
     if (propagate && Object.keys(newValues).length > 0) {
       this.#onUpdateCallbacks.forEach(listener => {
-        promises.push(listener(newValues, oldValues, context));
+        promises.push(listener(newValues, oldValues));
       });
     }
 
@@ -409,26 +407,23 @@ ${JSON.stringify(initValues, null, 2)}`);
    * ```
    *
    * @param {object} updates - Key / value pairs of updates to apply to the state.
-   * @param {mixed} [context=null] - Optionnal contextual object that will be propagated
-   *   alongside the updates of the state. The context is valid only for the
-   *   current call and will be passed as third argument to all update listeners.
    * @returns {Promise<Object>} A promise to the (coerced) updates.
    *
    * @example
    * const state = await client.stateManager.attach('globals');
    * const updates = await state.set({ myParam: Math.random() });
    */
-  async set(updates, context = null) {
+  async set(updates) {
     if (this.#detached) {
       return;
     }
 
-    if (!isPlainObject(updates)) {
-      throw new TypeError(`[SharedState] State "${this.#className}": state.set(updates[, context]) should receive an object as first parameter`);
+    if (isPlainObject(arguments[0]) && isPlainObject(arguments[1])) {
+      logger.deprecated('SharedState.set(updates, context)', 'a regular parameter set with `event=true` behavior', '4.0.0-alpha.29');
     }
 
-    if (context !== null && !isPlainObject(context)) {
-      throw new TypeError(`[SharedState] State "${this.#className}": state.set(updates[, context]) should receive an object as second parameter`);
+    if (!isPlainObject(updates)) {
+      throw new TypeError(`[SharedState] State "${this.#className}": state.set(updates) should receive an object as first parameter`);
     }
 
     const newValues = {};
@@ -502,7 +497,7 @@ ${JSON.stringify(initValues, null, 2)}`);
 
     // propagate immediate params if changed
     if (propagateNow) {
-      this.#onUpdateCallbacks.forEach(listener => listener(newValues, oldValues, context));
+      this.#onUpdateCallbacks.forEach(listener => listener(newValues, oldValues));
     }
 
     // check if we can resolve immediately or if we need to go through network
@@ -521,7 +516,7 @@ ${JSON.stringify(initValues, null, 2)}`);
     // go through server-side normal behavior
     return new Promise((resolve, reject) => {
       const reqId = this[kSharedStatePromiseStore].add(resolve, reject, 'SharedState#set', forwardParams);
-      this.#client.transport.emit(`${UPDATE_REQUEST}-${this.#id}-${this.#remoteId}`, reqId, updates, context);
+      this.#client.transport.emit(`${UPDATE_REQUEST}-${this.#id}-${this.#remoteId}`, reqId, updates);
     });
   }
 
@@ -719,10 +714,10 @@ ${JSON.stringify(initValues, null, 2)}`);
    * @param {sharedStateOnUpdateCallback} callback
    *  Callback to execute when an update is applied on the state.
    * @param {Boolean} [executeListener=false] - Execute the callback immediately
-   *  with current state values. (`oldValues` will be set to `{}`, and `context` to `null`)
+   *  with current state values. Note that `oldValues` will be set to `{}`.
    * @returns {sharedStateDeleteOnUpdateCallback}
    * @example
-   * const unsubscribe = state.onUpdate(async (newValues, oldValues, context) =>  {
+   * const unsubscribe = state.onUpdate(async (newValues, oldValues) =>  {
    *   for (let [key, value] of Object.entries(newValues)) {
    *      switch (key) {
    *        // do something
@@ -738,8 +733,8 @@ ${JSON.stringify(initValues, null, 2)}`);
 
     if (executeListener === true) {
       const currentValues = this.getValues();
-      // filter `event: true` parameters from currentValues, this is missleading
-      // as we are in the context of a callback, not from an active read
+      // filter `event: true` parameters from currentValues, having them here is
+      // misleading as we are in the context of a callback, not from an active read
       const classDescription = this.getDescription();
 
       for (let name in classDescription) {
@@ -749,8 +744,7 @@ ${JSON.stringify(initValues, null, 2)}`);
       }
 
       const oldValues = {};
-      const context = null;
-      listener(currentValues, oldValues, context);
+      listener(currentValues, oldValues);
     }
 
     return () => {

src/common/logger.js

@@ -168,7 +168,7 @@ dependencies on both your server and clients.
       throw new Error(`Invalid 'logger.deprecated call: a deprecation version is required`);
     }
 
-    const msg = `[Deprecation Warning] '${oldAPI}' is deprecated since version ${deprecationVersion} and will be removed in next major revision, please use '${newAPI}' instead`;
+    const msg = `[Deprecation Warning] ${oldAPI} is deprecated (last supported version: ${deprecationVersion}) and will be removed in next major revision, please use ${newAPI} instead`;
     console.warn(chalk.yellow(msg));
   }
 };

src/server/SharedStatePrivate.js

@@ -94,15 +94,15 @@ class SharedStatePrivate {
     }
 
     // attach client listeners
-    client.transport.addListener(`${UPDATE_REQUEST}-${this.id}-${remoteId}`, async (reqId, updates, context) => {
+    client.transport.addListener(`${UPDATE_REQUEST}-${this.id}-${remoteId}`, async (reqId, updates) => {
       // apply registered hooks
       const hooks = this.#manager[kServerStateManagerGetHooks](this.className);
       const values = this.#parameters.getValues();
       let hookAborted = false;
 
       // cf. https://github.com/collective-soundworks/soundworks/issues/45
       for (let hook of hooks.values()) {
-        const result = await hook(updates, values, context);
+        const result = await hook(updates, values);
 
         if (result === null) { // explicit abort if hook returns null
           hookAborted = true;
@@ -160,7 +160,8 @@ class SharedStatePrivate {
             // no need to filter updates on requested, is blocked on client-side
             client.transport.emit(
               `${UPDATE_RESPONSE}-${this.id}-${remoteId}`,
-              reqId, acknowledgedUpdates, context,
+              reqId,
+              acknowledgedUpdates
             );
           }
 
@@ -176,7 +177,7 @@ class SharedStatePrivate {
               if (Object.keys(filteredUpdates).length > 0) {
                 peer.transport.emit(
                   `${UPDATE_NOTIFICATION}-${this.id}-${peerRemoteId}`,
-                  filteredUpdates, context,
+                  filteredUpdates,
                 );
               }
             }
@@ -187,7 +188,8 @@ class SharedStatePrivate {
             // no need to filter updates on requested, is blocked on client-side
             client.transport.emit(
               `${UPDATE_RESPONSE}-${this.id}-${remoteId}`,
-              reqId, acknowledgedUpdates, context,
+              reqId,
+              acknowledgedUpdates
             );
           }
 
@@ -202,15 +204,15 @@ class SharedStatePrivate {
               if (Object.keys(filteredUpdates).length > 0) {
                 peer.transport.emit(
                   `${UPDATE_NOTIFICATION}-${this.id}-${peerRemoteId}`,
-                  filteredUpdates, context,
+                  filteredUpdates,
                 );
               }
             }
           }
         } else {
           // propagate back to the requester that the update has been aborted
           // ignore all other attached clients.
-          client.transport.emit(`${UPDATE_ABORT}-${this.id}-${remoteId}`, reqId, updates, context);
+          client.transport.emit(`${UPDATE_ABORT}-${this.id}-${remoteId}`, reqId, updates);
         }
       } else {
         // retrieve values from inner state (also handle immediate approriately)
@@ -220,7 +222,7 @@ class SharedStatePrivate {
           oldValues[name] = this.#parameters.get(name);
         }
         // aborted by hook (updates have been overriden to {})
-        client.transport.emit(`${UPDATE_ABORT}-${this.id}-${remoteId}`, reqId, oldValues, context);
+        client.transport.emit(`${UPDATE_ABORT}-${this.id}-${remoteId}`, reqId, oldValues);
       }
     });
 

tests/misc/deprecated.spec.js

@@ -70,5 +70,16 @@ describe('# deprecated API', () => {
       server.stateManager.deleteClass('a');
       await server.stop();
     });
+
+    it('SharedState#set(updates, context)', async () => {
+      const server = new Server(config);
+      server.stateManager.defineClass('a', a);
+      await server.start();
+
+      const state = await server.stateManager.create('a');
+      state.set({}, {});
+
+      await server.stop();
+    });
   });
 });

tests/states/SharedState.spec.js

@@ -32,7 +32,7 @@ describe('# SharedState', () => {
     server.stop();
   });
 
-  describe('## async set(updates[, context]) => updates', () => {
+  describe('## async set(updates) => updates', () => {
     it('should throw if first argument is not an object', async () => {
       const a = await server.stateManager.create('a');
 
@@ -52,25 +52,6 @@ describe('# SharedState', () => {
       await a.delete();
     });
 
-    it('should throw if second argument is not an object', async () => {
-      const a = await server.stateManager.create('a');
-
-      // weird stuff with async chai, did find better solution
-      let errored = false;
-      try {
-        await a.set({}, 'fail');
-      } catch(err) {
-        console.log(err.message);
-        errored = true;
-      }
-
-      if (errored === false) {
-        assert.fail('should throw error');
-      }
-
-      await a.delete();
-    });
-
     it('should throw on undefined param name', async () => {
       const a = await server.stateManager.create('a');
 
@@ -256,15 +237,13 @@ describe('# SharedState', () => {
         const statePromise = new Promise((resolve) => {
           let step = 0;
 
-          state.onUpdate((newValues, oldValues, context) => {
+          state.onUpdate((newValues, oldValues) => {
             if (step === 0) {
               assert.deepEqual(newValues, { bool: true, int: 42 });
               assert.deepEqual(oldValues, { bool: false, int: 0 });
-              assert.deepEqual(context, null);
             } else if (step === 1) {
               assert.deepEqual(newValues, { bool: false, int: 76 });
               assert.deepEqual(oldValues, { bool: true, int: 42 });
-              assert.deepEqual(context, { someContext: true });
               resolve();
             } else {
               reject('something wrong happened');
@@ -277,15 +256,13 @@ describe('# SharedState', () => {
         const attachedPromise = new Promise((resolve) => {
           let step = 0;
 
-          attached.onUpdate((newValues, oldValues, context) => {
+          attached.onUpdate((newValues, oldValues) => {
             if (step === 0) {
               assert.deepEqual(newValues, { bool: true, int: 42 });
               assert.deepEqual(oldValues, { bool: false, int: 0 });
-              assert.deepEqual(context, null);
             } else if (step === 1) {
               assert.deepEqual(newValues, { bool: false, int: 76 });
               assert.deepEqual(oldValues, { bool: true, int: 42 });
-              assert.deepEqual(context, { someContext: true });
               resolve();
             } else {
               reject('something wrong happened');
@@ -296,7 +273,7 @@ describe('# SharedState', () => {
         });
 
         await state.set({ bool: true, int: 42 });
-        await state.set({ bool: false, int: 76 }, { someContext: true });
+        await state.set({ bool: false, int: 76 });
 
         await Promise.all([statePromise, attachedPromise]);
 
@@ -337,11 +314,10 @@ describe('# SharedState', () => {
       const a = await server.stateManager.create('a');
 
       let onUpdateCalled = false;
-      const unsubsribe = a.onUpdate((newValues, oldValues, context) => {
+      const unsubsribe = a.onUpdate((newValues, oldValues) => {
         onUpdateCalled = true;
         assert.deepEqual(newValues, { bool: false, int: 0 });
         assert.deepEqual(oldValues, {});
-        assert.deepEqual(context, null);
       }, true);
 
       await delay(10);
@@ -358,11 +334,10 @@ describe('# SharedState', () => {
       const state = await server.stateManager.create('with-event');
 
       let onUpdateCalled = false;
-      state.onUpdate((newValues, oldValues, context) => {
+      state.onUpdate((newValues, oldValues) => {
         onUpdateCalled = true;
         assert.deepEqual(newValues, { int: 20 });
         assert.deepEqual(oldValues, {});
-        assert.deepEqual(context, null);
       }, true);
 
       await delay(10);

tests/states/StateManager.spec.js

@@ -776,11 +776,10 @@ describe(`# StateManager`, () => {
     it('hook API should be `hook(updates, currentValues, context)`', async () => {
       return new Promise(async (resolve, reject) => {
         server.stateManager.defineClass('hooked', hookSchema);
-        server.stateManager.registerUpdateHook('hooked', (updates, currentValues, context) => {
+        server.stateManager.registerUpdateHook('hooked', (updates, currentValues) => {
           try {
             assert.deepEqual(updates, { name: 'test' });
             assert.deepEqual(currentValues, { name: null, value: null });
-            assert.deepEqual(context, { myContext: true });
             resolve();
           } catch(err) {
             reject(err);
@@ -790,7 +789,7 @@ describe(`# StateManager`, () => {
 
         const h = await server.stateManager.create('hooked');
 
-        await h.set({ name: 'test' }, { myContext: true });
+        await h.set({ name: 'test' });
 
         server.stateManager.deleteClass('hooked');
       });