From 7b5a5fcbc1ebd4f94d891e8b4c51db34f091e97a Mon Sep 17 00:00:00 2001 From: Kevin Hawkins Date: Thu, 12 Oct 2023 11:39:45 -0700 Subject: [PATCH 1/7] Change to a two-way async messaging between webviews and controller pages --- resources/instructions/briefcase.html | 8 ++- resources/instructions/landingpage.html | 44 +++++++++------ .../projectBootstrapAcknowledgment.html | 8 ++- .../instructions/projectBootstrapChoice.html | 10 +++- .../instructions/salesforcemobileapp.html | 8 ++- resources/instructions/webviewMessaging.js | 54 ++++++++++++------- src/commands/wizard/authorizeCommand.ts | 2 +- .../wizard/configureProjectCommand.ts | 6 +-- src/webviews.ts | 54 +++++++++++++++---- 9 files changed, 139 insertions(+), 55 deletions(-) diff --git a/resources/instructions/briefcase.html b/resources/instructions/briefcase.html index 05e1e9fa..59e480c7 100644 --- a/resources/instructions/briefcase.html +++ b/resources/instructions/briefcase.html @@ -1,4 +1,4 @@ - + @@ -20,6 +20,12 @@ If you've already configured a briefcase, move on to the next step.

+ diff --git a/resources/instructions/landingpage.html b/resources/instructions/landingpage.html index 49b6cc80..fdf87cc0 100644 --- a/resources/instructions/landingpage.html +++ b/resources/instructions/landingpage.html @@ -1,19 +1,29 @@ - + + + + + Landing Page Customization + - - - - Landing Page Customization - - - -

A default landing page file (landing_page.json) has been created in the staticresources directory. - See Customize The Landing Page - for more information on how to modify the landing page file. -

- - - - - \ No newline at end of file + +

+ A default landing page file (landing_page.json) has been created in + the staticresources directory. See + + Customize The Landing Page + + for more information on how to modify the landing page file. +

+ + + + + diff --git a/resources/instructions/projectBootstrapAcknowledgment.html b/resources/instructions/projectBootstrapAcknowledgment.html index 27fcfb47..6838bf96 100644 --- a/resources/instructions/projectBootstrapAcknowledgment.html +++ b/resources/instructions/projectBootstrapAcknowledgment.html @@ -1,4 +1,4 @@ - + @@ -13,6 +13,12 @@ time to resume.

+ diff --git a/resources/instructions/projectBootstrapChoice.html b/resources/instructions/projectBootstrapChoice.html index 6245e2cc..c98f266c 100644 --- a/resources/instructions/projectBootstrapChoice.html +++ b/resources/instructions/projectBootstrapChoice.html @@ -1,4 +1,4 @@ - + @@ -14,6 +14,14 @@

Welcome to the Offline App Onboarding Wizard

+ diff --git a/resources/instructions/salesforcemobileapp.html b/resources/instructions/salesforcemobileapp.html index 6a2008e4..a32a3eb7 100644 --- a/resources/instructions/salesforcemobileapp.html +++ b/resources/instructions/salesforcemobileapp.html @@ -1,4 +1,4 @@ - + @@ -21,6 +21,12 @@ own.

+ diff --git a/resources/instructions/webviewMessaging.js b/resources/instructions/webviewMessaging.js index f82c592e..4305df63 100644 --- a/resources/instructions/webviewMessaging.js +++ b/resources/instructions/webviewMessaging.js @@ -5,23 +5,39 @@ * For full license text, see the LICENSE file in the repo root or https://opensource.org/licenses/MIT */ -const vscode = acquireVsCodeApi(); +// ------------------ +// Callback Messaging +// ------------------ +// +// Async, callback-based messaging mechanism for clients +// +const webviewMessaging = (function () { + const vscode = acquireVsCodeApi(); + let requestId = 0; + const asyncMessageCallbacks = {}; -// Set up button event handlers and message passing. -const buttons = document.getElementsByTagName("button"); -if (!buttons || buttons.length === 0) { - console.error("No buttons found! No event handlers will be created."); -} else { - for (const button of buttons) { - const buttonId = button.getAttribute("id"); - if (!buttonId) { - console.error( - "Button has no id value! No event handler will be created." - ); - } else { - button.addEventListener("click", () => { - vscode.postMessage({ button: buttonId }); - }); - } - } -} + window.addEventListener('message', (event) => { + const message = event.data; + if (message.callbackId && asyncMessageCallbacks[message.callbackId]) { + const callback = asyncMessageCallbacks[message.callbackId]; + delete asyncMessageCallbacks[message.callbackId]; + delete message.callbackId; + callback(message); + } + }); + + return { + sendMessageRequest: function (type, data, callback) { + let message; + if (callback) { + const asyncMessageRequestId = requestId++; + asyncMessageCallbacks[asyncMessageRequestId] = callback; + + message = { type, callbackId: asyncMessageRequestId, ...data }; + } else { + message = { type, ...data }; + } + vscode.postMessage(message); + } + }; +})(); diff --git a/src/commands/wizard/authorizeCommand.ts b/src/commands/wizard/authorizeCommand.ts index bcf7a4d4..dc5e3570 100644 --- a/src/commands/wizard/authorizeCommand.ts +++ b/src/commands/wizard/authorizeCommand.ts @@ -24,7 +24,7 @@ export class AuthorizeCommand { if (!result || result.title === l10n.t('No')) { return Promise.resolve(false); } else { - await commands.executeCommand('sfdx.force.auth.web.login'); + await commands.executeCommand('sfdx.org.login.web'); await window.showInformationMessage( l10n.t( "Once you've authorized your Org, click here to continue." diff --git a/src/commands/wizard/configureProjectCommand.ts b/src/commands/wizard/configureProjectCommand.ts index 86ee0ea5..9a9bf56a 100644 --- a/src/commands/wizard/configureProjectCommand.ts +++ b/src/commands/wizard/configureProjectCommand.ts @@ -41,7 +41,7 @@ export class DefaultProjectConfigurationProcessor 'resources/instructions/projectBootstrapAcknowledgment.html', [ { - buttonId: 'okButton', + type: 'okButton', action: async (panel) => { panel.dispose(); return resolve(); @@ -98,13 +98,13 @@ export class DefaultProjectConfigurationProcessor 'resources/instructions/projectBootstrapChoice.html', [ { - buttonId: 'createNewButton', + type: 'createNewButton', action: (panel) => { createChoice(panel); } }, { - buttonId: 'openExistingButton', + type: 'openExistingButton', action: (panel) => { openChoice(panel); } diff --git a/src/webviews.ts b/src/webviews.ts index d5dfb83d..11da3772 100644 --- a/src/webviews.ts +++ b/src/webviews.ts @@ -12,9 +12,15 @@ export const MESSAGING_SCRIPT_PATH_DEMARCATOR = '--- MESSAGING_SCRIPT_SRC ---'; export const MESSAGING_JS_PATH = 'resources/instructions/webviewMessaging.js'; const INSTRUCTION_VIEW_TYPE = 'instructionsView'; -export type ButtonAction = { - buttonId: string; - action: (panel: vscode.WebviewPanel) => void; +export type WebviewMessageCallback = (responseData?: object) => void; + +export type WebviewMessageHandler = { + type: string; + action: ( + panel: vscode.WebviewPanel, + data?: object, + callback?: WebviewMessageCallback + ) => void; }; export class InstructionsWebviewProvider { @@ -27,8 +33,9 @@ export class InstructionsWebviewProvider { public showInstructionWebview( title: string, contentPath: string, - buttonActions: ButtonAction[] + messageHandlers: WebviewMessageHandler[] ) { + this.validateMessageHanders(messageHandlers); const panel = vscode.window.createWebviewPanel( INSTRUCTION_VIEW_TYPE, title, @@ -40,12 +47,24 @@ export class InstructionsWebviewProvider { ); panel.webview.onDidReceiveMessage((data) => { - const clickedButtonId = data.button; - const buttonAction = buttonActions.find((action) => { - return action.buttonId === clickedButtonId; - }); - if (buttonAction) { - buttonAction.action(panel); + const responsiveHandlers = messageHandlers.filter( + (messageHandler) => data.type === messageHandler.type + ); + if (responsiveHandlers.length > 0) { + const handler = responsiveHandlers[0]; + let callback: WebviewMessageCallback | undefined; + if (data.callbackId) { + const returnedCallbackId = data.callbackId; + delete data.callbackId; + callback = (responseData?: object) => { + const fullResponseMessage = { + callbackId: returnedCallbackId, + ...responseData + }; + panel.webview.postMessage(fullResponseMessage); + }; + } + handler.action(panel, data, callback); } }); @@ -82,7 +101,7 @@ export class InstructionsWebviewProvider { new InstructionsWebviewProvider(extensionUri); provider.showInstructionWebview(title, contentPath, [ { - buttonId: 'okButton', + type: 'okButton', action: (panel) => { panel.dispose(); return resolve(); @@ -119,4 +138,17 @@ export class InstructionsWebviewProvider { return contentPath; } } + + private validateMessageHanders(messageHandlers: WebviewMessageHandler[]) { + const handlerMap: { [type: string]: boolean } = {}; + for (const handler of messageHandlers) { + if (handlerMap[handler.type] === true) { + throw new Error( + `There can be only one message handler per type. There are at least two handlers with type '${handler.type}'.` + ); + } else { + handlerMap[handler.type] = true; + } + } + } } From 165e16b29a888c1fca59d5af63b2479324d02abe Mon Sep 17 00:00:00 2001 From: Kevin Hawkins Date: Sun, 15 Oct 2023 22:44:04 -0700 Subject: [PATCH 2/7] Callback ID needs to start with a non-zero value --- resources/instructions/webviewMessaging.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/resources/instructions/webviewMessaging.js b/resources/instructions/webviewMessaging.js index 4305df63..2e24a350 100644 --- a/resources/instructions/webviewMessaging.js +++ b/resources/instructions/webviewMessaging.js @@ -30,7 +30,7 @@ const webviewMessaging = (function () { sendMessageRequest: function (type, data, callback) { let message; if (callback) { - const asyncMessageRequestId = requestId++; + const asyncMessageRequestId = ++requestId; asyncMessageCallbacks[asyncMessageRequestId] = callback; message = { type, callbackId: asyncMessageRequestId, ...data }; From 18e589da314fd3f3b236b7ed47075c2b321f8bfa Mon Sep 17 00:00:00 2001 From: Kevin Hawkins Date: Wed, 18 Oct 2023 16:55:19 -0700 Subject: [PATCH 3/7] Adding comments --- resources/instructions/webviewMessaging.js | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/resources/instructions/webviewMessaging.js b/resources/instructions/webviewMessaging.js index 2e24a350..ede38110 100644 --- a/resources/instructions/webviewMessaging.js +++ b/resources/instructions/webviewMessaging.js @@ -16,6 +16,10 @@ const webviewMessaging = (function () { let requestId = 0; const asyncMessageCallbacks = {}; + // Receives messages from the backing TypeScript controller page that + // created the hosted webview. These messages will be linked back to + // originating requests, passing any response data back to the async + // caller. window.addEventListener('message', (event) => { const message = event.data; if (message.callbackId && asyncMessageCallbacks[message.callbackId]) { @@ -27,6 +31,16 @@ const webviewMessaging = (function () { }); return { + /** + * Sends a message request to the backing TypeScript controller page that + * created the hosted webview. + * @param {string} type - A name representing the type of request. Basically + * the event key to which the controller page will subscribe. + * @param {object} [data] - An optional block of input data to pass to the + * controller. + * @param {Function} [callback] - An optional callback for receiving a + * response from the controller, if expected. + */ sendMessageRequest: function (type, data, callback) { let message; if (callback) { From 8c30a25c64e2a2224bf20636ff0780244bb8998f Mon Sep 17 00:00:00 2001 From: Kevin Hawkins Date: Mon, 23 Oct 2023 14:51:06 -0700 Subject: [PATCH 4/7] Starting refactoring for better testability of webviews logic --- src/commands/wizard/briefcaseCommand.ts | 2 +- src/commands/wizard/configureProjectCommand.ts | 2 +- src/commands/wizard/onboardingWizard.ts | 2 +- src/commands/wizard/templateChooserCommand.ts | 2 +- src/test/suite/commands/wizard/briefcaseCommand.test.ts | 2 +- src/test/suite/webviews.test.ts | 2 +- src/{webviews.ts => webviews/instructions.ts} | 0 7 files changed, 6 insertions(+), 6 deletions(-) rename src/{webviews.ts => webviews/instructions.ts} (100%) diff --git a/src/commands/wizard/briefcaseCommand.ts b/src/commands/wizard/briefcaseCommand.ts index f4028fb6..18efefa4 100644 --- a/src/commands/wizard/briefcaseCommand.ts +++ b/src/commands/wizard/briefcaseCommand.ts @@ -7,7 +7,7 @@ import { ProgressLocation, Uri, window, l10n } from 'vscode'; import { CommonUtils } from '@salesforce/lwc-dev-mobile-core/lib/common/CommonUtils'; -import { InstructionsWebviewProvider } from '../../webviews'; +import { InstructionsWebviewProvider } from '../../webviews/instructions'; export class BriefcaseCommand { static readonly OPEN_ORG_BRIEFCASE_PAGE_CMD = diff --git a/src/commands/wizard/configureProjectCommand.ts b/src/commands/wizard/configureProjectCommand.ts index 9a9bf56a..fd320b7f 100644 --- a/src/commands/wizard/configureProjectCommand.ts +++ b/src/commands/wizard/configureProjectCommand.ts @@ -8,7 +8,7 @@ import { Uri, WebviewPanel, commands, l10n, window } from 'vscode'; import * as process from 'process'; import { CommonUtils } from '@salesforce/lwc-dev-mobile-core/lib/common/CommonUtils'; -import { InstructionsWebviewProvider } from '../../webviews'; +import { InstructionsWebviewProvider } from '../../webviews/instructions'; export type ProjectManagementChoiceAction = (panel?: WebviewPanel) => void; diff --git a/src/commands/wizard/onboardingWizard.ts b/src/commands/wizard/onboardingWizard.ts index 7c2d88b2..e2c90057 100644 --- a/src/commands/wizard/onboardingWizard.ts +++ b/src/commands/wizard/onboardingWizard.ts @@ -11,7 +11,7 @@ import { BriefcaseCommand } from './briefcaseCommand'; import { DeployToOrgCommand } from './deployToOrgCommand'; import { ConfigureProjectCommand } from './configureProjectCommand'; import { AuthorizeCommand } from './authorizeCommand'; -import { InstructionsWebviewProvider } from '../../webviews'; +import { InstructionsWebviewProvider } from '../../webviews/instructions'; const wizardCommand = 'salesforcedx-vscode-offline-app.onboardingWizard'; const onboardingWizardStateKey = diff --git a/src/commands/wizard/templateChooserCommand.ts b/src/commands/wizard/templateChooserCommand.ts index 4b6789aa..b61576ad 100644 --- a/src/commands/wizard/templateChooserCommand.ts +++ b/src/commands/wizard/templateChooserCommand.ts @@ -10,7 +10,7 @@ import { UIUtils } from '../../utils/uiUtils'; import { workspace } from 'vscode'; import * as path from 'path'; import * as fs from 'fs'; -import { InstructionsWebviewProvider } from '../../webviews'; +import { InstructionsWebviewProvider } from '../../webviews/instructions'; export interface TemplateQuickPickItem extends QuickPickItem { filenamePrefix: string; diff --git a/src/test/suite/commands/wizard/briefcaseCommand.test.ts b/src/test/suite/commands/wizard/briefcaseCommand.test.ts index 1114fc49..166a950f 100644 --- a/src/test/suite/commands/wizard/briefcaseCommand.test.ts +++ b/src/test/suite/commands/wizard/briefcaseCommand.test.ts @@ -11,7 +11,7 @@ import { afterEach, beforeEach } from 'mocha'; import { BriefcaseCommand } from '../../../../commands/wizard/briefcaseCommand'; import { Uri, l10n, window, Progress, CancellationToken } from 'vscode'; import { CommonUtils } from '@salesforce/lwc-dev-mobile-core/lib/common/CommonUtils'; -import { InstructionsWebviewProvider } from '../../../../webviews'; +import { InstructionsWebviewProvider } from '../../../../webviews/instructions'; suite('Briefcase Command Test Suite', () => { beforeEach(function () {}); diff --git a/src/test/suite/webviews.test.ts b/src/test/suite/webviews.test.ts index ff50a8eb..b20c6bac 100644 --- a/src/test/suite/webviews.test.ts +++ b/src/test/suite/webviews.test.ts @@ -8,7 +8,7 @@ import * as assert from 'assert'; import * as sinon from 'sinon'; import { Uri, env, languages } from 'vscode'; -import { InstructionsWebviewProvider } from '../../webviews'; +import { InstructionsWebviewProvider } from '../../webviews/instructions'; import { afterEach, beforeEach } from 'mocha'; import * as fs from 'fs'; diff --git a/src/webviews.ts b/src/webviews/instructions.ts similarity index 100% rename from src/webviews.ts rename to src/webviews/instructions.ts From c382928d0789af8f87f60c3b82273a58bbed2ec0 Mon Sep 17 00:00:00 2001 From: Kevin Hawkins Date: Tue, 24 Oct 2023 23:13:28 -0700 Subject: [PATCH 5/7] More refactoring, started adding tests --- src/test/suite/webviews.test.ts | 191 +++++++++++++++++++++++++++++++- src/webviews/instructions.ts | 97 ++-------------- src/webviews/processor.ts | 112 +++++++++++++++++++ 3 files changed, 309 insertions(+), 91 deletions(-) create mode 100644 src/webviews/processor.ts diff --git a/src/test/suite/webviews.test.ts b/src/test/suite/webviews.test.ts index b20c6bac..3a77d2e3 100644 --- a/src/test/suite/webviews.test.ts +++ b/src/test/suite/webviews.test.ts @@ -7,10 +7,13 @@ import * as assert from 'assert'; import * as sinon from 'sinon'; -import { Uri, env, languages } from 'vscode'; -import { InstructionsWebviewProvider } from '../../webviews/instructions'; +import { Uri, WebviewPanel, env } from 'vscode'; import { afterEach, beforeEach } from 'mocha'; import * as fs from 'fs'; +import { + WebviewMessageHandler, + WebviewProcessor +} from '../../webviews/processor'; suite('InstructionsWebviewProvider Test Suite', () => { const extensionUri = Uri.parse('file:///tmp/testdir'); @@ -28,8 +31,8 @@ suite('InstructionsWebviewProvider Test Suite', () => { const fsExistStub = sinon.stub(fs, 'existsSync'); fsExistStub.returns(true); - const provider = new InstructionsWebviewProvider(extensionUri); - const path = provider.getLocaleContentPath(extensionUri, 'test.html'); + const processor = new WebviewProcessor(extensionUri); + const path = processor.getLocaleContentPath('test.html'); assert.equal(path, 'test.es.html'); }); @@ -41,9 +44,185 @@ suite('InstructionsWebviewProvider Test Suite', () => { const fsExistStub = sinon.stub(fs, 'existsSync'); fsExistStub.returns(false); - const provider = new InstructionsWebviewProvider(extensionUri); - const path = provider.getLocaleContentPath(extensionUri, 'test.html'); + const processor = new WebviewProcessor(extensionUri); + const path = processor.getLocaleContentPath('test.html'); assert.equal(path, 'test.html'); }); + + test('No responsive message handlers', async () => { + const messageHandlerType = 'testType'; + const testMessage = 'A test messsage for someNonResponsiveType'; + const messageHandler: WebviewMessageHandler = { + type: messageHandlerType, + action: (_panel, _data) => { + assert.fail('This callback should not have been executed.'); + } + }; + const data = { + type: 'someNonResponsiveType', + testMessage: testMessage + }; + + const processor = new WebviewProcessor(extensionUri); + const panel = processor.createWebviewPanel('someViewType', 'someTitle'); + processor.onWebviewReceivedMessage(data, panel, [messageHandler]); + panel.dispose(); + }); + + test('One message handler for web view, no callback', async () => { + const messageHandlerType = 'testType'; + const testMessage = 'A test messsage for testType'; + const messageHandler: WebviewMessageHandler = { + type: messageHandlerType, + action: (_panel, data, callback) => { + const testData = data as { type: string; testMessage: string }; + assert.ok(testData && testData.testMessage === testMessage); + assert.ok(callback === undefined); + } + }; + const data = { + type: messageHandlerType, + testMessage: testMessage + }; + + const processor = new WebviewProcessor(extensionUri); + const panel = processor.createWebviewPanel('someViewType', 'someTitle'); + processor.onWebviewReceivedMessage(data, panel, [messageHandler]); + panel.dispose(); + }); + + test('One message handler for web view, callback', async () => { + const processor = new WebviewProcessor(extensionUri); + const panel = processor.createWebviewPanel('someViewType', 'someTitle'); + const postMessageStub = sinon + .stub(panel.webview, 'postMessage') + .callsFake((message) => { + return new Promise((resolve) => { + assert.ok(message.callbackId === callbackId); + assert.ok( + message.testResponseMessage === testResponseMessage + ); + return resolve(true); + }); + }); + const messageHandlerType = 'testType'; + const testMessage = 'A test messsage for testType'; + const testResponseMessage = 'A test response'; + const testResponseObj = { testResponseMessage }; + const callbackId = 13; + const messageHandler: WebviewMessageHandler = { + type: messageHandlerType, + action: (_panel, data, callback) => { + const testData = data as { type: string; testMessage: string }; + assert.ok(testData && testData.testMessage === testMessage); + assert.ok(!!callback); + callback(testResponseObj); + } + }; + const data = { + type: messageHandlerType, + testMessage, + callbackId + }; + + processor.onWebviewReceivedMessage(data, panel, [messageHandler]); + panel.dispose(); + postMessageStub.restore(); + }); + + test('Multiple message handlers', async () => { + type HandlerData = { + type: string; + testMessage: string; + testResponseMessage: string; + testResponseObj: { testResponseMessage: string }; + callbackId: number; + }; + const processor = new WebviewProcessor(extensionUri); + const panel = processor.createWebviewPanel('someViewType', 'someTitle'); + const handler1Data: HandlerData = { + type: 'testType1', + testMessage: 'A test messsage for testType1', + testResponseMessage: 'A test response for testType1', + testResponseObj: { + testResponseMessage: 'A test messsage for testType1' + }, + callbackId: 7 + }; + const handler2Data: HandlerData = { + type: 'testType2', + testMessage: 'A test messsage for testType2', + testResponseMessage: 'A test response for testType2', + testResponseObj: { + testResponseMessage: 'A test messsage for testType2' + }, + callbackId: 8 + }; + const postMessageStub = sinon + .stub(panel.webview, 'postMessage') + .callsFake((message) => { + return new Promise((resolve) => { + const handlerData = [handler1Data, handler2Data].find( + (data) => { + return data.callbackId === message.callbackId; + } + ); + assert.ok(!!handlerData); + assert.ok( + message.testResponseMessage === + handlerData.testResponseMessage + ); + return resolve(true); + }); + }); + const messageHandlers: WebviewMessageHandler[] = [ + { + type: handler1Data.type, + action: (_panel, data, callback) => { + const testData = data as { + type: string; + testMessage: string; + }; + assert.ok( + testData && + testData.testMessage === handler1Data.testMessage + ); + assert.ok(!!callback); + callback(handler1Data.testResponseObj); + } + }, + { + type: handler2Data.type, + action: (_panel, data, callback) => { + const testData = data as { + type: string; + testMessage: string; + }; + assert.ok( + testData && + testData.testMessage === handler2Data.testMessage + ); + assert.ok(!!callback); + callback(handler2Data.testResponseObj); + } + } + ]; + const data1 = { + type: handler1Data.type, + testMessage: handler1Data.testMessage, + callbackId: handler1Data.callbackId + }; + const data2 = { + type: handler2Data.type, + testMessage: handler2Data.testMessage, + callbackId: handler2Data.callbackId + }; + + for (const data of [data1, data2]) { + processor.onWebviewReceivedMessage(data, panel, messageHandlers); + } + panel.dispose(); + postMessageStub.restore(); + }); }); diff --git a/src/webviews/instructions.ts b/src/webviews/instructions.ts index 11da3772..57d87e7f 100644 --- a/src/webviews/instructions.ts +++ b/src/webviews/instructions.ts @@ -7,27 +7,17 @@ import * as vscode from 'vscode'; import * as fs from 'fs'; +import { WebviewMessageHandler, WebviewProcessor } from './processor'; -export const MESSAGING_SCRIPT_PATH_DEMARCATOR = '--- MESSAGING_SCRIPT_SRC ---'; -export const MESSAGING_JS_PATH = 'resources/instructions/webviewMessaging.js'; const INSTRUCTION_VIEW_TYPE = 'instructionsView'; -export type WebviewMessageCallback = (responseData?: object) => void; - -export type WebviewMessageHandler = { - type: string; - action: ( - panel: vscode.WebviewPanel, - data?: object, - callback?: WebviewMessageCallback - ) => void; -}; - export class InstructionsWebviewProvider { extensionUri: vscode.Uri; + processor: WebviewProcessor; - constructor(extensionUri: vscode.Uri) { + constructor(extensionUri: vscode.Uri, processor?: WebviewProcessor) { this.extensionUri = extensionUri; + this.processor = processor ?? new WebviewProcessor(extensionUri); } public showInstructionWebview( @@ -36,58 +26,23 @@ export class InstructionsWebviewProvider { messageHandlers: WebviewMessageHandler[] ) { this.validateMessageHanders(messageHandlers); - const panel = vscode.window.createWebviewPanel( + const panel = this.processor.createWebviewPanel( INSTRUCTION_VIEW_TYPE, - title, - vscode.ViewColumn.Beside, - { - enableScripts: true, - localResourceRoots: [this.extensionUri] - } + title ); panel.webview.onDidReceiveMessage((data) => { - const responsiveHandlers = messageHandlers.filter( - (messageHandler) => data.type === messageHandler.type + this.processor.onWebviewReceivedMessage( + data, + panel, + messageHandlers ); - if (responsiveHandlers.length > 0) { - const handler = responsiveHandlers[0]; - let callback: WebviewMessageCallback | undefined; - if (data.callbackId) { - const returnedCallbackId = data.callbackId; - delete data.callbackId; - callback = (responseData?: object) => { - const fullResponseMessage = { - callbackId: returnedCallbackId, - ...responseData - }; - panel.webview.postMessage(fullResponseMessage); - }; - } - handler.action(panel, data, callback); - } }); - const localeContentPath = this.getLocaleContentPath( - this.extensionUri, + const webviewContent = this.processor.getWebviewContent( + panel, contentPath ); - const htmlPath = vscode.Uri.joinPath( - this.extensionUri, - localeContentPath - ); - const messagingJsPath = vscode.Uri.joinPath( - this.extensionUri, - MESSAGING_JS_PATH - ); - - let webviewContent = fs.readFileSync(htmlPath.fsPath, { - encoding: 'utf-8' - }); - webviewContent = webviewContent.replace( - MESSAGING_SCRIPT_PATH_DEMARCATOR, - panel.webview.asWebviewUri(messagingJsPath).toString() - ); panel.webview.html = webviewContent; } @@ -111,34 +66,6 @@ export class InstructionsWebviewProvider { }); } - /** - * Check to see if a locale-specific file exists, otherwise return the default. - * @param extensionUri Uri representing the path to this extension, supplied by vscode. - * @param contentPath The relative path (and filename) of the content to display. - */ - getLocaleContentPath( - extensionUri: vscode.Uri, - contentPath: string - ): string { - const language = vscode.env.language; - - // check to see if a file exists for this locale. - const localeContentPath = contentPath.replace( - /\.html$/, - `.${language}.html` - ); - - const fullPath = vscode.Uri.joinPath(extensionUri, localeContentPath); - - if (fs.existsSync(fullPath.fsPath)) { - // a file exists for this locale, so return it instead. - return localeContentPath; - } else { - // fall back - return contentPath; - } - } - private validateMessageHanders(messageHandlers: WebviewMessageHandler[]) { const handlerMap: { [type: string]: boolean } = {}; for (const handler of messageHandlers) { diff --git a/src/webviews/processor.ts b/src/webviews/processor.ts new file mode 100644 index 00000000..f5d36728 --- /dev/null +++ b/src/webviews/processor.ts @@ -0,0 +1,112 @@ +/* + * Copyright (c) 2023, salesforce.com, inc. + * All rights reserved. + * SPDX-License-Identifier: MIT + * For full license text, see the LICENSE file in the repo root or https://opensource.org/licenses/MIT + */ + +import { existsSync, readFileSync } from 'node:fs'; +import { Uri, ViewColumn, WebviewPanel, env, window } from 'vscode'; + +export const MESSAGING_JS_PATH = 'resources/instructions/webviewMessaging.js'; +export const MESSAGING_SCRIPT_PATH_DEMARCATOR = '--- MESSAGING_SCRIPT_SRC ---'; + +export type WebviewMessageCallback = (responseData?: object) => void; + +export type WebviewMessageHandler = { + type: string; + action: ( + panel: WebviewPanel, + data?: object, + callback?: WebviewMessageCallback + ) => void; +}; + +export class WebviewProcessor { + extensionUri: Uri; + + constructor(extensionUri: Uri) { + this.extensionUri = extensionUri; + } + + public createWebviewPanel(viewType: string, title: string): WebviewPanel { + const panel = window.createWebviewPanel( + viewType, + title, + ViewColumn.Beside, + { + enableScripts: true, + localResourceRoots: [this.extensionUri] + } + ); + return panel; + } + + public onWebviewReceivedMessage( + data: any, + panel: WebviewPanel, + messageHandlers: WebviewMessageHandler[] + ) { + const responsiveHandlers = messageHandlers.filter( + (messageHandler) => data.type === messageHandler.type + ); + if (responsiveHandlers.length > 0) { + const handler = responsiveHandlers[0]; + let callback: WebviewMessageCallback | undefined; + if (data.callbackId) { + const returnedCallbackId = data.callbackId; + delete data.callbackId; + callback = (responseData?: object) => { + const fullResponseMessage = { + callbackId: returnedCallbackId, + ...responseData + }; + panel.webview.postMessage(fullResponseMessage); + }; + } + handler.action(panel, data, callback); + } + } + + public getWebviewContent(panel: WebviewPanel, contentPath: string): string { + const localeContentPath = this.getLocaleContentPath(contentPath); + const htmlPath = Uri.joinPath(this.extensionUri, localeContentPath); + const messagingJsPath = Uri.joinPath( + this.extensionUri, + MESSAGING_JS_PATH + ); + + let webviewContent = readFileSync(htmlPath.fsPath, { + encoding: 'utf-8' + }); + webviewContent = webviewContent.replace( + MESSAGING_SCRIPT_PATH_DEMARCATOR, + panel.webview.asWebviewUri(messagingJsPath).toString() + ); + return webviewContent; + } + + /** + * Check to see if a locale-specific file exists, otherwise return the default. + * @param contentPath The relative path (and filename) of the content to display. + */ + public getLocaleContentPath(contentPath: string): string { + const language = env.language; + + // check to see if a file exists for this locale. + const localeContentPath = contentPath.replace( + /\.html$/, + `.${language}.html` + ); + + const fullPath = Uri.joinPath(this.extensionUri, localeContentPath); + + if (existsSync(fullPath.fsPath)) { + // a file exists for this locale, so return it instead. + return localeContentPath; + } else { + // fall back + return contentPath; + } + } +} From 5028e0bf85e576fbee62d7236b1038cf8e799c32 Mon Sep 17 00:00:00 2001 From: Kevin Hawkins Date: Wed, 25 Oct 2023 13:11:34 -0700 Subject: [PATCH 6/7] Finalizing tests --- src/test/suite/webviews.test.ts | 81 ++++++++++++++++++++++++++++++++- src/webviews/instructions.ts | 15 +----- src/webviews/processor.ts | 25 ++++++++-- 3 files changed, 100 insertions(+), 21 deletions(-) diff --git a/src/test/suite/webviews.test.ts b/src/test/suite/webviews.test.ts index 3a77d2e3..9fc04828 100644 --- a/src/test/suite/webviews.test.ts +++ b/src/test/suite/webviews.test.ts @@ -9,13 +9,16 @@ import * as assert from 'assert'; import * as sinon from 'sinon'; import { Uri, WebviewPanel, env } from 'vscode'; import { afterEach, beforeEach } from 'mocha'; -import * as fs from 'fs'; +import * as fs from 'node:fs'; +import { mkdir, mkdtemp, writeFile } from 'node:fs/promises'; +import { join } from 'node:path'; +import { tmpdir } from 'node:os'; import { WebviewMessageHandler, WebviewProcessor } from '../../webviews/processor'; -suite('InstructionsWebviewProvider Test Suite', () => { +suite('Webview Test Suite', () => { const extensionUri = Uri.parse('file:///tmp/testdir'); beforeEach(function () {}); @@ -225,4 +228,78 @@ suite('InstructionsWebviewProvider Test Suite', () => { panel.dispose(); postMessageStub.restore(); }); + + test('Unique types for message handler collection passes validation', async () => { + const messageHandlers: WebviewMessageHandler[] = [ + { + type: 'type1', + action: (_panel, data, callback) => {} + }, + { + type: 'type2', + action: (_panel, data, callback) => {} + } + ]; + WebviewProcessor.validateMessageHanders(messageHandlers); + }); + + test('Repeated types for message handler collection fails validation', async () => { + const messageHandlers: WebviewMessageHandler[] = [ + { + type: 'type1', + action: (_panel, _data) => {} + }, + { + type: 'type1', + action: (_panel, _data) => {} + } + ]; + assert.throws(() => { + WebviewProcessor.validateMessageHanders(messageHandlers); + }, 'A collection of message handlers that has more than one instance of a given type, should fail validation.'); + }); + + test('Get webview content with script demarcator', async () => { + const extensionUriTempDir = await mkdtemp( + join(tmpdir(), 'salesforcedx-vscode-mobile-') + ); + const extensionUri = Uri.file(extensionUriTempDir); + const processor = new WebviewProcessor(extensionUri); + const webviewPanel = processor.createWebviewPanel( + 'someViewType', + 'someTitle' + ); + const contentWithDemarcator = + ''; + const contentWithDemarcatorDereferenced = ``; + + const contentFilename = 'contentFile.html'; + const contentDirPathRelative = 'content'; + const contentDirPathAbsolute = join( + extensionUriTempDir, + contentDirPathRelative + ); + const contentPathRelative = join( + contentDirPathRelative, + contentFilename + ); + const contentPathAbsolute = join( + contentDirPathAbsolute, + contentFilename + ); + await mkdir(join(extensionUriTempDir, contentDirPathRelative)); + await writeFile(contentPathAbsolute, contentWithDemarcator); + + const generatedWebviewContent = processor.getWebviewContent( + webviewPanel, + contentPathRelative + ); + assert.equal( + generatedWebviewContent, + contentWithDemarcatorDereferenced + ); + webviewPanel.dispose(); + }); }); diff --git a/src/webviews/instructions.ts b/src/webviews/instructions.ts index 57d87e7f..f1da3bd3 100644 --- a/src/webviews/instructions.ts +++ b/src/webviews/instructions.ts @@ -25,7 +25,7 @@ export class InstructionsWebviewProvider { contentPath: string, messageHandlers: WebviewMessageHandler[] ) { - this.validateMessageHanders(messageHandlers); + WebviewProcessor.validateMessageHanders(messageHandlers); const panel = this.processor.createWebviewPanel( INSTRUCTION_VIEW_TYPE, title @@ -65,17 +65,4 @@ export class InstructionsWebviewProvider { ]); }); } - - private validateMessageHanders(messageHandlers: WebviewMessageHandler[]) { - const handlerMap: { [type: string]: boolean } = {}; - for (const handler of messageHandlers) { - if (handlerMap[handler.type] === true) { - throw new Error( - `There can be only one message handler per type. There are at least two handlers with type '${handler.type}'.` - ); - } else { - handlerMap[handler.type] = true; - } - } - } } diff --git a/src/webviews/processor.ts b/src/webviews/processor.ts index f5d36728..e066d6b8 100644 --- a/src/webviews/processor.ts +++ b/src/webviews/processor.ts @@ -42,6 +42,21 @@ export class WebviewProcessor { return panel; } + public static validateMessageHanders( + messageHandlers: WebviewMessageHandler[] + ) { + const handlerMap: { [type: string]: boolean } = {}; + for (const handler of messageHandlers) { + if (handlerMap[handler.type] === true) { + throw new Error( + `There can be only one message handler per type. There are at least two handlers with type '${handler.type}'.` + ); + } else { + handlerMap[handler.type] = true; + } + } + } + public onWebviewReceivedMessage( data: any, panel: WebviewPanel, @@ -71,11 +86,7 @@ export class WebviewProcessor { public getWebviewContent(panel: WebviewPanel, contentPath: string): string { const localeContentPath = this.getLocaleContentPath(contentPath); const htmlPath = Uri.joinPath(this.extensionUri, localeContentPath); - const messagingJsPath = Uri.joinPath( - this.extensionUri, - MESSAGING_JS_PATH - ); - + const messagingJsPath = this.getMessagingJsPathUri(); let webviewContent = readFileSync(htmlPath.fsPath, { encoding: 'utf-8' }); @@ -86,6 +97,10 @@ export class WebviewProcessor { return webviewContent; } + public getMessagingJsPathUri(): Uri { + return Uri.joinPath(this.extensionUri, MESSAGING_JS_PATH); + } + /** * Check to see if a locale-specific file exists, otherwise return the default. * @param contentPath The relative path (and filename) of the content to display. From f384c8ad222a39db8507e9805e2e07dbc98db3cd Mon Sep 17 00:00:00 2001 From: Kevin Hawkins Date: Thu, 26 Oct 2023 23:10:53 -0700 Subject: [PATCH 7/7] Making message handler choice more clear --- src/webviews/processor.ts | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/webviews/processor.ts b/src/webviews/processor.ts index e066d6b8..48367759 100644 --- a/src/webviews/processor.ts +++ b/src/webviews/processor.ts @@ -62,11 +62,11 @@ export class WebviewProcessor { panel: WebviewPanel, messageHandlers: WebviewMessageHandler[] ) { - const responsiveHandlers = messageHandlers.filter( + // There can be at most one message handler responsive to a given type. + const responsiveHandler = messageHandlers.find( (messageHandler) => data.type === messageHandler.type ); - if (responsiveHandlers.length > 0) { - const handler = responsiveHandlers[0]; + if (responsiveHandler) { let callback: WebviewMessageCallback | undefined; if (data.callbackId) { const returnedCallbackId = data.callbackId; @@ -79,7 +79,7 @@ export class WebviewProcessor { panel.webview.postMessage(fullResponseMessage); }; } - handler.action(panel, data, callback); + responsiveHandler.action(panel, data, callback); } }