Skip to content

Pr/make web worker friendly #8

New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Open
wants to merge 6 commits into
base: main
Choose a base branch
from

Conversation

Almar
Copy link

@Almar Almar commented May 28, 2021

With this change fetchEventSource can be used from within a web worker

@ghost
Copy link

ghost commented May 28, 2021

CLA assistant check
All CLA requirements met.

@romain-guillot-symphony
Copy link

Hello, is there any blockers to merge this PR?

@arthyn
Copy link

arthyn commented Jul 29, 2021

I'd also like this PR to be merged, this would be a huge help, thanks!

@arthyn
Copy link

arthyn commented Dec 9, 2021

@vishwam hey just bumping this again, if you have a chance to look at it, thanks!

@mbiegert
Copy link

I'm also very interested in seeing this PR getting merged. In regards to the other open PRs, is this project still in use at microsoft/ Azure? Is it still maintained?

@benallfree
Copy link

FYI adding

Object.defineProperty(global, 'self', {
    writable: true,
    enumerable: false,
    configurable: true,
    value: global,
});

in index.ts will also make this node-compatible!

src/fetch.ts Outdated
@@ -74,20 +74,20 @@ export function fetchEventSource(input: RequestInfo, {
let curRequestController: AbortController;
function onVisibilityChange() {
curRequestController.abort(); // close existing request on every visibility change
if (!document.hidden) {
if (!self.document?.hidden) {
Copy link

@benallfree benallfree Dec 7, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Add node compatibility:

// index.ts
if (typeof self === 'undefined') {
  Object.defineProperty(global, 'self', {
    writable: true,
    enumerable: false,
    configurable: true,
    value: global,
  })
}

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi,
Thanks for you comment. I had to look up the use of global. From mdn I now understand we could use globalThis, which is available everywhere, instead of self. What do you think?

NB. this PR is open since 2021-05-21, so I don't think it will be merged anytime soon.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Agreed. Unfortunately I don't think this package is actively maintained, as you mentioned. I'm using a patched version of your fork right now and it works great.

Here's my patch:

diff --git a/node_modules/@microsoft/fetch-event-source/package.json b/node_modules/@microsoft/fetch-event-source/package.json
index 8528735..2e9bac3 100644
--- a/node_modules/@microsoft/fetch-event-source/package.json
+++ b/node_modules/@microsoft/fetch-event-source/package.json
@@ -9,9 +9,9 @@
   },
   "author": "Microsoft",
   "license": "MIT",
-  "main": "lib/cjs/index.js",
-  "module": "lib/esm/index.js",
-  "types": "lib/cjs/index.d.ts",
+  "main": "src/index.ts",
+  "module": "src/index.ts",
+  "types": "src/index.ts",
   "sideEffects": false,
   "scripts": {
     "clean": "rimraf ./lib ./coverage",
diff --git a/node_modules/@microsoft/fetch-event-source/src/fetch.ts b/node_modules/@microsoft/fetch-event-source/src/fetch.ts
index d6612fb..ccfdf6d 100644
--- a/node_modules/@microsoft/fetch-event-source/src/fetch.ts
+++ b/node_modules/@microsoft/fetch-event-source/src/fetch.ts
@@ -74,20 +74,20 @@ export function fetchEventSource(input: RequestInfo, {
         let curRequestController: AbortController;
         function onVisibilityChange() {
             curRequestController.abort(); // close existing request on every visibility change
-            if (!self.document?.hidden) {
+            if (!globalThis.document?.hidden) {
                 create(); // page is now visible again, recreate request.
             }
         }
 
-        if (self.document && !openWhenHidden) {
-            self.document?.addEventListener('visibilitychange', onVisibilityChange);
+        if (globalThis.document && !openWhenHidden) {
+            globalThis.document?.addEventListener('visibilitychange', onVisibilityChange);
         }
 
         let retryInterval = DefaultRetryInterval;
-        let retryTimer = 0;
+        let retryTimer : ReturnType<typeof globalThis['setTimeout']> | undefined = undefined;
         function dispose() {
-            self.document?.removeEventListener('visibilitychange', onVisibilityChange);
-            self.clearTimeout(retryTimer);
+            globalThis.document?.removeEventListener('visibilitychange', onVisibilityChange);
+            globalThis.clearTimeout(retryTimer);
             curRequestController.abort();
         }
 
@@ -97,7 +97,7 @@ export function fetchEventSource(input: RequestInfo, {
             resolve(); // don't waste time constructing/logging errors
         });
 
-        const fetch = inputFetch ?? self.fetch;
+        const fetch = inputFetch ?? globalThis.fetch;
         const onopen = inputOnOpen ?? defaultOnOpen;
         async function create() {
             if (curRequestController) {
@@ -134,8 +134,8 @@ export function fetchEventSource(input: RequestInfo, {
                     try {
                         // check if we need to retry:
                         const interval: any = onerror?.(err) ?? retryInterval;
-                        self.clearTimeout(retryTimer);
-                        retryTimer = self.setTimeout(create, interval);
+                        globalThis.clearTimeout(retryTimer);
+                        retryTimer = globalThis.setTimeout(create, interval);
                     } catch (innerErr) {
                         // we should not retry anymore:
                         dispose();

@yaxiaoliu
Copy link

Do you have any plans to support the running of service workers?

@aroman
Copy link

aroman commented Jun 27, 2023

it seems this project is abandoned, so I have forked it to make it compatible with Cloudflare Workers here and published our forked version on npm at @magiccircle/fetch-event-source-cfworker.

I do not intend to maintain this fork, but dropping a link here in case it's helpful to others!

# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants