Skip to content
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 api #19

Open
wants to merge 15 commits into
base: master
Choose a base branch
from

Conversation

tonyfettes
Copy link

@tonyfettes tonyfettes commented Jun 26, 2024

This PR adds Web Worker DOM bindings.

This PR should be used with PR #1147 in melange.

Fix #18

[@mel.get] external origin: t => string = "origin";
[@mel.get] external lastEventId: t => string = "lastEventId";
[@mel.get] external source: t => [@mel.unwrap] [ | `MessagePort(Dom.messagePort) | `ServiceWorker(Dom.serviceWorker) ] = "source";
[@mel.get] external ports: t => Js.Array.t(Dom.messagePort) = "ports";
Copy link
Member

Choose a reason for hiding this comment

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

you can use array instead of Js.Array.t

Copy link
Author

Choose a reason for hiding this comment

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

Sure, but how is that different? I went to its definition and found out that 'a t = 'a array.

Copy link
Member

Choose a reason for hiding this comment

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

it's equivalent indeed

@tonyfettes tonyfettes marked this pull request as ready for review August 5, 2024 16:22

include Webapi__Dom__Types;

external window: Dom.window = "window";
external dedicatedWorkerGlobalScope: Dom.dedicatedWorkerGlobalScope = "self";
Copy link
Author

Choose a reason for hiding this comment

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

@davesnx Is it better to have a globalThis here? Like

external globalThis: 'a = "globalThis"

Copy link
Member

Choose a reason for hiding this comment

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

Yes, we should add globalThis

@tonyfettes
Copy link
Author

@davesnx can you please re-run the job for me? The upstream PR is merged and I believed these runs can succeed now.

@davesnx
Copy link
Member

davesnx commented Aug 12, 2024

Done

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

Successfully merging this pull request may close these issues.

Missing web worker related API
2 participants