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(checkout): CHECKOUT-7538 Introduce extension messenger and command handler #2050

Merged
merged 9 commits into from
Jul 16, 2023

Conversation

bc-peng
Copy link
Contributor

@bc-peng bc-peng commented Jul 7, 2023

What?

  • Establish a two-way communication mechanism between checkout-js and an extension.
  • The message from an extension should always include the extension ID.
  • Define command handling mechanism and its related interfaces.

Why?

checkout-js will be able to carry out the corresponding actions when receving a command from an extension.

Testing / Proof

  • CI.

@bc-peng bc-peng changed the title feat(checkout): CHECKOUT-7538 WIP feat(checkout): CHECKOUT-7538 Introduce extension messenger and command handler Jul 7, 2023
@bc-peng bc-peng force-pushed the IFRAME-MESSENGER branch from bb644e5 to 19828b4 Compare July 7, 2023 07:45
@bc-peng bc-peng marked this pull request as ready for review July 7, 2023 07:51
@bc-peng bc-peng requested a review from a team as a code owner July 7, 2023 07:51
@bc-peng bc-peng force-pushed the IFRAME-MESSENGER branch 2 times, most recently from 658bc5e to efe284a Compare July 10, 2023 00:04
@bc-peng bc-peng force-pushed the IFRAME-MESSENGER branch from efe284a to 593abbd Compare July 10, 2023 00:23
packages/core/src/checkout/checkout-service.ts Outdated Show resolved Hide resolved
packages/core/src/common/iframe/iframe-event-poster.ts Outdated Show resolved Hide resolved
packages/core/src/extension/extension-action-creator.ts Outdated Show resolved Hide resolved
packages/core/src/extension/extension-post-event.ts Outdated Show resolved Hide resolved
packages/core/src/checkout/checkout-service.ts Outdated Show resolved Hide resolved
packages/core/src/extension/extension-messenger.ts Outdated Show resolved Hide resolved
packages/core/src/extension/extension-messenger.ts Outdated Show resolved Hide resolved
packages/core/src/extension/extension.ts Outdated Show resolved Hide resolved
@bc-peng bc-peng force-pushed the IFRAME-MESSENGER branch from 9630507 to ebe0b53 Compare July 12, 2023 06:57
* @param extensionId - The ID of an extension sending the commands.
* @param handlers - A set of handlers for the extension commands.
*/
addExtensionCommandHandlers(extensionId: string, handlers: ExtensionCommandHandlers): void {
Copy link
Contributor

Choose a reason for hiding this comment

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

I think it's better to return a function that could unsubscribe the handler, similar to the existing subscribe method. e.g.:

listenExtensionCommand(extensionId: string, commandType: ExtensionCommand, commandHandler, ExtensionCommandHandler): () => void

This makes it easier to unsubscribe a handler, as calling the return value can specifically remove such handler. Otherwise, clients need to hold a reference to the handler and therefore can't use inline anonymous function as the handler.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I have updated it as:

listenExtensionCommand(
        extensionId: string,
        command: ExtensionCommand,
        handler: ExtensionCommandHandler,
    ): () => void

To entirely remove the command listening, I also added a method in the checkoutService to stop listening for an extension.

stopListenExtensionCommand(extensionId: string): void

private _posters: { [extensionId: string]: IframeEventPoster<HostOriginEvent> } = {};

listen(
extensions: Extension[],
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm thinking you can just inject ReadableCheckoutStore as a constructor parameter of ExtensionMessenger. That way you don't need to pass extensions. You can just call the following to get the available extensions.

const { data: { getExtensions } } = this.store.getState();
const extensions = getExtensions();

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sure. Now the constructor is:

constructor(
        private _store: CheckoutStore,
        private _listeners: { [extensionId: string]: IframeEventListener<ExtensionOriginEventMap> },
        private _posters: { [extensionId: string]: IframeEventPoster<HostOriginEvent> },
    ) {}

packages/core/src/extension/extension-messenger.ts Outdated Show resolved Hide resolved
@bc-peng bc-peng force-pushed the IFRAME-MESSENGER branch from e0aac1f to 24beff1 Compare July 14, 2023 03:49
@bc-peng bc-peng force-pushed the IFRAME-MESSENGER branch from 24beff1 to e8ae208 Compare July 14, 2023 04:18
* @alpha
* @param extensionId - The ID of the extension that originally sent the commands.
*/
stopListenExtensionCommand(extensionId: string): void {
Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe we could skip this method for now unless we really need it. IframeEventListener#stopListen doesn't actually remove the references to the handlers previously registered. So we need to do a bit of work to actually do what we want.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Removed. We can circle back to it once we have the use case.

private _extensions: Extension[] | undefined;

constructor(
private _store: CheckoutStore,
Copy link
Contributor

Choose a reason for hiding this comment

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

Preferably use the read-only version, i.e.: ReadableCheckoutStore

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, ReadableCheckoutStore is used in ways I was not aware of before.

Copy link
Contributor

@davidchin davidchin left a comment

Choose a reason for hiding this comment

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

Fantastic work @bc-peng, LGTM! 👏

@bc-peng bc-peng merged commit b43adec into bigcommerce:master Jul 16, 2023
@bc-peng bc-peng deleted the IFRAME-MESSENGER branch July 16, 2023 23:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants