Skip to content

Introduce Messageable and Transferable for postMessage #534

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

Merged
merged 4 commits into from
Jul 17, 2018
Merged

Introduce Messageable and Transferable for postMessage #534

merged 4 commits into from
Jul 17, 2018

Conversation

@mhegazy
Copy link
Contributor

mhegazy commented Jul 13, 2018

Map and Set are defined in the ES6 lib file. you will need to split the declaration into two overloads, and have the Map/Set ones in dom.iterable.generated.d.ts.

@mhegazy
Copy link
Contributor

mhegazy commented Jul 13, 2018

@saschanaz can you please review.

@nikeee
Copy link
Contributor Author

nikeee commented Jul 13, 2018

Map and Set are defined in the ES6 lib file. you will need to split the declaration into two overloads, and have the Map/Set ones in dom.iterable.generated.d.ts

How do I add types that sould only be included in dom.iterable.generated.d.ts.

@mhegazy
Copy link
Contributor

mhegazy commented Jul 13, 2018

@nikeee
Copy link
Contributor Author

nikeee commented Jul 13, 2018

I couldn't find any way to include different type defs without changing the emitter code. I removed the Messageable type def for now.

In the future, we should consider adding OffscreenCanvas to the Transferable type def since it's also Transferable according to the spec IDL. However, OffscreenCanvas is not available in lib.d.ts.

Copy link
Contributor

@saschanaz saschanaz left a comment

Choose a reason for hiding this comment

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

IMO okay for now, but I ultimately would prefer more automatic way as we can detect Transferable extended attribute from IDL. We may do what we are already doing for createElement.

"postMessage": {
"name": "postMessage",
"override-signatures": [
"postMessage(message: any, transferList?: Transferable[]): void"
Copy link
Contributor

Choose a reason for hiding this comment

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

Why transferList instead of transfer here?

Copy link
Contributor Author

@nikeee nikeee Jul 14, 2018

Choose a reason for hiding this comment

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

I named it transferList because it is landed so in the MDN docs.

https://developer.mozilla.org/en-US/docs/Web/API/Worker/postMessage

I can change it to transfer if it's preferred.

I'd prefer the automatic way as well, but for now I think this suffices.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I changed it to transfer.

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.

3 participants