-
Notifications
You must be signed in to change notification settings - Fork 179
Pr/make web worker friendly #8
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
Conversation
...o pr/make_web_worker_friendly # Conflicts: # package.json
CLA assistant check
All CLA requirements met.
romain-guillot-symphony
commented
Jul 7, 2021
Hello, is there any blockers to merge this PR?
arthyn
commented
Jul 29, 2021
I'd also like this PR to be merged, this would be a huge help, thanks!
arthyn
commented
Dec 9, 2021
@vishwam hey just bumping this again, if you have a chance to look at it, thanks!
mbiegert
commented
Mar 24, 2022
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
commented
Dec 7, 2022
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
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.
@benallfree
benallfree
Dec 7, 2022
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();
yaxiaoliu
commented
May 19, 2023
Do you have any plans to support the running of service workers?
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!
With this change fetchEventSource can be used from within a web worker