Skip to content
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

Added formatToParts to es5.d.ts #15369

Merged
merged 1 commit into from
May 24, 2017
Merged

Conversation

donaldpipowitch
Copy link
Contributor

Added Intl.DateTimeFormat.prototype.formatToParts() to es5.d.ts. See here.

Not sure if tests are mandatory for interface changes?

@msftclas
Copy link

This seems like a small (but important) contribution, so no Contribution License Agreement is required at this point. We will now review your pull request.
Thanks,
Microsoft Pull Request Bot

@mhegazy
Copy link
Contributor

mhegazy commented Apr 26, 2017

From the internationalization spec: https://tc39.github.io/ecma402/

The ECMAScript 2017 Internationalization API Specification (ECMA-402 4th Edition), provides key languagesensitive functionality as a complement to the ECMAScript 2017 Language Specification (ECMA-262 8th Edition or successor). Its functionality has been selected from that of well-established internationalization APIs such as those of the Internationalization Components for Unicode (ICU) library, of the .NET framework, or of the Java platform.

Based on this i would say these should be added to es2017.d.ts, or a new file es2017.intl.d.ts that is included in es2017.d.ts.

@mhegazy
Copy link
Contributor

mhegazy commented Apr 26, 2017

also cc @bterlson for his expert opinion.

Copy link
Contributor

@mhegazy mhegazy left a comment

Choose a reason for hiding this comment

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

Spoke to @bterlson offline, and he confirmed that the intl is coupled with the ecmascript spec. so i would put these in es2017 instead.

@donaldpipowitch
Copy link
Contributor Author

Based on this i would say these should be added to es2017.d.ts, or a new file es2017.intl.d.ts that is included in es2017.d.ts.

So I was a little bit unsure about this. es2017.d.ts contained nothing, but imports so I created a new file es2017.intl.d.ts. However it seems that every other es2017.*.d.ts files is referenced in other places, too.

Is this correct?

@mhegazy
Copy link
Contributor

mhegazy commented May 23, 2017

You also need to add it in JakeFile.js

@donaldpipowitch
Copy link
Contributor Author

Now based on current master. Phew... touched quite a few places.

Run $ npm run build. Seems to work fine.

@mhegazy mhegazy merged commit f309996 into microsoft:master May 24, 2017
@mhegazy
Copy link
Contributor

mhegazy commented May 24, 2017

thanks. for future references you do not need to touch files in lib\* these are generated from the build jake LKG.

@donaldpipowitch
Copy link
Contributor Author

Awesome, thanks.

@donaldpipowitch donaldpipowitch deleted the patch-2 branch May 24, 2017 17:01
saschanaz pushed a commit to saschanaz/TypeScript that referenced this pull request May 25, 2017
@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.

3 participants