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

feat(embedded-dashboard): Share Switchboard State for Sending Events from Plugins #21319

Merged
merged 10 commits into from
Oct 10, 2022
Original file line number Diff line number Diff line change
Expand Up @@ -17,4 +17,7 @@
* under the License.
*/

import Switchboard from './switchboard';

export * from './switchboard';
export default Switchboard;
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@
* under the License.
*/

import { Switchboard } from './switchboard';
import SingletonSwitchboard, { Switchboard } from './switchboard';

type EventHandler = (event: MessageEvent) => void;

Expand Down Expand Up @@ -112,6 +112,7 @@ describe('comms', () => {

beforeEach(() => {
console.debug = jest.fn(); // silencio bruno
console.error = jest.fn();
});

afterEach(() => {
Expand All @@ -126,6 +127,30 @@ describe('comms', () => {
expect(sb).toHaveProperty('debugMode');
});

it('singleton', async () => {
SingletonSwitchboard.start();
expect(console.error).toHaveBeenCalledWith(
'[]',
'Switchboard not initialised',
);
SingletonSwitchboard.emit('someEvent', 42);
expect(console.error).toHaveBeenCalledWith(
'[]',
'Switchboard not initialised',
);
await expect(SingletonSwitchboard.get('failing')).rejects.toThrow(
'Switchboard not initialised',
);
SingletonSwitchboard.init({ port: new MessageChannel().port1 });
expect(SingletonSwitchboard).toHaveProperty('name');
expect(SingletonSwitchboard).toHaveProperty('debugMode');
SingletonSwitchboard.init({ port: new MessageChannel().port1 });
expect(console.error).toHaveBeenCalledWith(
'[switchboard]',
'already initialized',
);
});

describe('emit', () => {
it('triggers the method', async () => {
const channel = new MessageChannel();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -90,7 +90,7 @@ function isError(message: Message): message is ErrorMessage {
export class Switchboard {
port: MessagePort;

name: string;
name = '';
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this change needed? I feel like the previous one was cleaner.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

+1 to Ville's comment, I'd prefer string as well ... and rest of the file does it that way too!

Copy link
Contributor Author

@sinhashubham95 sinhashubham95 Oct 8, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is added because there can be instances of logging the error even when Switchboard is not initialised, which will lead to logging undefined if this change is not made. It's just like providing a default initial value.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So, it should be name: string = ''; then?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's automatically detected in typescript based on the value assigned.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@sinhashubham95 makes sense, I hadn't thought of that 👍


methods: Record<string, Method<any, unknown>> = {};

Expand All @@ -99,7 +99,23 @@ export class Switchboard {

debugMode: boolean;

constructor({ port, name = 'switchboard', debug = false }: Params) {
private isInitialised: boolean;

constructor(params?: Params) {
if (!params) {
return;
}
this.init(params);
}

init(params: Params) {
if (this.isInitialised) {
this.logError('already initialized');
return;
}

const { port, name = 'switchboard', debug = false } = params;

this.port = port;
this.name = name;
this.debugMode = debug;
Expand All @@ -122,6 +138,8 @@ export class Switchboard {
}
}
});

this.isInitialised = true;
}

private async getMethodResult({
Expand Down Expand Up @@ -178,6 +196,10 @@ export class Switchboard {
*/
get<T = unknown>(method: string, args: unknown = undefined): Promise<T> {
return new Promise((resolve, reject) => {
if (!this.isInitialised) {
reject(new Error('Switchboard not initialised'));
return;
}
// In order to "call a method" on the other side of the port,
// we will send a message with a unique id
const messageId = this.getNewMessageId();
Expand Down Expand Up @@ -215,6 +237,10 @@ export class Switchboard {
* @param args
*/
emit(method: string, args: unknown = undefined) {
if (!this.isInitialised) {
this.logError('Switchboard not initialised');
return;
}
const message: EmitMessage = {
switchboardAction: Actions.EMIT,
method,
Expand All @@ -224,6 +250,10 @@ export class Switchboard {
}

start() {
if (!this.isInitialised) {
this.logError('Switchboard not initialised');
return;
}
this.port.start();
}

Expand All @@ -242,3 +272,5 @@ export class Switchboard {
return `m_${this.name}_${this.incrementor++}`;
}
}

export default new Switchboard();
14 changes: 7 additions & 7 deletions superset-frontend/src/embedded/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ import React, { lazy, Suspense } from 'react';
import ReactDOM from 'react-dom';
import { BrowserRouter as Router, Route } from 'react-router-dom';
import { makeApi, t, logging } from '@superset-ui/core';
import { Switchboard } from '@superset-ui/switchboard';
import Switchboard from '@superset-ui/switchboard';
import { bootstrapData } from 'src/preamble';
import setupClient from 'src/setup/setupClient';
import { RootContextProviders } from 'src/views/RootContextProviders';
Expand Down Expand Up @@ -176,15 +176,15 @@ window.addEventListener('message', function embeddedPageInitializer(event) {
if (event.data.handshake === 'port transfer' && port) {
log('message port received', event);

const switchboard = new Switchboard({
Switchboard.init({
port,
name: 'superset',
debug: debugMode,
});

let started = false;

switchboard.defineMethod(
Switchboard.defineMethod(
'guestToken',
({ guestToken }: { guestToken: string }) => {
setupGuestClient(guestToken);
Expand All @@ -195,13 +195,13 @@ window.addEventListener('message', function embeddedPageInitializer(event) {
},
);

switchboard.defineMethod('getScrollSize', embeddedApi.getScrollSize);
switchboard.defineMethod(
Switchboard.defineMethod('getScrollSize', embeddedApi.getScrollSize);
Switchboard.defineMethod(
'getDashboardPermalink',
embeddedApi.getDashboardPermalink,
);
switchboard.defineMethod('getActiveTabs', embeddedApi.getActiveTabs);
switchboard.start();
Switchboard.defineMethod('getActiveTabs', embeddedApi.getActiveTabs);
Switchboard.start();
}
});

Expand Down