Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Made webview state not stringified entries while preserving check to make sure it is serializable #586

Merged
merged 1 commit into from
Oct 23, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions extensions/src/hello-world/hello-world.ts
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,10 @@ const reactWebViewProvider: IWebViewProviderWithType = {
title: 'Hello World React',
content: helloWorldReactWebView,
styles: helloWorldReactWebViewStyles,
state: {
...savedWebView.state,
clicks: Math.floor(Math.random() * 100),
},
};
},
};
Expand Down
15 changes: 12 additions & 3 deletions lib/papi-dts/papi.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -197,6 +197,15 @@ declare module 'shared/utils/papi-util' {
}
/** Check that two objects are deeply equal, comparing members of each object and such */
export function deepEqual(a: unknown, b: unknown): boolean;
/**
* Check to see if the value is JSON.stringify serializable without losing information
* @param value value to test
* @returns true if serializable; false otherwise
*
* WARNING: This is inefficient right now as it stringifies, parses, and deepEquals the value.
* Please only use this if you need to
*/
export function isSerializable(value: unknown): boolean;
/** Separator between parts of a serialized request */
const REQUEST_TYPE_SEPARATOR = ':';
/** Information about a request that tells us what to do with it */
Expand Down Expand Up @@ -1766,7 +1775,7 @@ declare module 'shared/data/web-view.model' {
/** Name of the tab for the WebView */
title?: string;
/** General object to store unique state for this webview */
state?: Record<string, string>;
state?: Record<string, unknown>;
};
/** WebView representation using React */
export type WebViewDefinitionReact = WebViewDefinitionBase & {
Expand Down Expand Up @@ -1968,14 +1977,14 @@ declare module 'renderer/services/web-view-state.service' {
* @param id ID of the web view
* @returns state object of the given web view
*/
export function getFullWebViewStateById(id: string): Record<string, string>;
export function getFullWebViewStateById(id: string): Record<string, unknown>;
/**
* Set the web view state associated with the given ID
* This function is only intended to be used at startup. setWebViewState is intended for web views to call.
* @param id ID of the web view
* @param state State to set for the given web view
*/
export function setFullWebViewStateById(id: string, state: Record<string, string>): void;
export function setFullWebViewStateById(id: string, state: Record<string, unknown>): void;
/**
* Get the web view state associated with the given ID
* @param id ID of the web view
Expand Down
33 changes: 14 additions & 19 deletions src/renderer/services/web-view-state.service.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
import { isSerializable } from '@shared/utils/papi-util';

const WEBVIEW_STATE_KEY = 'web-view-state';
const stateMap = new Map<string, Record<string, string>>();
const stateMap = new Map<string, Record<string, unknown>>();
const idsLookedUp = new Set<string>();

function loadIfNeeded(): void {
Expand All @@ -9,7 +11,7 @@ function loadIfNeeded(): void {
const serializedState = localStorage.getItem(WEBVIEW_STATE_KEY);
if (!serializedState) return;

const entries: [[string, Record<string, string>]] = JSON.parse(serializedState);
const entries: [[string, Record<string, unknown>]] = JSON.parse(serializedState);
entries.forEach(([key, value]) => {
if (key && value) stateMap.set(key, value);
});
Expand All @@ -23,7 +25,7 @@ function save(): void {
localStorage.setItem(WEBVIEW_STATE_KEY, stateToSave);
}

function getRecord(id: string): Record<string, string> {
function getRecord(id: string): Record<string, unknown> {
loadIfNeeded();
idsLookedUp.add(id);

Expand All @@ -41,7 +43,7 @@ function getRecord(id: string): Record<string, string> {
* @param id ID of the web view
* @returns state object of the given web view
*/
export function getFullWebViewStateById(id: string): Record<string, string> {
export function getFullWebViewStateById(id: string): Record<string, unknown> {
if (!id) throw new Error('id must be provided to get webview state');
return getRecord(id);
}
Expand All @@ -52,7 +54,7 @@ export function getFullWebViewStateById(id: string): Record<string, string> {
* @param id ID of the web view
* @param state State to set for the given web view
*/
export function setFullWebViewStateById(id: string, state: Record<string, string>): void {
export function setFullWebViewStateById(id: string, state: Record<string, unknown>): void {
if (!id || !state) throw new Error('id and state must be provided to set webview state');
loadIfNeeded();
idsLookedUp.add(id);
Expand All @@ -68,9 +70,10 @@ export function setFullWebViewStateById(id: string, state: Record<string, string
*/
export function getWebViewStateById<T>(id: string, stateKey: string): T | undefined {
if (!id || !stateKey) throw new Error('id and stateKey must be provided to get webview state');
const state: Record<string, string> = getRecord(id);
const stateValue: string | undefined = state[stateKey];
return stateValue ? JSON.parse(stateValue) : undefined;
const state = getRecord(id);
// We don't have any way to know what type this is, so just type assert for convenience
// eslint-disable-next-line no-type-assertion/no-type-assertion
return state[stateKey] as T | undefined;
}

/**
Expand All @@ -81,19 +84,11 @@ export function getWebViewStateById<T>(id: string, stateKey: string): T | undefi
*/
export function setWebViewStateById<T>(id: string, stateKey: string, stateValue: T): void {
if (!id || !stateKey) throw new Error('id and stateKey must be provided to set webview state');
const stringifiedValue = JSON.stringify(stateValue);
try {
const roundTripped = JSON.parse(stringifiedValue);
const roundTrippedStringified = JSON.stringify(roundTripped);
if (stringifiedValue !== roundTrippedStringified) {
throw new Error(`roundtrip failure`);
}
} catch (err) {
if (!isSerializable(stateValue))
throw new Error(`"${stateKey}" value cannot round trip with JSON.stringify and JSON.parse.`);
}

const state: Record<string, string> = getRecord(id);
state[stateKey] = stringifiedValue;
const state = getRecord(id);
state[stateKey] = stateValue;
save();
}

Expand Down
2 changes: 1 addition & 1 deletion src/shared/data/web-view.model.ts
Original file line number Diff line number Diff line change
Expand Up @@ -106,7 +106,7 @@ type WebViewDefinitionBase = {
/** Name of the tab for the WebView */
title?: string;
/** General object to store unique state for this webview */
state?: Record<string, string>;
state?: Record<string, unknown>;
};

/** WebView representation using React */
Expand Down
17 changes: 17 additions & 0 deletions src/shared/services/web-view-provider.service.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ import {
import networkObjectService from '@shared/services/network-object.service';
import * as networkService from '@shared/services/network.service';
import logger from '@shared/services/logger.service';
import { isSerializable } from '@shared/utils/papi-util';

/** Suffix on network objects that indicates that the network object is a data provider */
const WEB_VIEW_PROVIDER_LABEL = 'webViewProvider';
Expand Down Expand Up @@ -74,6 +75,22 @@ async function register(
if (!webViewProvider.getWebView || typeof webViewProvider.getWebView !== 'function')
throw new Error(`WebView provider does not have a getWebView function`);

// Layer over the web view provider's getWebView method to make sure web view state is
// serializable
/** Saved bound version of the web view provider's getWebView so we can call it from here */
const wvpGetWebView = webViewProvider.getWebView.bind(webViewProvider);
webViewProvider.getWebView = async (...args) => {
const webViewDefinition = await wvpGetWebView(...args);

// If the web view provider provided a state, make sure it is serializable
if (webViewDefinition?.state && !isSerializable(webViewDefinition.state))
throw new Error(
`web-view-provider.service error: WebView Provider with type ${webViewType} responded to getWebView with a state that is not serializable`,
);

return webViewDefinition;
};

// We are good to go! Create the WebView provider

// Get the object id for this web view provider name
Expand Down
17 changes: 17 additions & 0 deletions src/shared/utils/papi-util.ts
Original file line number Diff line number Diff line change
Expand Up @@ -112,6 +112,23 @@ export function deepEqual(a: unknown, b: unknown) {
return equal(a, b);
}

/**
* Check to see if the value is JSON.stringify serializable without losing information
* @param value value to test
* @returns true if serializable; false otherwise
*
* WARNING: This is inefficient right now as it stringifies, parses, and deepEquals the value.
* Please only use this if you need to
*/
export function isSerializable(value: unknown): boolean {
try {
const valueJson = JSON.stringify(value);
return deepEqual(value, JSON.parse(valueJson));
} catch (e) {
return false;
}
}

/** Separator between parts of a serialized request */
const REQUEST_TYPE_SEPARATOR = ':';

Expand Down