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

ES6 typings #987

Merged
merged 38 commits into from
Dec 2, 2014
Merged

ES6 typings #987

merged 38 commits into from
Dec 2, 2014

Conversation

mhegazy
Copy link
Contributor

@mhegazy mhegazy commented Oct 29, 2014

This is the first stab at ES6 typing's. i think i have all the main ones covered in this review.

There are still a main issue here that we are not handling, which is the manageability of the library. currently we have multiple files, for DOM webworkers, etc.. and the target adds another dimension.

We have talked about this before in the design meeting to add a new attribute that allows us to filter out types based on tags. this is a different issue and will be handled in a future change.

Still missing:

  • Use of known symbols as we do not have support for computed properties yet.

* Allows manipulation and formatting of text strings and determination and location of substrings within strings.
*/
declare var String: {
interface StringConstructor {
Copy link
Member

Choose a reason for hiding this comment

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

Are we set on _Constructor? I have reservations because it's more than just a constructor; you have

fromCharCode(...codes: number[]): string;

which is a method on the static side. A lot of .d.ts libraries tend to use the _Static as a naming convention for these types.

I can honestly view it either way.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I have no strong feelings about the name. Personally I would rather not introduce a new named type and kept it the way it is now; and maybe we should.
But if we were to change it, am ok with type_static.

Copy link
Member

Choose a reason for hiding this comment

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

I would prefer StringConstructor. The ES spec calls it "the String Constructor" when describing the object and does not ever use the word "static" except to reserve it as a keyword.

http://www.ecma-international.org/ecma-262/5.1/#sec-15.5.3

@@ -193,7 +193,7 @@ module ts {

return {
getSourceFile: getSourceFile,
getDefaultLibFilename: () => combinePaths(getDirectoryPath(normalizePath(sys.getExecutingFilePath())), "lib.d.ts"),
getDefaultLibFilename: (options) => combinePaths(getDirectoryPath(normalizePath(sys.getExecutingFilePath())), options.target === ScriptTarget.ES6 ? "lib.es6.d.ts" : "lib.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.

no parens.

* String.raw is intended for use as a tag function of a Tagged Template String. When called as such the first argument will be a well formed
* template call site object and the rest parameter will contain the substitution values
*/
raw(callSite: { raw: string; }, ...substitutions: any[]);
Copy link
Member

Choose a reason for hiding this comment

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

callSite is a misleading name; also, this doesn't have a return type annotation. It should be string[].

@DanielRosenwasser
Copy link
Member

Are we going to include TemplateStringsArray in this series of commits?

Side note: We've been having issues with how to deal with that when we try to typecheck a tagged template in ES5 mode, but when TemplateStringsArray isn't in the lib. @sheetalkamat and I talked about it and I'm going to make the checker use unknown in pre-ES6 targets for now.

@mhegazy
Copy link
Contributor Author

mhegazy commented Nov 13, 2014

I think this a fair addition to the existing lib.d.ts, nothing dependent on the runtime here. So I would say just add the definition and I will be it up when I merge.

@mhegazy
Copy link
Contributor Author

mhegazy commented Nov 17, 2014

@rbuckton and @bterlson this the ES6 typings. your feedback would be appreciated:)
you would want to look at sr\lib\es.d.ts; this should include the ES6 specific additions. thanks!

@bterlson
Copy link
Member

you mean es6.d.ts?

Took a look, looks good to me. Only minor concern is the duplication of methods on each TypedArray when it's very unlikely the different TA's will have different methods on them.

@mhegazy
Copy link
Contributor Author

mhegazy commented Nov 18, 2014

Thanks @bterlson, I originally had one interface TypesArray that others inherited from bu did not want to add the extra name in the global namespace. i can go back there, should be less definitions overall.

@rbuckton
Copy link
Member

  • SymbolConstructor is missing the prototype property (per 19.4.2.7 of ES6 spec)
  • Symbol is missing the constructor property (per 19.4.3.1)
  • Comment on line 43 says "should be flatten", though it seems like it should read "should flatten"
  • Line 238, Array.prototype.keys is an Iterator, not an Array, according to the spec (22.1.3.13).
  • Line 243, Array.prototype.values is an Iterator, not an Array, according to the spec (22.1.3.14).
  • The value property of the IteratorResult interface should be optional, per the spec (25.1.1.3).
  • Since es6.d.ts currently documents symbolic members with comments, the Iterator interface should have a comment that notes that it contains Symbol.Iterator, assuming this is describing "%IteratorPrototype%", per the spec (25.1.2.1.1).
  • ES6 also defines a new GeneratorFunction object, which inherits from Function, per the spec (25.2.1). I would recommend adding GeneratorFunction and GeneratorFunctionConstructor, inheriting from Function and FunctionConstructor respectively. There also would be a GeneratorConstructor in the value space.
  • The Generator interface is may not be easy to apply to an actual generator. The reason is that the value of its IteratorResult can come from either a yield in the body of the generator function, as well as from a return in the body of a generator function. Anyone using Generator will likely have to reference it as Generator<T|R> where T is the type for value when done is false, and R is the type for value when done is true.
  • Generator is missing the return method, per the spec (25.3.1.3)
  • keys and values on Map should be Iterator, not Array, per the spec (23.1.3.8, 23.1.3.11)
  • keys and values on Set should be Iterator, not `Array, per the spec (23.2.3.8, 23.2.3.10)
  • Proxy does not have a ProxyConstructor, which would be consistent with the other _Constructor interfaces in the file.
  • Though not in the typings for Promise that I provided, the ES6 Promise constructor can be called as a regular function, per the spec (25.4.3.1)
  • PromiseConstructor is missing the prototype property
  • Promise is missing the constructor property

@@ -1171,6 +1171,7 @@ module ts {
version?: boolean;
watch?: boolean;
preserveConstEnums?: boolean;
allowNonTsExtensions?: boolean;
Copy link
Member

Choose a reason for hiding this comment

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

What happens on compile on save - So what happens with emit - what file name do we give.. Should it be emitted? Should it be considered ts file and not .d.ts file? What about declaration file generation - I think we should give error if emit is called with this option. (Emit time error)
We need tests with and without this option and .d.ts and .js file generation. Can be compiler baseline I guess.

Conflicts:
	src/compiler/parser.ts
	src/compiler/tsc.ts
	src/harness/harness.ts
	src/harness/projectsRunner.ts
	tests/baselines/reference/templateStringsArrayTypeDefinedInES5Mode.errors.txt
@mhegazy
Copy link
Contributor Author

mhegazy commented Nov 27, 2014

Notes on @rbuckton 's remarks:

  • SymbolConstructor is missing the prototype property (per 19.4.2.7 of ES6 spec)
    • done
  • Symbol is missing the constructor property (per 19.4.3.1)
    • you should get that from object anyways, we do not usually add it as it polutes the completion list
  • Comment on line 43 says "should be flatten", though it seems like it should read "should flatten"
    • done
  • Line 238, Array.prototype.keys is an Iterator, not an Array, according to the spec (22.1.3.13).
    • done
  • Line 243, Array.prototype.values is an Iterator, not an Array, according to the spec (22.1.3.14).
    • done
  • The value property of the IteratorResult interface should be optional, per the spec (25.1.1.3).
    • done
  • Since es6.d.ts currently documents symbolic members with comments, the Iterator interface should have a comment that notes that it contains Symbol.Iterator, assuming this is describing "%IteratorPrototype%", per the spec (25.1.2.1.1).
    • done
  • ES6 also defines a new GeneratorFunction object, which inherits from Function, per the spec (25.2.1). I would recommend adding GeneratorFunction and GeneratorFunctionConstructor, inheriting from Function and FunctionConstructor respectively. There also would be a GeneratorConstructor in the value space.
    • done
  • The Generator interface is may not be easy to apply to an actual generator. The reason is that the value of its IteratorResult can come from either a yield in the body of the generator function, as well as from a return in the body of a generator function. Anyone using Generator will likely have to reference it as Generator<T|R> where T is the type for value when done is false, and R is the type for value when done is true.
    • i think we need to revisit this when we do generators. generatos have to use a special function, so i am not sure when or how we are going to use this generic type.
      may be type it like
      next(): IteratorResult<T>; next<R>(value: R): IteratorResult<T|R>;
  • Generator is missing the return method, per the spec (25.3.1.3)
    • done
  • keys and values on Map should be Iterator, not Array, per the spec (23.1.3.8, 23.1.3.11)
    • done
  • keys and values on Set should be Iterator, not `Array, per the spec (23.2.3.8, 23.2.3.10)
    • done
  • Proxy does not have a ProxyConstructor, which would be consistent with the other _Constructor interfaces in the file.
    • done
  • Though not in the typings for Promise that I provided, the ES6 Promise constructor can be called as a regular function, per the spec (25.4.3.1)
    • I do not think so, from step 2 in section 25.4.3.1, "If Type(promise) is not Object, then throw a TypeError exception."
  • PromiseConstructor is missing the prototype property
    • done
  • Promise is missing the constructor property
    • you should get that from object anyways, we do not usually add it as it polutes the completion list

@rbuckton
Copy link
Member

Regarding Promise Constructor, you are correct. It looks like the behavior of
25.4.3.1 is to support subclassing Promise, though subclassing does not seem to be supported in TypeScript given Promise is defined via interfaces.

mhegazy added a commit that referenced this pull request Dec 2, 2014
@mhegazy mhegazy merged commit 28a3745 into master Dec 2, 2014
@mhegazy mhegazy deleted the es6Typings branch December 2, 2014 01:32
@microsoft microsoft locked and limited conversation to collaborators Jun 18, 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.

10 participants