If/else loop executes both conditions

Question

OK, so I have the following code. The idea is to check if a page exists, if not, open a new tab. If exists, navigate to it.

chrome.browserAction.onClicked.addListener(function(tab) {
  function check() {
        chrome.tabs.query(
            {currentWindow: true, url: 'https://www.google.com/*'},
            function(tabs) {
            tabs.forEach(function(tab) {
              console.log('Tab ID, URL: ', tab.id, ' ', tab.url);
                  if(tab.url !== '')
                  {
                    var updateProperties = {"active": true};
                    chrome.tabs.update(tab.id, updateProperties, function(tab){ });
                    console.log('Assign true to functon');
                    return true;
                  }
            });
        });
      };

      if(check() === true) {
        console.log('Function is true, do nothing')
      }
      else {
        console.log('Function is false, open page')
        chrome.tabs.create({ url: "https://www.google.com"});
      }
});

The first part works correctly, but the second one executes both true and false sections. It's like it executes the if/else statement before the function?

I would have thought the lack of a `return` statement in your `check()` function would hint towards something being very wrong here. — Niet the Dark Absol, May 08 '17 at 10:26
only way it is possible is if your listener is bound to 2 different elements and called for each of them for the same event. — Vladimir M, May 08 '17 at 10:26
It definitely *doesn't* execute both branches. In fact, using *that* code, it definitely never executes the `if` branch, because `check` never returns a value, so `check() === true` will never be true. If you're seeing "Function is true, do nothing" in the console, it's not coming from that code. — T.J. Crowder, May 08 '17 at 10:27
Also, beware that based on the fact it accepts a callback rather than returning an array, it would seem [`chrome.tabs.query`](https://developer.chrome.com/extensions/tabs#method-query) is *asynchronous* (although irritatingly it doesn't actually say that in the documentation). That means `check` **cannot** return a flag for whether the tab exists, [see this question's answers for details](http://stackoverflow.com/questions/14220321/how-do-i-return-the-response-from-an-asynchronous-call). You'd want your create logic *inside* the `query` callback. — T.J. Crowder, May 08 '17 at 10:30
just want to add that chrome.tabs.query is asynchronous, you're thinking as your code is synchronous — Karim, May 08 '17 at 10:33
@Karim: Thanks for confirming that, I'll close as a duplicate. Odd the docs don't say so explicitly. — T.J. Crowder, May 08 '17 at 10:37

score 2 · Answer 1 · edited May 23 '17 at 12:34

There are a few issues in the code:

check never returns anything, so check() === true will never be true. If you're seeing "Function is true, do nothing" in the console, it isn't coming from that code.
check cannot return anything useful, because chrome.tabs.query is asynchronous*.

So we get rid of check entirely and move the create logic into the callback:

chrome.browserAction.onClicked.addListener(function(tab) {
    chrome.tabs.query({
        currentWindow: true,
        url: 'https://www.google.com/*'
    }, function(tabs) {
        const tab = tabs.find(tab => tab.url !== '');
        if (tab) {
            const updateProperties = {
                "active": true
            };
            chrome.tabs.update(tab.id, updateProperties, function(tab) {});
        } else {
            chrome.tabs.create({
                url: "https://www.google.com"
            });
        }
    }
});

A couple of side notes because I haven't written a Chrome extension in a long time:

Do you really need to search tabs? Given that you're passing url into query, wouldn't if (tabs.length) tell you the tab already exists?
Do you really need to pass the no-op function into chrome.tabs.update?

* ~~I assume it is, because it accepts a callback it calls rather than returning the array. Irritatingly, the documentation doesn't actually say that.~~ Karim has confirmed it does in the comments, I'll trust him. :-)

score 0 · Answer 2 · answered May 08 '17 at 10:35

When you add callbacks, the return value doesn't come back directly in the function response - so in your case, check() returns nothing. The callback used inside check() returns a value, but this value isn't actually returned by the function, it essentially goes nowhere. There are at least 2 possible solutions to this:

1. put the `if/else` inside the callback, and run the function once independently:

chrome.browserAction.onClicked.addListener(function(tab) {
  function check() {
    chrome.tabs.query({
        currentWindow: true,
        url: 'https://www.google.com/*'
      },
      function(tabs) {
        tabs.forEach(function(tab) {
          console.log('Tab ID, URL: ', tab.id, ' ', tab.url);
          if (tab.url !== '') {
            var updateProperties = {
              "active": true
            };
            chrome.tabs.update(tab.id, updateProperties, function(tab) {});
            console.log('Assign true to functon');
            // right here!
            console.log('Function is true, do nothing')
          } else {
            // and here!
            console.log('Function is false, open page')
            chrome.tabs.create({
              url: "https://www.google.com"
            });
          }
        });
      });
  };
  // execute it once
  check();
});

2. use a `Promise`

chrome.browserAction.onClicked.addListener(function(tab) {
  function check() {
    // return the promise
    return new Promise(function(resolve, reject) {
      chrome.tabs.query({
          currentWindow: true,
          url: 'https://www.google.com/*'
        },
        function(tabs) {
          tabs.forEach(function(tab) {
            console.log('Tab ID, URL: ', tab.id, ' ', tab.url);
            if (tab.url !== '') {
              var updateProperties = {
                "active": true
              };
              chrome.tabs.update(tab.id, updateProperties, function(tab) {});
              console.log('Assign true to functon');
              resolve('pass any extra success info you might need here');
            } else {
              reject('pass any extra rejection info you might need here')
            }
          });
        });
    })
  };

  // promise usage
  check().then(function(msg) {
    console.log('Function is true, do nothing. Extra info:', msg)
  }).catch(function(msg) {
      console.log('Function is false, open page. Extra info:', msg)
      chrome.tabs.create({
        url: "https://www.google.com"
      });
    }
  })
});

If/else loop executes both conditions

2 Answers2

1. put the if/else inside the callback, and run the function once independently:

2. use a Promise

1. put the `if/else` inside the callback, and run the function once independently:

2. use a `Promise`