-
Notifications
You must be signed in to change notification settings - Fork 2.6k
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
replace window with 'self' in workers automatically #1769
Conversation
and cache the value
Although if users build hlsjs themselves from source, this is another thing they will have to handle. |
} | ||
// see https://stackoverflow.com/a/11237259/589493 | ||
/* eslint-disable-next-line no-undef */ | ||
export default typeof window === 'undefined' ? self : window; |
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.
oh ok i guess i understand now
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.
nice one, saves quite a few codelines 👍 :)
Closing this, as we have solved it via here: #1841 |
I think this is still valid. How is it related to the other issue? It is a way of not needing getSelfScope() #1756 Not actually sure if it's better being more explicit though instead of replacing window |
Yes, it's not really the same problem. #1841 could have been solved by passing the global for the current environment ( I ended up here before and decided not to make myself even less popular with my opinions, but I'm wondering, why not just use Maybe it's too hard to ensure that people won't add At least Also, in case you want set the environment to specific files with eslint, their comments should work |
Great point I didn't realise 'self' was an alias of 'window'. If it really does work like this in all browsers, and typescript understands this, I think we should just switch to 'self' The Mozilla docs also mention this https://developer.mozilla.org/en-US/docs/Web/API/Window/self |
I'm late to the game on this and can't claim to know by experience. As I understand it, it was there way before WebWorkers (maybe since the start), similar to Found an old stack overflow question indicating it's been supported at least since April 2010 (IE8): I tried the typescript online REPL and it seems to alias self.document;
self.localStorage;
self.asdf; The last row got me "Property 'asdf' does not exist on type 'Window'." You may have to watch out for shadowing though. But maybe typescript catches it? Lines 249 to 275 in 741d531
(this particular place might not be a problem) |
This PR will...
use the webpack imports loader to automatically replace
window
withself
when running in a worker.Why is this Pull Request needed?
It removes some duplicated code, and also caches
window
orself
, so might have slightly better performance.E.g in the build
is injected at the top of each module, and in this built module
window
has been rewritten asdemuxer_window
.