feat(extension): optimistically apply local config updates

Author: sqs

src/extension/features/configuration.test.ts

@@ -9,7 +9,7 @@ import {
 } from '../../protocol'
 import { Configuration, Observable } from '../api'
 import { observableValue } from '../util'
-import { createExtConfiguration } from './configuration'
+import { createExtConfiguration, setValueAtKeyPath } from './configuration'
 
 interface Settings {
     [key: string]: string
@@ -41,16 +41,81 @@ describe('ExtConfiguration', () => {
     })
 
     describe('update', () => {
-        it('sends to the client', async () => {
+        it('sends to the client and immediately reflects locally', async () => {
             const { extConfiguration, mockConnection } = create()
             mockConnection.mockResults.set(ConfigurationUpdateRequest.type.method, void 0)
-            await extConfiguration.update('a', 'b')
+            const updated = extConfiguration.update('a', 'b')
+            const want = { a: 'b' } as Settings
+            assert.deepStrictEqual(observableValue(extConfiguration), want)
+            await updated
+            assert.deepStrictEqual(observableValue(extConfiguration), want)
             assert.deepStrictEqual(mockConnection.sentMessages, [
                 {
                     method: ConfigurationUpdateRequest.type.method,
                     params: { path: ['a'], value: 'b' } as ConfigurationUpdateParams,
                 },
             ])
         })
+
+        it('handles interleaved update calls and didChangeConfiguration notifications', async () => {
+            const { extConfiguration, mockConnection } = create()
+            mockConnection.mockResults.set(ConfigurationUpdateRequest.type.method, void 0)
+            const updated = extConfiguration.update('a', 'b')
+            assert.deepStrictEqual(observableValue(extConfiguration), { a: 'b' } as Settings)
+            mockConnection.recvNotification(DidChangeConfigurationNotification.type.method, {
+                configurationCascade: { merged: { c: 'd' } as Settings },
+            } as DidChangeConfigurationParams)
+            assert.deepStrictEqual(observableValue(extConfiguration), { c: 'd' } as Settings)
+            await updated
+            assert.deepStrictEqual(observableValue(extConfiguration), { c: 'd' } as Settings)
+        })
+    })
+})
+
+describe('setValueAtKeyPath', () => {
+    it('overwrites the top level', () => assert.deepStrictEqual(setValueAtKeyPath({ a: 1 }, [], { b: 2 }), { b: 2 }))
+    it('overwrites an existing property', () => assert.deepStrictEqual(setValueAtKeyPath({ a: 1 }, ['a'], 2), { a: 2 }))
+    it('sets a new property', () => assert.deepStrictEqual(setValueAtKeyPath({ a: 1 }, ['b'], 2), { a: 1, b: 2 }))
+    it('sets a property overwriting an array', () => assert.deepStrictEqual(setValueAtKeyPath([1], ['a'], 2), { a: 2 }))
+    it('sets a property overwriting a primitive', () =>
+        assert.deepStrictEqual(setValueAtKeyPath(1 as any, ['a'], 2), { a: 2 }))
+    it('overwrites an existing nested property', () =>
+        assert.deepStrictEqual(setValueAtKeyPath({ a: { b: 1 } }, ['a', 'b'], 2), { a: { b: 2 } }))
+    it('deletes a property', () =>
+        assert.deepStrictEqual(setValueAtKeyPath({ a: 1, b: 2 }, ['a'], undefined), { b: 2 }))
+    it('sets a new nested property', () =>
+        assert.deepStrictEqual(setValueAtKeyPath({ a: { b: 1 } }, ['a', 'c'], 2), { a: { b: 1, c: 2 } }))
+    it('sets a new deeply nested property', () =>
+        assert.deepStrictEqual(setValueAtKeyPath({ a: { b: { c: 1 } } }, ['a', 'b', 'd'], 2), {
+            a: { b: { c: 1, d: 2 } },
+        }))
+    it('overwrites an object', () => assert.deepStrictEqual(setValueAtKeyPath({ a: { b: 1 } }, ['a'], 2), { a: 2 }))
+    it('sets a value that requires a new object', () =>
+        assert.deepStrictEqual(setValueAtKeyPath({}, ['a', 'b'], 1), { a: { b: 1 } }))
+
+    it('overwrites an existing index', () => assert.deepStrictEqual(setValueAtKeyPath([1], [0], 2), [2]))
+    it('inserts a new index', () => assert.deepStrictEqual(setValueAtKeyPath([1], [1], 2), [1, 2]))
+    it('inserts a new index at end', () => assert.deepStrictEqual(setValueAtKeyPath([1, 2], [-1], 3), [1, 2, 3]))
+    it('inserts an index overwriting an object', () => assert.deepStrictEqual(setValueAtKeyPath({ a: 1 }, [0], 2), [2]))
+    it('inserts an index overwriting a primitive', () =>
+        assert.deepStrictEqual(setValueAtKeyPath(1 as any, [0], 2), [2]))
+    it('overwrites an existing nested index', () =>
+        assert.deepStrictEqual(setValueAtKeyPath([1, [2]], [1, 0], 3), [1, [3]]))
+    it('deletes an index', () => assert.deepStrictEqual(setValueAtKeyPath([1, 2, 3], [1], undefined), [1, 3]))
+    it('sets a new nested index', () =>
+        assert.deepStrictEqual(setValueAtKeyPath([1, [1, 2, [1, 2, 3, 4]]], [1, 2, 3], 5), [1, [1, 2, [1, 2, 3, 5]]]))
+    it('inserts a new nested index at end', () =>
+        assert.deepStrictEqual(setValueAtKeyPath([1, [2]], [1, -1], 3), [1, [2, 3]]))
+    it('overwrites an array', () => assert.deepStrictEqual(setValueAtKeyPath([1, [2]], [1], 3), [1, 3]))
+    it('sets a value that requires a new array', () => assert.deepStrictEqual(setValueAtKeyPath([], [0, 0], 1), [[1]]))
+
+    it('sets a nested property (and does not modify input)', () => {
+        const input = { a: [{}, { b: [1, 2] }] }
+        const origInput = JSON.parse(JSON.stringify(input))
+        assert.deepStrictEqual(setValueAtKeyPath(input, ['a', 1, 'b', -1], { c: 3 }), {
+            a: [{}, { b: [1, 2, { c: 3 }] }],
+        })
+        assert.deepStrictEqual(input, origInput)
     })
+    it('throws on invalid key type', () => assert.throws(() => setValueAtKeyPath({}, [true as any], {})))
 })

src/extension/features/configuration.ts

@@ -5,6 +5,7 @@ import {
     ConfigurationUpdateParams,
     ConfigurationUpdateRequest,
     DidChangeConfigurationNotification,
+    KeyPath,
 } from '../../protocol'
 import { isEqual } from '../../util'
 import { Configuration, CXP, Observable } from '../api'
@@ -27,16 +28,82 @@ class ExtConfiguration<C> extends BehaviorSubject<C> implements Configuration<C>
     }
 
     public update<K extends keyof C>(key: K, value: C[K] | undefined): Promise<void> {
-        // TODO!(sqs): update config locally here
+        const path: KeyPath = [key as string | number]
+
+        // Optimistically apply configuration update locally. If this diverges from the server's state, a
+        // subsequent didChangeConfiguration notification will inform us.
+        const cur = this.value
+        this.next(setValueAtKeyPath(cur, path, value))
+
         return this.ext.rawConnection.sendRequest(ConfigurationUpdateRequest.type, {
-            path: [key],
+            path,
             value,
         } as ConfigurationUpdateParams)
     }
 
     public readonly [Symbol.observable] = () => this
 }
 
+/**
+ * Returns the source object with the given value inserted in the location specified by the key path. The source
+ * object is not modified. The key path indexes into the object successively for each element in the key path.
+ *
+ * If the value is `undefined`, the value at the key path is removed.
+ *
+ * This must behave identically to {@link module:jsonc-parser.setProperty}.
+ */
+export function setValueAtKeyPath(source: any, path: KeyPath, value: any): any {
+    if (path.length === 0) {
+        // Overwrite entire value.
+        return value
+    }
+
+    const root = [source]
+    let prev: any = root // maintain an lvalue that we can assign to
+    for (const [i, key] of path.entries()) {
+        const last = i === path.length - 1
+        const prevKey = i === 0 ? 0 : path[i - 1]
+        if (typeof key === 'string') {
+            if (last) {
+                if (value === undefined) {
+                    prev[prevKey] = { ...prev[prevKey] }
+                    delete prev[prevKey][key]
+                } else if (
+                    prev[prevKey] !== null &&
+                    typeof prev[prevKey] === 'object' &&
+                    !Array.isArray(prev[prevKey])
+                ) {
+                    prev[prevKey] = { ...prev[prevKey], [key]: value }
+                } else {
+                    prev[prevKey] = { [key]: value }
+                }
+            } else {
+                prev[prevKey] =
+                    prev[prevKey] !== null && typeof prev[prevKey] === 'object' && !Array.isArray(prev[prevKey])
+                        ? { ...prev[prevKey] }
+                        : {}
+            }
+        } else if (typeof key === 'number') {
+            if (last) {
+                const index = key === -1 ? prev[prevKey].length : key
+                const head = Array.isArray(prev[prevKey]) ? prev[prevKey].slice(0, index) : []
+                const tail = Array.isArray(prev[prevKey]) ? prev[prevKey].slice(index + 1) : []
+                if (value === undefined) {
+                    prev[prevKey] = [...head, ...tail]
+                } else {
+                    prev[prevKey] = [...head, value, ...tail]
+                }
+            } else {
+                prev[prevKey] = Array.isArray(prev[prevKey]) ? [...prev[prevKey]] : []
+            }
+        } else {
+            throw new Error(`invalid key in key path: ${key} (full key path: ${JSON.stringify(path)}`)
+        }
+        prev = prev[prevKey]
+    }
+    return root[0]
+}
+
 /**
  * Creates the CXP extension API's {@link CXP#configuration} value.
  *