Skip to content

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

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.

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.

"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