add types for escape and unescape methods #18813 by wbhob · Pull Request #19015 · microsoft/TypeScript · GitHub
Skip to content

add types for escape and unescape methods #18813#19015

Merged
mhegazy merged 3 commits into
microsoft:masterfrom
wbhob:master
Nov 13, 2017
Merged

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

Conversation

@wbhob

@wbhob wbhob commented Oct 8, 2017

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

wbhob added 2 commits October 7, 2017 23:24
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

msftclas commented Oct 8, 2017

Copy link
Copy Markdown

@mhegazy

mhegazy commented Oct 9, 2017

Copy link
Copy Markdown
Contributor

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

@mhegazy mhegazy closed this Oct 9, 2017
@akehir

akehir commented Oct 16, 2017

Copy link
Copy Markdown

@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

mhegazy commented Oct 16, 2017

Copy link
Copy Markdown
Contributor

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

@mhegazy mhegazy reopened this Oct 16, 2017
@mhegazy

mhegazy commented Nov 9, 2017

Copy link
Copy Markdown
Contributor

@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

wbhob commented Nov 9, 2017

Copy link
Copy Markdown
Contributor Author

I'll look into fixing those tests soon

@mhegazy mhegazy merged commit c2f0c58 into microsoft:master Nov 13, 2017
@wbhob

wbhob commented Nov 13, 2017

Copy link
Copy Markdown
Contributor Author

@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