Promise as a if condition

Question

I'm quite bad with JS promises and async and I face a problem about it.

First things first the simplified code:

self.addEventListener('push', (event) => {
    const data = event.data ? event.data.json() : {}
    if (isClientFocused()) {
        console.log('Don\'t need to show a notification.');
        return;
    }
    event.waitUntil(
        self.registration.showNotification(data.title, data)
    )
});

function isClientFocused() {
    return self.clients.matchAll({
        type: 'window',
        includeUncontrolled: true
    })
        .then((windowClients) => {
            let clientIsFocused = false;
            for (let i = 0; i < windowClients.length; i++) {
                const windowClient = windowClients[i];
                if (windowClient.focused) {
                    clientIsFocused = true;
                    break;
                }
            }
            return clientIsFocused;
        });
}

The concept is quite straight forward: my function isClientFocused return true or false depending on the situation and I want to use this value as a condition in my if in my event listener.

I precise that isClientFocused function works well and everytime return the result as expected, but when used in the if condition, it always log Don't neet to show notification.

I guess it's just an implementation problem but I'm a bit lost right now.

Thanks in advance for your help.

Alexander Nenashev · Accepted Answer · 2023-07-01T16:25:46.073

1

Just add async/await because isClientFocused() returns a promise and it's truthy and condition is always true. With the resolved value with await you get a proper logic. Also you should call event.waitUntil immediately (sync):

self.addEventListener('push', event => event.waitUntil((async () => {
   if (await isClientFocused()) {
       console.log('Don\'t need to show a notification.');
       return;
   const data = event.data?.json() ?? {};
   return self.registration.showNotification(data.title, data)
})()));

I suggest to refactor isClientFocused() with async/await and use async/await as much as possible to avoid then(). Also you could use Array::some() instead of your loop.

In my experience async/await leads to less bugs and more readable and maintainable when async/await is learned.

It's just amazing when I refactor a buggy then() code to async/await the bugs disappeared automatically.

async function isClientFocused() {
    const windowClients = await self.clients.matchAll({
        type: 'window',
        includeUncontrolled: true
    });
    return windowClients.some(windowClient => windowClient.focused);
}

edited Jul 01 '23 at 16:25

answered Jul 01 '23 at 11:13

Alexander Nenashev

8,775
2
6
17

I edited as you said and it seems that now the difference between focused or not works. But when not focused I get an error: serviceworker.js:39 Uncaught (in promise) DOMException: Failed to execute 'waitUntil' on 'ExtendableEvent': The event handler is already finished and no extend lifetime promises are outstanding. So it seems that if not focused, everything works welle but when focused it does not – Tartempion34 Jul 01 '23 at 11:53
@Tartempion34 i think it's not related, another issue – Alexander Nenashev Jul 01 '23 at 11:55
@Tartempion34 seems you can't use a promise here and should call waitUntil immediately. waitUntil is from which library? – Alexander Nenashev Jul 01 '23 at 11:58
from ExtendableEvent in service workers: https://developer.mozilla.org/en-US/docs/Web/API/ExtendableEvent/waitUntil – Tartempion34 Jul 01 '23 at 12:01
@Tartempion34 fixed – Alexander Nenashev Jul 01 '23 at 12:08
I'm not sure to understand what is wrote here :o But it does not work now it does not do anything no console log no call to notfication :o – Tartempion34 Jul 01 '23 at 12:16
Let us [continue this discussion in chat](https://chat.stackoverflow.com/rooms/254329/discussion-between-tartempion34-and-alexander-nenashev). – Tartempion34 Jul 01 '23 at 12:34
[Never pass an `async function` as the executor to `new Promise`](https://stackoverflow.com/q/43036229/1048572)! – Bergi Jul 01 '23 at 13:21
@Bergi the link is useless. unless i'm explained when this approach fails i'm not jumping into the bandwagon with the flock – Alexander Nenashev Jul 01 '23 at 13:27
@Bergi there's no diferrence with using ’then’. the returned promise isn't used anywhere – Alexander Nenashev Jul 01 '23 at 13:29
1

@AlexanderNenashev I'm not suggesting to us `.then()`. I'm suggesting to avoid `new Promise`, as that introduces the need for `try`/`catch` to handle errors correctly (which you initially forgot). Use `event.waitUntil((async () => { if (await isClientFocused()) return; const data = event.data?.json() ?? {}; await self.registration.showNotification(data.title, data); })());` - much simpler and less error-prone! – Bergi Jul 01 '23 at 13:37
@Bergi now i see, looks not perfect (iife) but better, thanks a lot – Alexander Nenashev Jul 01 '23 at 13:43
@AlexanderNenashev If you don't like the IIFE, you can declare it as a named function or define a helper `const run = f => f()` and use `event.waitUntil(run(async () => { … }));`, still better than abusing `new Promise` for this purpose – Bergi Jul 01 '23 at 13:50
@Bergi thanks for your answer but still doesn't do anything. Even added some log but stays empty like if nothing happened at al – Tartempion34 Jul 01 '23 at 13:50
@Bergi I don't know if it can help but Originaly I'm trying to implement the part "The Exception to the Rule" of this : https://web-push-book.gauntface.com/common-notification-patterns/ – Tartempion34 Jul 01 '23 at 13:53

score 0 · Answer 2 · answered Jul 01 '23 at 11:21

Cause of this, isClientFocused function is a Promise, when you call it, you have to wait for complete this function.

The code should look like this;

self.addEventListener('push', (event) => {
    const data = event.data ? event.data.json() : {}
    isClientFocused().then(res=>{
      if (res) {
          console.log('Don\'t need to show a notification.');
          return;
      }
      event.waitUntil(self.registration.showNotification(data.title, data))
    })
});

Note : Promise gives an output similar to this if you do not wait for a function to run ;

{"_h": 0, "_i": 3, "_j": {"_h": 0, "_i": 1, "_j": 3, "_k": null}, "_k": null}

score 0 · Answer 3 · answered Jul 01 '23 at 14:44

Finally I fixed it like that:

async function isClientFocused() {
    const windowClients = await self.clients.matchAll({
        type: 'window',
        includeUncontrolled: true
    });
    return windowClients.some(windowClient => windowClient.focused);
}

self.addEventListener('push', async (event) => {
    if (await isClientFocused()) {
        console.log("Don't need to show a notification.");
        return;
    }

    const data = event.data ? event.data.json() : {};
    self.registration.showNotification(data.title, data);
});

Thanks to @Bergi and @AlexanderNenashev for the help

Promise as a if condition

3 Answers3