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

fix: init waits for the iframe to be loaded #236

Merged
merged 4 commits into from
Jul 29, 2024
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
8 changes: 4 additions & 4 deletions jest.config.js
Original file line number Diff line number Diff line change
Expand Up @@ -41,10 +41,10 @@ module.exports = {
// An object that configures minimum threshold enforcement for coverage results
coverageThreshold: {
global: {
branches: 89.28,
functions: 95.91,
lines: 91.32,
statements: 91.41,
branches: 90.64,
functions: 95.95,
lines: 93.02,
statements: 93.09,
},
},

Expand Down
30 changes: 8 additions & 22 deletions src/ledger-iframe-bridge.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -36,20 +36,6 @@ function isIFrameValid(
);
}

/**
* Simulates the loading of an iframe by calling the onload function.
*
* @param iframe - The iframe to simulate the loading of.
* @returns Returns a promise that resolves when the onload function is called.
*/
async function simulateIFrameLoad(iframe?: HTMLIFrameElementShim) {
if (!isIFrameValid(iframe)) {
throw new Error('the iframe is not valid');
}
// we call manually the onload event to simulate the iframe loading
return await iframe.onload();
}

const LEDGER_IFRAME_ID = 'LEDGER-IFRAME';
const BRIDGE_URL = 'https://metamask.github.io/eth-ledger-bridge-keyring';
const INVALID_URL_ERROR = 'bridgeURL is not a valid URL';
Expand Down Expand Up @@ -80,7 +66,6 @@ describe('LedgerIframeBridge', function () {
bridgeUrl: BRIDGE_URL,
});
await bridge.init();
await simulateIFrameLoad(bridge.iframe);
});

afterEach(function () {
Expand Down Expand Up @@ -123,15 +108,11 @@ describe('LedgerIframeBridge', function () {
describe('init', function () {
it('sets up the listener and iframe', async function () {
bridge = new LedgerIframeBridge();

const addEventListenerSpy = jest.spyOn(global.window, 'addEventListener');

await bridge.init();

expect(addEventListenerSpy).toHaveBeenCalledTimes(1);
expect(bridge.iframeLoaded).toBe(false);

await simulateIFrameLoad(bridge.iframe);
expect(bridge.iframeLoaded).toBe(true);
});
});
Expand Down Expand Up @@ -200,8 +181,6 @@ describe('LedgerIframeBridge', function () {

describe('updateTransportMethod', function () {
it('sends and processes a successful ledger-update-transport message', async function () {
bridge.iframeLoaded = true;

const transportType = 'u2f';

stubKeyringIFramePostMessage(bridge, (message) => {
Expand All @@ -227,6 +206,14 @@ describe('LedgerIframeBridge', function () {
expect(bridge.iframe?.contentWindow?.postMessage).toHaveBeenCalled();
});

it('throws an error when the bridge is not initialized', async function () {
bridge = new LedgerIframeBridge();

await expect(bridge.updateTransportMethod('u2f')).rejects.toThrow(
'The iframe is not loaded yet',
);
});

it('throws an error when a ledger-update-transport message is not successful', async function () {
bridge.iframeLoaded = true;

Expand Down Expand Up @@ -527,7 +514,6 @@ describe('LedgerIframeBridge', function () {
addEventListenerSpy = jest.spyOn(global.window, 'addEventListener');
bridge = new LedgerIframeBridge();
await bridge.init();
await simulateIFrameLoad(bridge.iframe);
});

describe('when configurate bridge url', function () {
Expand Down
48 changes: 13 additions & 35 deletions src/ledger-iframe-bridge.ts
Original file line number Diff line number Diff line change
Expand Up @@ -110,12 +110,6 @@ export class LedgerIframeBridge
messageCallbacks: Record<number, (response: IFrameMessageResponse) => void> =
{};

delayedPromise?: {
resolve: (value: boolean) => void;
reject: (error: unknown) => void;
transportType: string;
};

constructor(
opts: LedgerIframeBridgeOptions = {
bridgeUrl: 'https://metamask.github.io/eth-ledger-bridge-keyring',
Expand All @@ -128,7 +122,7 @@ export class LedgerIframeBridge
}

async init() {
this.#setupIframe(this.#opts.bridgeUrl);
await this.#setupIframe(this.#opts.bridgeUrl);

this.eventListener = this.#eventListener.bind(this, this.#opts.bridgeUrl);

Expand Down Expand Up @@ -178,12 +172,7 @@ export class LedgerIframeBridge
// If the iframe isn't loaded yet, let's store the desired transportType value and
// optimistically return a successful promise
if (!this.iframeLoaded) {
this.delayedPromise = {
resolve,
reject,
transportType,
};
return;
throw new Error('The iframe is not loaded yet');
}

this.#sendMessage(
Expand Down Expand Up @@ -282,28 +271,17 @@ export class LedgerIframeBridge
});
}

#setupIframe(bridgeUrl: string) {
this.iframe = document.createElement('iframe');
this.iframe.src = bridgeUrl;
this.iframe.allow = `hid 'src'`;
this.iframe.onload = async () => {
// If the ledger live preference was set before the iframe is loaded,
// set it after the iframe has loaded
this.iframeLoaded = true;
if (this.delayedPromise) {
try {
const result = await this.updateTransportMethod(
this.delayedPromise.transportType,
);
this.delayedPromise.resolve(result);
} catch (error) {
this.delayedPromise.reject(error);
} finally {
delete this.delayedPromise;
}
}
};
document.head.appendChild(this.iframe);
async #setupIframe(bridgeUrl: string): Promise<void> {
return new Promise((resolve) => {
this.iframe = document.createElement('iframe');
this.iframe.src = bridgeUrl;
this.iframe.allow = `hid 'src'`;
this.iframe.onload = async () => {
this.iframeLoaded = true;
resolve();
};
document.head.appendChild(this.iframe);
});
}

#getOrigin(bridgeUrl: string) {
Expand Down
41 changes: 17 additions & 24 deletions test/document.shim.ts
Original file line number Diff line number Diff line change
@@ -1,32 +1,25 @@
// eslint-disable-next-line import/no-mutable-exports
let documentShim: any;

try {
documentShim = document || {
head: {
appendChild: () => false,
const shim = {
head: {
appendChild: (child: { onload?: () => void }) => {
child.onload?.();
Comment on lines +4 to +7
Copy link
Member Author

Choose a reason for hiding this comment

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

documentShim.head.appendChild seems to be the best place to forge the iframe onload. I removed some code from tests that was intended to do this

},
createElement: () => ({
src: false,
contentWindow: {
postMessage: () => false,
},
}),
addEventListener: () => false,
};
} catch (error) {
documentShim = {
head: {
appendChild: () => false,
},
createElement: () => ({
src: false,
contentWindow: {
postMessage: () => false,
},
createElement: () => ({
src: false,
contentWindow: {
postMessage: () => false,
},
}),
addEventListener: () => false,
};
}),
addEventListener: () => false,
};

try {
documentShim = document || shim;
} catch (error) {
documentShim = shim;
}

export default documentShim;
Loading