Skip to content

add URL typing to webworker - #223

Merged
Mohamed Hegazy (mhegazy) merged 4 commits into
microsoft:masterfrom
vincentbel:webworker-add-URL
Mar 29, 2017
Merged

Mohamed Hegazy (mhegazy) merged 4 commits into
microsoft:masterfrom
vincentbel:webworker-add-URL

Conversation

@vincentbel

@vincentbel Vincent Bel (vincentbel) commented Mar 28, 2017 •

Copy link
Copy Markdown
Contributor

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

@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

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

Thanks, Microsoft Pull Request Bot

Comment thread baselines/dom.generated.d.ts Outdated
close(): void;
}

interface URL {

Copy link
Copy Markdown
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.

Comment thread baselines/webworker.generated.d.ts Outdated
close(): void;
}

interface URL {

Copy link
Copy Markdown
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
Copy Markdown
Contributor Author

Mohamed Hegazy (@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
Copy Markdown
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
Copy Markdown
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
Copy Markdown
Contributor

thanks!

@mhegazy
Mohamed Hegazy (mhegazy) merged commit cac1198 into microsoft:master Mar 29, 2017
@vincentbel
Vincent Bel (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