Skip to content

add types for escape and unescape methods #18813 - #19015

Merged
Mohamed Hegazy (mhegazy) merged 3 commits into
microsoft:masterfrom
wbhob:master
Nov 13, 2017
Merged

add types for escape and unescape methods #18813#19015
Mohamed Hegazy (mhegazy) merged 3 commits into
microsoft:masterfrom
wbhob:master

Conversation

@wbhob

Copy link
Copy Markdown
Contributor

Although the issue is marked working as expected, it is important to mention that most major browsers maintain support for escape and unescape, and some javascript codebases moving to typescript may have escape and unescape in them. They are valid JavaScript, and thus they should be included in the global definition.

Fixes #18813

although the issue is marked working as expected, it is important to mention that most major browsers maintain support for escape and unescape, and some javascript codebases moving to typescript may have escape and unescape in them. They are valid JavaScript, and thus they should be included in the global definition.
@msftclas

Microsoft Contribution License Agreements (msftclas) commented Oct 8, 2017

Copy link
Copy Markdown

CLA assistant check
All CLA requirements met.

@mhegazy

Copy link
Copy Markdown
Contributor

Thanks for the contribution. but the omission of these functions from the library is intentional. please see #18813 (comment)

@akehir

Copy link
Copy Markdown

Mohamed Hegazy (@mhegazy) - In the current ES Draft spec, as well as in MDN I see no reference to escape / unescape being deprecated anymore. What is your reference for it still being deprecated?

@mhegazy

Copy link
Copy Markdown
Contributor

u are right. there does not seem to be a deprecation notice any longer.

@mhegazy

Copy link
Copy Markdown
Contributor

Wilson Hobbs (@wbhob) mind refreshing the PR and addressing the failing tests?

Comment thread tests/lib/lib.d.ts
@@ -1,14 +1,14 @@
/*! *****************************************************************************

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.

do not change this file

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.

I think that was automatically generated

@wbhob

Copy link
Copy Markdown
Contributor Author

I'll look into fixing those tests soon

@mhegazy
Mohamed Hegazy (mhegazy) merged commit c2f0c58 into microsoft:master Nov 13, 2017
@wbhob

Copy link
Copy Markdown
Contributor Author

🎉 Yay! Thank you

@microsoft Microsoft (microsoft) locked and limited conversation to collaborators Jun 14, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants