KeithHenry / chromeExtensionAsync

Promise wrapper for the Chrome extension API so that it can be used with async/await rather than callbacks
MIT License
229 stars 32 forks source link

await in for loop #4

Closed hnakamur closed 7 years ago

hnakamur commented 7 years ago

First of all, thanks for a wonderful wrapper library for Chrome extension API!

Can I ask you a question? I have a trouble in using await in a for loop. I'm running Chrome version 61.0.3163.91 (Official Build) (64-bit) on Windows 10.

In my extension FormatLink-Chrome: Format a link and title of the active tab of Chrome and copy it the clipboard, I would like to remove all context menu items and then create menu items needed.

So I tried to call await chrome.contextMenus.create in a for loop like below:

https://github.com/hnakamur/FormatLink-Chrome/blob/f0080158ac09fa2816808d3ec0453f5a4f8e014e/common.js#L55-L77

async function createContextMenus(options) {
  await chrome.contextMenus.removeAll();
  if (options.createSubmenus) {
    var count = getFormatCount(options);
    for (var i = 0; i < count; i++) {
      var format = options['title' + (i + 1)];
      // NOTE: Some of menu items weren't created when I added 'await' here.
      // So I deleted 'await' as a workaround.
      chrome.contextMenus.create({
        id: "format-link-format" + (i + 1),
        title: "as " + format,
        contexts: ["link", "selection", "page"]
      });
    }
  } else {
    var defaultFormat = options['title' + options['defaultFormat']];
    await chrome.contextMenus.create({
      id: "format-link-format-default",
      title: "Format Link as " + defaultFormat,
      contexts: ["link", "selection", "page"]
    });
  }
}

The createContextMenus is called from 2 places:

When I added await before chrome.contextMenus.create, the menu items are all created successfully in initialization, but the some of the menu items were not created when I clicked the "Save changes" button in the options page.

Concretely speaking, all 4 menu items were created successfully in initialization, but only the first two menu items were created when I clicked the "Save changes" button.

There was no error printed in console which is opened by clicking the link options.html (iframe) to the right of inspecting views in chrome://extensions/

As a workaround, I deleted await before chrome.contextMenus.create and all menu items are created successfully both in initialization and when I clicked the "Save changes" button.

Is there a way to use await in for loop properly?

For an experiment, I created a HTML page which uses await in loop without chrome extension API. https://bl.ocks.org/hnakamur/f456f37e849a6853ea2ace0c47a372b7 When I opened the devtool, the following output was printed as expected.

foo 0
foo 1
foo 2
foo 3

Also I created a similar extension for Firefox hnakamur/FormatLink-Firefox: A Firefox web-extension to copy the URL and the title or the selected text to clipboard.

I used the almost same code for removing all menu items and creating menu items like below.

https://github.com/hnakamur/FormatLink-Firefox/blob/0e5d21c5ab4d87b758ef2da4f671406bc64a940b/common.js#L50-L83

function creatingContextMenuItem(props) {
  return new Promise((resolve, reject) => {
    browser.contextMenus.create(props, () => {
      var err = browser.runtime.lastError;
      if (err) {
        reject(err);
      } else {
        resolve();
      }
    });
  });
}

async function createContextMenus(options) {
  await browser.contextMenus.removeAll();
  if (options.createSubmenus) {
    var count = getFormatCount(options);
    for (var i = 0; i < count; i++) {
      var format = options['title' + (i + 1)];
      await creatingContextMenuItem({
        id: "format-link-format" + (i + 1),
        title: "as " + format,
        contexts: ["link", "selection", "page"]
      });
    }
  } else {
    var defaultFormat = options['title' + options['defaultFormat']];
    await creatingContextMenuItem({
      id: "format-link-format-default",
      title: "Format Link as " + defaultFormat,
      contexts: ["link", "selection", "page"]
    });
  }
}

Most Firefox webextension APIs return a Promise like menus.removeAll(). However menus.create() uses callback style, so I created a wrapped function creatingContextMenuItem wihch returns a Promise.

And I use await before creatingContextMenuItem All 4 menu items are runs successfully both in initialization and when I click the "Save changes" in the options page.

hnakamur commented 7 years ago

Also I tried with a handwritten Promise wrapper for chrome.contextMenus.create at https://github.com/hnakamur/FormatLink-Chrome/commit/64af8cae907b4f0f04581b69dcaf64596a5dd54e The problem still remains. Only the first two menu items are created when I click the "Save changes" button.

hnakamur commented 7 years ago

I found an answer: Promise.all The problem went away with this fix. Propery wait multilple promises by hnakamur · Pull Request #4 · hnakamur/FormatLink-Chrome

Sorry for the noise. Thanks again for a wonderful library!

KeithHenry commented 7 years ago

Cheers, glad to hear you've found it useful. Sorry for the late reply (I'm on holiday in Spain).

Under the hood this library does a pretty similar promisfy to the creatingContextMenuItem that you rolled yourself :-) Here is the relevant code: https://github.com/KeithHenry/chromeExtensionAsync/blob/8952e249e790a4034caaf82eea653873abf5e417/chrome-extension-async.js#L31-L62

The additional code is error handling, dealing with more complex return types, and allowing the old callback syntax to still work (so users of the library don't have to upgrade everything to await at once).

You have to be careful with async in loops, as it runs the callbacks in sequence. You generally want to map to an array of promises and then use all on that, as you've found.