Skip to content

Commit 8fb8c95

Browse files
committed
feat(web): cleanup of variable stores
This addresses code review comments in #15437 and removes some unnecessary definitions related to variable stores. It keeps the `VariableStoreSerializer` interface although currently `VariableStoreCookieSerializer` is the only implementation. Conceptually IMO it makes sense to use a more abstract interface so that in the future for example we could easily use a different store for a node implementation. Details of this change: - remove `VariableStoreDictionary` interface and replace with `VariableStore` type - remove `VarStoreSerializer` class and replace with ` CookieSerializer<VariableStore>` - also fix baseline tests to use dynamic time value and set cookies to expire in 60s instead of at some arbitrary time which might be in the past. Follow-up-of: #15437 Test-bot: skip
1 parent 2e40439 commit 8fb8c95

File tree

7 files changed

+19
-44
lines changed

7 files changed

+19
-44
lines changed

web/src/engine/src/js-processor/jsKeyboardInterface.ts

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,6 @@ import {
2222
type KeyEvent,
2323
type TextStore,
2424
VariableStore,
25-
VariableStoreDictionary,
2625
VariableStoreSerializer,
2726
} from "keyman/engine/keyboard";
2827
import { PlatformSystemStore } from './platformSystemStore.js';
@@ -1103,7 +1102,7 @@ export class JSKeyboardInterface extends KeyboardHarness {
11031102
* @param stores A dictionary of stores which should be found in the
11041103
* keyboard
11051104
*/
1106-
applyVariableStores(stores: VariableStoreDictionary): void {
1105+
applyVariableStores(stores: VariableStore): void {
11071106
this.activeKeyboard.variableStores = stores;
11081107
}
11091108

web/src/engine/src/keyboard/index.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -30,7 +30,7 @@ export { EmulationKeystrokes, LogMessages, DefaultRules } from "./defaultRules.j
3030
export { type KeyDistribution, KeyEventSpec, KeyEvent } from "./keyEvent.js";
3131
export { KeyMapping } from "./keyMapping.js";
3232
export { type SystemStoreMutationHandler, MutableSystemStore, SystemStore, SystemStoreIDs, type SystemStoreDictionary } from "./systemStore.js";
33-
export { type VariableStore, VariableStoreSerializer, VariableStoreDictionary } from "./variableStore.js";
33+
export { type VariableStore, VariableStoreSerializer } from "./variableStore.js";
3434

3535
export { DOMKeyboardLoader } from './keyboards/loaders/domKeyboardLoader.js';
3636
export { SyntheticTextStore } from "./syntheticTextStore.js";

web/src/engine/src/keyboard/keyboards/jsKeyboard.ts

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@ import { ActiveKey, ActiveLayout, ActiveSubKey } from "./activeLayout.js";
44
import { KeyEvent } from "../keyEvent.js";
55
import { type TextStore } from "../textStore.js";
66
import { KeymanWebKeyboard, ModifierKeyConstants, TouchLayout } from "@keymanapp/common-types";
7-
import { VariableStoreDictionary } from "../variableStore.js";
7+
import { VariableStore } from "../variableStore.js";
88

99
import ComplexKeyboardStore = KeymanWebKeyboard.ComplexKeyboardStore;
1010
import KeyboardObject = KeymanWebKeyboard.KeyboardObject;
@@ -120,9 +120,9 @@ export class JSKeyboard {
120120
*
121121
* @returns an object with each property referencing a variable store
122122
*/
123-
get variableStores(): VariableStoreDictionary {
123+
get variableStores(): VariableStore {
124124
const storeNames = this.scriptObject['KVS'];
125-
let values: VariableStoreDictionary = {};
125+
let values: VariableStore = {};
126126
if(Array.isArray(storeNames)) {
127127
for(let store of storeNames) {
128128
values[store] = this.scriptObject[store];
@@ -139,7 +139,7 @@ export class JSKeyboard {
139139
*
140140
* @param values name-value pairs for each store value
141141
*/
142-
set variableStores(values: VariableStoreDictionary) {
142+
set variableStores(values: VariableStore) {
143143
const storeNames = this.scriptObject['KVS'];
144144
if(Array.isArray(storeNames)) {
145145
for(let store of storeNames) {

web/src/engine/src/keyboard/keyboards/processorAction.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@
33
*/
44
import { LexicalModelTypes } from '@keymanapp/common-types';
55
import { Transcription } from './transcription.js';
6-
import { VariableStore, VariableStoreDictionary } from '../variableStore.js';
6+
import { VariableStore } from '../variableStore.js';
77
import { SystemStoreDictionary } from '../systemStore.js';
88

99
/**
@@ -34,7 +34,7 @@ export class ProcessorAction {
3434
/**
3535
* A set of variable stores with possible changes to be applied during finalization.
3636
*/
37-
variableStores: VariableStoreDictionary = {};
37+
variableStores: VariableStore = {};
3838

3939
/**
4040
* Denotes a non-output default behavior; this should be evaluated later, against the true keystroke.

web/src/engine/src/keyboard/variableStore.ts

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -4,10 +4,6 @@
44
* Definitions for variable stores (see kmn reference)
55
*/
66

7-
export interface VariableStoreDictionary {
8-
[name: string]: string;
9-
};
10-
117
export type VariableStore = { [name: string]: string; };
128

139
export interface VariableStoreSerializer {

web/src/engine/src/main/variableStoreCookieSerializer.ts

Lines changed: 10 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -8,42 +8,20 @@ import { CookieSerializer } from "keyman/engine/dom-utils";
88
// Also of note: there's nothing we can do to allow TS to provide type-checking of
99
// dynamic property names; they'd have to be known at compile time to facilitate
1010
// strict type checking.
11-
class VarStoreSerializer extends CookieSerializer<VariableStore> {
12-
public constructor(keyboardID: string, storeName: string) {
13-
super(`KeymanWeb_${keyboardID}_Option_${storeName}`);
14-
}
15-
16-
public load(): VariableStore {
17-
return super.load(decodeURIComponent);
18-
}
19-
20-
public save(storeMap: VariableStore) {
21-
super.save(storeMap, encodeURIComponent);
22-
}
2311

24-
/**
25-
* Find all variable stores associated with a given keyboard.
26-
*
27-
* @param {string} keyboardID The keyboard ID whose variable stores are to be found.
28-
*
29-
* @returns An array of VariableStore objects found for the keyboard.
30-
*/
31-
public static findStores(keyboardID: string): VariableStore[] {
32-
const pattern = new RegExp(`^KeymanWeb_${keyboardID}_Option_`);
33-
const matching = CookieSerializer.loadAllMatching<VariableStore>(pattern, decodeURIComponent);
34-
return matching.map(m => m.value);
12+
export class VariableStoreCookieSerializer implements VariableStoreSerializer {
13+
private getStoreCookieName(keyboardID: string, storeName: string): string {
14+
return `KeymanWeb_${keyboardID}_Option_${storeName}`;
3515
}
36-
}
3716

38-
export class VariableStoreCookieSerializer implements VariableStoreSerializer {
3917
public loadStore(keyboardID: string, storeName: string): VariableStore {
40-
const storeCookieSerializer = new VarStoreSerializer(keyboardID, storeName);
41-
return storeCookieSerializer.load();
18+
const storeCookieSerializer = new CookieSerializer<VariableStore>(this.getStoreCookieName(keyboardID, storeName));
19+
return storeCookieSerializer.load(decodeURIComponent);
4220
}
4321

4422
public saveStore(keyboardID: string, storeName: string, storeMap: VariableStore) {
45-
const storeCookieSerializer = new VarStoreSerializer(keyboardID, storeName);
46-
storeCookieSerializer.save(storeMap);
23+
const storeCookieSerializer = new CookieSerializer<VariableStore>(this.getStoreCookieName(keyboardID, storeName));
24+
storeCookieSerializer.save(storeMap, encodeURIComponent);
4725
}
4826

4927
/**
@@ -54,6 +32,8 @@ export class VariableStoreCookieSerializer implements VariableStoreSerializer {
5432
* @returns An array of VariableStore objects found for the keyboard.
5533
*/
5634
public findStores(keyboardID: string): VariableStore[] {
57-
return VarStoreSerializer.findStores(keyboardID);
35+
const pattern = new RegExp(`^${this.getStoreCookieName(keyboardID, '')}`);
36+
const matching = CookieSerializer.loadAllMatching<VariableStore>(pattern, decodeURIComponent);
37+
return matching.map(m => m.value);
5838
}
5939
}

web/src/test/auto/e2e/baseline/baseline.tests.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -140,7 +140,7 @@ test.describe('Baseline tests', () => {
140140
value: encodeURIComponent(`${option.key}=${option.value};`),
141141
domain: 'localhost',
142142
path: '/',
143-
expires: 1768672167
143+
expires: Date.now() / 1000 + 60, // expire in 1 minute
144144
});
145145
}
146146
}

0 commit comments

Comments
 (0)