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

fix(es2018): add a target and an initial lib #20385

Merged
merged 2 commits into from
Dec 2, 2017

Conversation

benbraou
Copy link
Contributor

@benbraou benbraou commented Dec 1, 2017

With this PR:

  1. jake LKG is run (first commit)
  2. es2018 target and library is added (part of second commit)
  3. jake LKG is run again (part of second commit) -

Fixes #20342

@msftclas
Copy link

msftclas commented Dec 1, 2017

CLA assistant check
All CLA requirements met.

@@ -120,6 +121,7 @@ namespace ts {
"es7": "lib.es2016.d.ts",
"es2016": "lib.es2016.d.ts",
"es2017": "lib.es2017.d.ts",
"es2018": "lib.es2018.d.ts",
Copy link
Contributor

Choose a reason for hiding this comment

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

you need to build the new library as well. this is defined in our Jake/gulp files (sorry we have two build systems for now untill we are done with the migration).
JakeFile: https://github.com/Microsoft/TypeScript/blob/8f1cdc9b0c649f1e4fe9d96718be93277baa2ecd/Jakefile.js#L229
GulpFle: https://github.com/Microsoft/TypeScript/blob/8f1cdc9b0c649f1e4fe9d96718be93277baa2ecd/Gulpfile.ts#L168
See 6a737c8 that added esNext target.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@mhegazy thank you for the review. I am updating my PR accordingly.

FYI, I see that jake LKG generates diagnosticMessages.generated.json file per locale. I think this is related to PR #19308

Therefore, in my PR you will have 2 commits:

  1. 'run jake LKG before es2018 addition'
  2. 'fix(es2018): add a target and an initial lib' (contains the es2018 lib + jake LKG run a second time)
    Please let me know if I need to squash both commits

@mhegazy
Copy link
Contributor

mhegazy commented Dec 2, 2017

Thanks! For future references please do not include LKG updates in PRs, they happen as part of publishing or manually directly on master.

@mhegazy mhegazy merged commit 49a48ff into microsoft:master Dec 2, 2017
@mhegazy
Copy link
Contributor

mhegazy commented Dec 2, 2017

@benbraou do you want to pick up adding definitions for the new array methods: https://github.com/tc39/proposal-flatMap (a new es2018.array.d.ts file in src\lib) and Promise.prototype.finally https://github.com/tc39/proposal-promise-finally ( es2018.promise.d.ts)

@benbraou
Copy link
Contributor Author

benbraou commented Dec 2, 2017

@mhegazy Thanks you! I got the point to not include LKG updates in future PRs.
Yes, I would love to add definitions for flatMap and promise finally.
Do you want me to create 2 separate issues ? I will be having a look at the implementation tomorrow.

@mhegazy
Copy link
Contributor

mhegazy commented Dec 2, 2017

Do you want me to create 2 separate issues ?

that would be appreciated. thanks!

values(): IterableIterator<string | File>;

[Symbol.iterator](): IterableIterator<string | File>;
}
Copy link

@Jessidhia Jessidhia Dec 5, 2017

Choose a reason for hiding this comment

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

(out of scope for this PR but) this pattern repeats often enough that maybe there should be a virtual interface in lib.es2015.iterable.d.ts for this 🤔

interface IterableCollection<K, V = K, I = V> {
  entries(): IterableIterator<[K, V]>;
  keys(): IterableIterator<K>;
  values(): IterableIterator<V>;
  [Symbol.iterator](): IterableIterator<I>
}

Only useful in lib.*.d.ts if interface augmentations are allowed to add extends, though. Alternatively, end the separation between iterable and non-iterable declarations; but that might be undesirable for the folks still supporting IE without polyfills (please stop 🙏😉).

* @param mapfn A mapping function to call on every element of the array.
* @param thisArg Value of 'this' used to invoke the mapfn.
*/
from<T, U = T>(arrayLike: ArrayLike<T>, mapfn?: (v: T, k: number) => U, thisArg?: any): U[];
from<T, U>(arrayLike: ArrayLike<T>, mapfn: (v: T, k: number) => U, thisArg?: any): U[];

Choose a reason for hiding this comment

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

from<T, U, This>(arrayLike: ArrayLike<T>, mapfn: (this: This, v: T, k: number) => U, thisArg?: This): U[]?

mjbvz added a commit to mjbvz/schemastore that referenced this pull request Feb 9, 2018
madskristensen pushed a commit to SchemaStore/schemastore that referenced this pull request Feb 9, 2018
@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