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

Popup ready signal to service worker #174

Merged
merged 12 commits into from
Aug 28, 2024
1 change: 1 addition & 0 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -62,3 +62,4 @@ packages/*/package

apps/extension/chromium-profile

scripts/private
Copy link
Contributor

Choose a reason for hiding this comment

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

suggestion: revert gitignore diff

17 changes: 17 additions & 0 deletions apps/extension/src/hooks/popup-ready.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
import { useEffect, useRef } from 'react';
import { useSearchParams } from 'react-router-dom';

// signals that react is ready (mounted) to service worker
export const usePopupReady = () => {
const sentMessagesRef = useRef(new Set());
const [searchParams] = useSearchParams();
const popupId = searchParams.get('popupId');

useEffect(() => {
if (popupId && !sentMessagesRef.current.has(popupId)) {
sentMessagesRef.current.add(popupId);

void chrome.runtime.sendMessage(popupId);
}
}, [popupId]);
};
47 changes: 35 additions & 12 deletions apps/extension/src/popup.ts
Original file line number Diff line number Diff line change
Expand Up @@ -18,9 +18,13 @@ const isChromeResponderDroppedError = (
export const popup = async <M extends PopupMessage>(
req: PopupRequest<M>,
): Promise<M['response']> => {
await spawnPopup(req.type);
// We have to wait for React to bootup, navigate to the page, and render the components
await new Promise(resolve => setTimeout(resolve, 800));
const popupId = crypto.randomUUID();
await spawnPopup(req.type, popupId);

// this is necessary given it takes a bit of time for the popup
// to be ready to accept messages from the service worker.
await popupReady(popupId);
Copy link
Contributor

Choose a reason for hiding this comment

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

suggestion: we should add a code comment here that informs devs that this is necessary given it takes a bit of time for the popup to be ready to accept messages.


const response = await chrome.runtime
.sendMessage<InternalRequest<M>, InternalResponse<M>>(req)
.catch((e: unknown) => {
Expand All @@ -30,20 +34,22 @@ export const popup = async <M extends PopupMessage>(
throw e;
}
});

if (response && 'error' in response) {
throw errorFromJson(response.error, undefined, ConnectError.from(response));
} else {
return response && response.data;
}
};

const spawnDetachedPopup = async (path: string) => {
await throwIfAlreadyOpen(path);
const spawnDetachedPopup = async (url: URL) => {
const [hashPath] = url.hash.split('?');
await throwIfAlreadyOpen(hashPath!);

const { top, left, width } = await chrome.windows.getLastFocused();

await chrome.windows.create({
url: path,
url: url.href,
type: 'popup',
width: 400,
height: 628,
Expand Down Expand Up @@ -73,19 +79,36 @@ const throwIfNeedsLogin = async () => {
}
};

const spawnPopup = async (pop: PopupType) => {
const spawnPopup = async (pop: PopupType, popupId: string) => {
const popUrl = new URL(chrome.runtime.getURL('popup.html'));

await throwIfNeedsLogin();

switch (pop) {
// set path as hash since we use a hash router within the popup
case PopupType.OriginApproval:
popUrl.hash = PopupPath.ORIGIN_APPROVAL;
return spawnDetachedPopup(popUrl.href);
popUrl.hash = `${PopupPath.ORIGIN_APPROVAL}?popupId=${popupId}`;
return spawnDetachedPopup(popUrl);
case PopupType.TxApproval:
popUrl.hash = PopupPath.TRANSACTION_APPROVAL;
return spawnDetachedPopup(popUrl.href);
popUrl.hash = `${PopupPath.TRANSACTION_APPROVAL}?popupId=${popupId}`;
return spawnDetachedPopup(popUrl);
default:
throw Error('Unknown popup type');
}
};

const POPUP_READY_TIMEOUT = 60 * 1000;

const popupReady = (popupId: string): Promise<void> =>
new Promise((resolve, reject) => {
AbortSignal.timeout(POPUP_READY_TIMEOUT).onabort = reject;

const idListen = (msg: unknown, _: chrome.runtime.MessageSender, respond: () => void) => {
if (msg === popupId) {
resolve();
chrome.runtime.onMessage.removeListener(idListen);
respond();
}
};

chrome.runtime.onMessage.addListener(idListen);
});
15 changes: 10 additions & 5 deletions apps/extension/src/routes/popup/popup-layout.tsx
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import { Outlet } from 'react-router-dom';
import { usePopupReady } from '../../hooks/popup-ready';

/**
* @todo: Fix the issue where the detached popup isn't sized correctly. This
Expand All @@ -11,8 +12,12 @@ import { Outlet } from 'react-router-dom';
* routes here in `PopupLayout`, and using a different root class name for each,
* then removing the hard-coded width from `globals.css`.
*/
export const PopupLayout = () => (
<div className='flex grow flex-col bg-card-radial'>
<Outlet />
</div>
);
export const PopupLayout = () => {
usePopupReady();
turbocrime marked this conversation as resolved.
Show resolved Hide resolved

return (
<div className='flex grow flex-col bg-card-radial'>
<Outlet />
</div>
);
};