Add addHandler and getHandler functions

[Paulinakhew]

TO-DO

• Add tests
• Change the webhookRegistry to have a dictionary with topic as key and a dictionary with {path, topic, webhookHandler} as values
• Completely remove the webhookHandler parameter
• in register function, handle multiple webhooks in a loop as registerAll
• be able to add multiple webhooks
• add tests for registerAll

WHY are these changes introduced?

• Refactored the register function

WHAT is this pull request doing?

• extracted a loadHandler method out of register that adds the handler to the registry
• deleted the handler parameter in register

Type of change

• Patch: Bug (non-breaking change which fixes an issue)
• Minor: New feature (non-breaking change which adds functionality)
• Major: Breaking change (fix or feature that would cause existing functionality to not work as expected)

Checklist

• I have added a changelog entry, prefixed by the type of change noted above
• I have added/updated tests for this change
• I have documented new APIs/updated the documentation for modified APIs (for public APIs)

[paulomarg]

Looking great! Had a couple of comments but the methods themselves are pretty much good to go!

[paulomarg]

Couple of nits, but looks good to me!

(Note that we shouldn't merge this yet!)

[paulomarg]

LGTM! Had a couple of non-blocking comments but I'm otherwise happy with this.

[arsnl]

Hey guys. I understand the need to remove the webhook handler from the Shopify.Webhooks.Registry.register method and changing the Shopify.Webhooks.Registry.webhookRegistry from an array to an object, but I'm not super sure about the current Shopify.Webhooks.Registry.addHandler and Shopify.Webhooks.Registry.getHandler methods.

Shopify.Webhooks.Registry.getHandler

This method is not useless, but the difference between doing Shopify.Webhooks.Registry.getHandler("PRODUCT") and Shopify.Webhooks.Registry.webhookRegistry.PRODUCT is negligible.

Shopify.Webhooks.Registry.addHandler

This method is needed, but I suggest to change it a bit to add the possibility to add an object of handlers.

  addHandler(handlers: {[topic: string]: Omit<WebhookRegistryEntry, "topic">}): void {
    for (const topic in handlers) {
      WebhooksRegistry.webhookRegistry[topic] = handlers[topic]
    }
  }

This is more DX friendly since most of the app's use multiple webhooks. :)

Also, I know this is not covered into this PR, but have you think about a Shopify.Webhooks.Registry.reloadHandlers method or something like that?

Currently, since the webhookRegistry live only in memory, the Shopify.Webhooks.Registry cannot be used as is in a serverless/stateless environment (Vercel, AWS Lambda, Cloud Functions, etc).

What I personally did to fix this behavior was to create a shared constant containing an array of all my webhook registry entries

export const WEBHOOK_REGISTRY_ENTRIES: WebhookRegistryEntry[] = [
  {
    path: "/webhooks/shopify",
    topic: "APP_UNINSTALLED",
    webhookHandler: async (topic, shop, webhookRequestBody) => {
      console.log("APP_UNINSTALLED handler was executed");
    },
  },
];

Then I've added the Shopify.Webhooks.Registry.reloadHandlers method

/**
 * Add method Shopify.Webhooks.Registry.reloadHandlers
 *
 * Give a way to reload the webhooks handlers.
 * This is necessary to reload the registered webhooks before processing them because
 * on serverless/stateless application the webhookRegistry is not in memory anymore.
 * Also very important to support a server reboot.
 */
declare module "@shopify/shopify-api/dist/webhooks/registry" {
  interface RegistryInterface {
    reloadHandlers: () => void;
  }
}

Shopify.Webhooks.Registry.reloadHandlers = () => {
  // Only reload if the webhookRegistry is empty
  if (!Shopify.Webhooks.Registry.webhookRegistry.length) {
    Shopify.Webhooks.Registry.webhookRegistry = WEBHOOK_REGISTRY_ENTRIES;
  }
};

After authentication, I register them in a loop

  await Promise.all(
    WEBHOOK_REGISTRY_ENTRIES.map(async (webhookRegistryEntry) => {
      const response = await Shopify.Webhooks.Registry.register({
        shop,
        accessToken,
        ...webhookRegistryEntry,
      });

      if (!response.success) {
        console.log(
          `Failed to register ${webhookRegistryEntry.topic} webhook: ${response.result}`
        );
      } else {
        console.log(
          `${webhookRegistryEntry.topic} Webhook was successfully registered`
        );
      }
    })
  );

And when the server (in my case, Vercel which is an AWS lambda behind the scene) receive a Webhook call from Shopify, I reload the handlers since they are not in memory.

  Shopify.Webhooks.Registry.reloadHandlers();

  try {
    await Shopify.Webhooks.Registry.process(req, res);
    console.log(`Webhook processed, returned status code 200`);
  } catch (error) {
    console.log(`Failed to process webhook: ${error}`);
  }

What I'm suggesting is maybe;

• Add the property WEBHOOK_REGISTRY_ENTRIES to the Shopify.Context
• Adapt the Shopify.Webhooks.Registry.register method be like this register(options: {shop: string; accessToken: string; deliveryMethod?: DeliveryMethod;}): Promise<RegisterReturn> and to use Shopify.Context.WEBHOOK_REGISTRY_ENTRIES under the hood.
• Add the Shopify.Webhooks.Registry.reloadHandlers method that also use Shopify.Context.WEBHOOK_REGISTRY_ENTRIES under the hood.
• Probably no need of the Shopify.Webhooks.Registry.addHandler method.

I just open a PR about those suggestions: #226

[cmnstmntmn]

@arsnl hey,

i'm trying your example:

i get

{
  webhookSubscriptionCreate: {
    userErrors: [],
    webhookSubscription: { id: 'gid://shopify/WebhookSubscription/1069434383796' }
  }
}
// -> APP_UNINSTALLED Webhook was successfully registered

however, when i uninstall the app the route /api/shopify/webhooks is called

    Shopify.Webhooks.Registry.reloadHandlers = () => {
      // Only reload if the webhookRegistry is empty
      if (!Shopify.Webhooks.Registry.webhookRegistry.length) {
        Shopify.Webhooks.Registry.webhookRegistry = WEBHOOK_REGISTRY_ENTRIES;
      }
    };

    console.log(req.body);

    const isWebhookPath = Shopify.Webhooks.Registry.isWebhookPath(
      "/api/shopify/webhooks"
    );

    console.log("isWebhookPath:", isWebhookPath); // -> true

    try {
      const response = await Shopify.Webhooks.Registry.process(req, res);

      console.log(response);

      console.log(`Webhook processed, returned status code 200`);
    } catch (error) {
      console.log(`Failed to process webhook: ${error}`);
    }

however

No webhook is registered for topic app/uninstalled

[pveen2]

Shopify.Webhooks.Registry.reloadHandlers();

  try {
    await Shopify.Webhooks.Registry.process(req, res);
    console.log(`Webhook processed, returned status code 200`);
  } catch (error) {
    console.log(`Failed to process webhook: ${error}`);
  }

+1 Experiencing exactly the same in a NEXT.js app, did you solve it @cmnstmntmn