Skip to content

add URL typing to webworker #223

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
Mar 29, 2017
Merged

add URL typing to webworker #223

merged 4 commits into from
Mar 29, 2017

Conversation

vincentbel
Copy link
Contributor

@vincentbel vincentbel commented Mar 28, 2017

URL is also available in worker. (URL IDL spec). Fix microsoft/TypeScript#14878.


Questions:

  1. URL tying is already defined in inputfiles/browser.webidl.xml#L11367-L11395, so it is only need to added to worker. So I added "flavor": "Worker", but it seems doesn't work.(same as "flavor": "All" for URLSearchParams) 😅
  2. I'm not sure whether the URL and URLSearchParams should be added to WorkerGlobalScope. It is signed as Exposed=(Window,Worker) in the spec IDL
  3. The spec IDL use USVString, but I use string here (keep it same as previous added URLSearchParams typing). Doesn't it matters?

Updates:

  1. Just add URL and URLSearcParams to knownWorkerInterfaces.json.
  2. URL and URLSearchParams shouldn't be added to WorkerGlobalScope.(The same as fetch related typing)

@msftclas
Copy link

@VincentBel,
Thanks for your contribution.
To ensure that the project team has proper rights to use your work, please complete the Contribution License Agreement at https://cla.microsoft.com.

It will cover your contributions to all Microsoft-managed open source projects.
Thanks,
Microsoft Pull Request Bot

@msftclas
Copy link

@vincentbel, thanks for signing the contribution license agreement. We will now validate the agreement and then the pull request.

Thanks, Microsoft Pull Request Bot

@@ -13858,6 +13859,25 @@ interface ImageBitmap {
close(): void;
}

interface URL {
Copy link
Contributor

Choose a reason for hiding this comment

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

interface URL is already defined in this file. no need to redefine it.

@@ -1484,6 +1486,25 @@ interface ImageBitmap {
close(): void;
}

interface URL {
Copy link
Contributor

Choose a reason for hiding this comment

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

just edit inputfiles\knownWorkerInterfaces.json to add URL and URLSearcParams.

@vincentbel
Copy link
Contributor Author

@mhegazy updated. But ci is still failing. I have see some other PRs, and they seems failed with the same error.

@@ -91,11 +91,13 @@
"SyncEventInit",
"SyncManager",
"USVString",
"URL",
"URLSearchParams",
Copy link
Contributor

Choose a reason for hiding this comment

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

looks like you need ObjectURLOptions as well

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Added.

@mhegazy
Copy link
Contributor

mhegazy commented Mar 29, 2017

thanks!

@mhegazy mhegazy merged commit cac1198 into microsoft:master Mar 29, 2017
@vincentbel vincentbel deleted the webworker-add-URL branch April 14, 2017 09:25
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