Skip to content

Tweaks #46

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

Merged
merged 11 commits into from
May 20, 2025
Merged

Tweaks #46

merged 11 commits into from
May 20, 2025

Conversation

gjmooney
Copy link
Collaborator

This updates the listCommands interface to return a promise and sends a message to the host indicating the extension has loaded.

@gjmooney gjmooney added the bug Something isn't working label Apr 25, 2025
return commands.listCommands();
}
};

const endpoint = windowEndpoint(self.parent);
expose(api, endpoint);

app.started.then(() => {
window.parent?.postMessage('extension-loaded', '*');
Copy link
Collaborator

Choose a reason for hiding this comment

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

Wondering if createBridge could expose a new ready: Promise<void>, to make waiting for the extension in the IFrame to be ready more convenient?

Copy link
Collaborator

Choose a reason for hiding this comment

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

Also there may be some race conditions depending on when the host defines the event listener and when the IFrame posts the message (too early / late).

Copy link
Collaborator

Choose a reason for hiding this comment

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

Looks like there is an open issue about that in Comlink: GoogleChromeLabs/comlink#665

So in our case we can likely implement a workaround by defining custom logic for exposing the ready state of the IFrame.

Copy link
Collaborator

@jtpio jtpio Apr 29, 2025

Choose a reason for hiding this comment

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

Opened #47 against this PR to explore adding a ready promise.

@jtpio jtpio mentioned this pull request Apr 29, 2025
* ready promise

* custom proxy

* update demo

* lint
@gjmooney gjmooney marked this pull request as ready for review May 14, 2025 13:23
@gjmooney gjmooney closed this May 14, 2025
@gjmooney gjmooney reopened this May 14, 2025
@gjmooney
Copy link
Collaborator Author

please update snapshots

@gjmooney gjmooney closed this May 14, 2025
@gjmooney gjmooney reopened this May 14, 2025
@gjmooney
Copy link
Collaborator Author

please update snapshots

@gjmooney gjmooney closed this May 15, 2025
@gjmooney gjmooney reopened this May 15, 2025
@gjmooney
Copy link
Collaborator Author

please update snapshots

@gjmooney gjmooney closed this May 20, 2025
@gjmooney gjmooney reopened this May 20, 2025
Copy link
Collaborator

@jtpio jtpio left a comment

Choose a reason for hiding this comment

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

Thanks!

@jtpio jtpio merged commit 518131c into main May 20, 2025
15 checks passed
@jtpio jtpio deleted the tweaks branch May 20, 2025 18:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants