Support synchronising extension's global state

[sandy081]

Support synchronising extension's global state

[bwateratmsft]

For Docker extension, we'd need to be able to sync on a per-key basis. There are some areas where sync would be nice to have (nothing imperative, right now), but there are some where we cannot sync (e.g. machine-specific information that we want to cache forever because it's expensive to compute but never changes).

[legomushroom]

This could be useful for Live Share to sync up the chat history (currently stored locally for single machine only) cc @jramsay @fubaduba

[sandy081]

Proposal:

Provide a contribution point for the extension to register memento keys to sync

"contributes": {
		"settingsSync.memento": {
			"global": [
				"key1"
			]
		}
}

[bwateratmsft]

I like that plan. We do have some keys in the Docker extension that are determined at runtime (thus couldn't be in package.json), but I don't think any of them need synchronization.

[jrieken]

export interface Memento {

		/**
		 * Return a value.
		 *
		 * @param key A string.
		 * @return The stored value or `undefined`.
		 */
		get<T>(key: string): T | undefined;

		/**
		 * Return a value.
		 *
		 * @param key A string.
		 * @param defaultValue A value that should be returned when there is no
		 * value (`undefined`) with the given key.
		 * @return The stored value or the defaultValue.
		 */
		get<T>(key: string, defaultValue: T): T;

		/**
		 * Store a value. The value must be JSON-stringifyable.
		 *
		 * @param key A string.
		 * @param value A value. MUST not contain cyclic references.
		 */
		update(key: string, value: any, includeInSettingsSync?: boolean): Thenable<void>;

		keys(): ReadonlyArray<string>;

		onDidChange: Event<{ readonly keys: ReadonlyArray<string> }>;
	}

[jrieken]

Alternative proposal avoiding update-sync-not-sync confusion and very simple migration.

export interface Memento {

		/**
		 * Return a value.
		 *
		 * @param key A string.
		 * @return The stored value or `undefined`.
		 */
		get<T>(key: string): T | undefined;

		/**
		 * Return a value.
		 *
		 * @param key A string.
		 * @param defaultValue A value that should be returned when there is no
		 * value (`undefined`) with the given key.
		 * @return The stored value or the defaultValue.
		 */
		get<T>(key: string, defaultValue: T): T;

		/**
		 * Store a value. The value must be JSON-stringifyable.
		 *
		 * @param key A string.
		 * @param value A value. MUST not contain cyclic references.
		 */
		update(key: string, value: any): Thenable<void>;

		/**
		 *
		 */
		syncedKeys: string[];

		onDidChange: Event<{ readonly keys: ReadonlyArray<string> }>;
	}

[legomushroom]

@bwateratmsft agreed! But that is a different feature - we need context properties for the settings just like commands have :) Worths creating an issue for that if there isn't one yet 😊

[bwateratmsft]

@legomushroom I'm not sure I follow. This issue is about using settings sync to synchronize global state (mementos), right? I'm not sure commands are related.

[sandy081]

Added following proposed API

export interface ExtensionContext {

	readonly syncedGlobalState: Memento & {
		/**
		 * List of keys whose values should be synced across devices when extensions synchronization is enabled .
		 * Set synced keys to an empty array to unset the synced state.
		 */
		syncedKeys: ReadonlyArray<string>;
	};

}

Note: syncedGlobalState will be removed and merged with globalState in `ExtensionContext once finalized

@jrieken let me know if there are any suggestions.

[jrieken]

Note: syncedGlobalState will be removed and merged with globalState in `ExtensionContext once finalized

Is having SyncedMemento only because of type-gymnastics or do you really want to introduce a new type? Also, what happened to the event? Are we still planning an adding that?

[sandy081]

Is having SyncedMemento only because of type-gymnastics or do you really want to introduce a new type?

Mainly to support syncedKeys only for globalState. As discussed in stand up, I will inline it as follows once finalised.

readonly globalState: Memento & {
	/**
	 * List of keys whose values should be synced across devices when extensions synchronization is enabled .
	 * Set synced keys to an empty array to unset the synced state.
	*/
	syncedKeys: ReadonlyArray<string>;
};

Edit: Also inlined in the proposed API

Also, what happened to the event? Are we still planning an adding that?

Planning to add the event later.

[bwateratmsft]

This would still include the change to package.json to specify which keys to sync, right?

[sandy081]

This would still include the change to package.json to specify which keys to sync, right?

package.json contribution is not needed and redundant. Extension shall update the keys using the above API.

[bwateratmsft]

If it's read-only, how do they update it?

[sandy081]

If it's read-only, how do they update it?

syncedKeys is not read-only. You can update it by setting the new value.

[bwateratmsft]

You have it as a ReadonlyArray<string> in your proposed API. Should it actually be readonly string[], which would allow adding/removing elements but not replacing the entire array?

Or are we just approaching this from opposite directions, I'm thinking you add/remove elements, you're thinking you replace the whole array?

[sandy081]

I do not think there is an easy way to listen to changes when elements are added or removed from an array. Hence went with updating the entire array for updates.

[bwateratmsft]

Good point, that makes sense. 👍

[sandy081]

Finalised following:

export interface ExtensionContext {
	readonly syncedGlobalState: Memento & {
		/**
		 * Set the keys whose values should be synced across devices when extensions synchronization is enabled .
		 */
		setKeysForSync(keys: string[]): void;
	};
}```

[RandomFractals]

this is great! please link to this issue in your vscode v1.51 update docs, or to better docs on this ext. state sync feature since the api link in that section is circular atm: https://code.visualstudio.com/updates/v1_51#_settings-sync

should probably be linked to this: https://code.visualstudio.com/updates/v1_51#_sync-global-state