-
Notifications
You must be signed in to change notification settings - Fork 140
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Pr/make web worker friendly #8
base: main
Are you sure you want to change the base?
Conversation
…o pr/make_web_worker_friendly # Conflicts: # package.json
Hello, is there any blockers to merge this PR? |
I'd also like this PR to be merged, this would be a huge help, thanks! |
@vishwam hey just bumping this again, if you have a chance to look at it, thanks! |
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? |
FYI adding
in |
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) { |
There was a problem hiding this comment.
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,
})
}
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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();
Do you have any plans to support the running of service workers? |
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! |
With this change fetchEventSource can be used from within a web worker