Re: [w3c/push-api] Rewrite .subscribe() to fix various bugs (PR #350)

@beverloo approved this pull request.

Generally these changes look good to me, modulo one type hint that should be generalized.

Our implementation runs one of the steps out of order: the `userVisibleOnly` requirement is only checked quite a late in the process, as there may be (toady hypothetical) cases in which we don't require it, e.g. for extensions. 

>          </li>
-        <li>Return |promise| and continue the following steps asynchronously.
+        <li>If the |options| argument does not include a non-null value for the
+        {{PushSubscriptionOptionsInit/applicationServerKey}} member, and the <a>push service</a>
+        requires one to be given, return [=a promise rejected with=] a {{"NotSupportedError"}}
+        {{DOMException}}.
+        </li>
+        <li>Let |promise| be [=a new promise=].
+        </li>
+        <li>Let |global:Window| be [=this=]' [=relevant global object=].

The relevant global object within a service worker isn't a window, so this definition may be invalid. None of the uses of `global` seem to require it to be a Window though, the worker global (in my reading) also has a task queue.

-- 
Reply to this email directly or view it on GitHub:
https://github.com/w3c/push-api/pull/350#pullrequestreview-1018633728
You are receiving this because you are subscribed to this thread.

Message ID: <w3c/push-api/pull/350/review/1018633728@github.com>