-
-
Notifications
You must be signed in to change notification settings - Fork 52
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
feat: add worker related doms #1147
Conversation
jscomp/others/dom.ml
Outdated
@@ -203,6 +209,7 @@ type _htmlVideoElement | |||
type htmlVideoElement = _htmlVideoElement htmlElement_like | |||
type location | |||
type window | |||
type windowProxy |
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.
why do you need windowProxy
? According to MDN it's only available in a "browsing context"
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.
I was reading the MDN page of MessageEvent.source, and it seems that the source can be one of a WindowProxy, MessagePort or a ServiceWorker, so I added it.
After reading the page you linked above, I think it might be totally fine if we use window for all windowproxy...
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.
looks good, just have one question
jscomp/others/dom.ml
Outdated
type 'a _workerGlobalScope | ||
type 'a workerGlobalScope_like = 'a _workerGlobalScope eventTarget_like | ||
type workerGlobalScope = _baseClass workerGlobalScope_like | ||
type _dedicatedWorkerGlobalScope | ||
type dedicatedWorkerGlobalScope = _dedicatedWorkerGlobalScope workerGlobalScope_like | ||
type _serviceWorkerGlobalScope | ||
type serviceWorkerGlobalScope = _serviceWorkerGlobalScope workerGlobalScope_like | ||
type _sharedWorkerGlobalScope | ||
type sharedWorkerGlobalScope = _sharedWorkerGlobalScope workerGlobalScope_like |
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.
@anmonteiro Hi, I add these types since your last review. Could you please take a look at it?
This PR add type definition needed for web worker.
Issue in melange-webapi
Related PR in melange-webapi