TypeScript type definitions

[jonathanhefner]

Sugar.js is wonderful and makes writing JavaScript much more enjoyable. As I've been looking into TypeScript and some its tooling, I've been thinking Sugar.js would be an excellent complement. IntelliSense-like code completion with Sugar.js methods could be a huge productivity boost. Have there been any discussions about creating a TypeScript type definitions file for Sugar.js? (For reference, DefinitelyTyped is a repository of TypeScript type definitions for other popular JavaScript libraries.)

[andrewplummer]

This should be easy enough by tweaking the doc build tools in place. Will have a look after the new versions are out.

[ghost]

@andrewplummer Any news on this?

[hadnet]

@andrewplummer version 2.0.1 is out, any news on sugar.d.ts for this new version?

[andrewplummer]

Yes, now that this is released I'm going to have a look into this. It raises a question however: now that the ability to extend natives is opt-in, it seems that this means that 2 definition files are now required... 1 for non-extended use and one that will included methods that may potentially be extended.

I'm thinking maybe:

sugar.d.ts
sugar-extended.d.ts

Does this make sense?

[andrewplummer]

Alternately it could just be a single definition file with everything together? Would having extended-native definitions in the d.ts file offend people who are adamantly against extending natives? Or does this still make sense to do since this is still behavior that exists regardless of whether or not they choose to use it?

[andrewplummer]

Ok, I'm having a look at this right now but there's a few roadblocks I'm hitting, and it would be nice to have some TS experts here to help out.

First, is there a way for a type to explicitly refer to a global object? For example:

type Native = Object | Array | String | ...;

Here I would like Native to not be "an object" or "an array", but refer to the global objects themselves. Is this even possible?

[andrewplummer]

Also, could someone check my work so far? This seems to get the job done for the main core features in 2.0.0:

declare module sugarjs {

  interface Sugar {
    (options?: SugarOptions): Sugar;
    extend(options?: SugarOptions): Sugar;
    Array: SugarNamespace;
    // Other namespaces listed here...
  }

  interface SugarOptions {
    objectPrototype?: boolean;
    methods?: Array<string>;
    except?: Array<string>;
    // TODO: Figure out global literals
    namespaces?: Array<string>;
    enhance?: boolean;
    enhanceString?: boolean;
    enhanceArray?: boolean;
  }

  interface SugarNamespace {
    <T>(init?: T): SugarChainable<T>;
    new<T> (init?: T): SugarChainable<T>;
    extend(options?: SugarOptions): Sugar;

    defineInstance(methods: Object): SugarNamespace;
    defineInstance(name, Function): SugarNamespace;
    // Other helper and method definition functions listed here...

    flatten<T>(array: T[]): T[];
    // Other Sugar static methods listed here...

  }

  interface SugarChainable<T> {
    raw: T;
    valueOf(): T;

    flatten(limit?: number): SugarChainable<T>;
    // Other sugar chainable methods listed here...

    join(separator: string): SugarChainable<T>;
    // Other native methods mapped to chainables listed here...

  }

}

interface Array<T> {

  flatten(limit?: number): T[];
  // Other extended mode methods listed here...
}

declare var Sugar: sugarjs.Sugar;

With this as a base it should be a matter of:

• listing up the method signatures in all 3 places (static, chainable, extended) which are slightly different
• Pulling out interfaces for option objects (for example Date#create
• Pulling out interfaces for callback functions (for example Array#unique
• Anything else I'm missing?

Edit: Some typos and stuff I forgot.

[trikadin]

Here I would like Native to not be "an object" or "an array", but refer to the global objects themselves. Is this even possible?

Yes, it's possible, you should use ObjectConstructor, ArrayConstructor, StringConstructor, etc., instead of Object, Array, String, respectively.

[trikadin]

And you have to declare SugarConstructor for sugar static options and methods.

[andrewplummer]

@trikadin Cool, that seems to have worked!

For the second part, I'm declaring interface Sugar and it seems to have worked. Is that what you meant or something else?

[trikadin]

In TS you have to split on class declaration into two parts: one for the static methods and properties and one for the the instance methods and properties. For example:

interface ObjectConstructor { // static methods
  assign<T, U>(target: T, source: U): T & U; 
}

interface Object { // instance methods
  hasOwnProperty(v: string): boolean;
}

Thereby, you should rename your Sugar interface into SugarConstructor.

[trikadin]

http://www.typescriptlang.org/docs/handbook/interfaces.html#difference-between-the-static-and-instance-sides-of-classes

[andrewplummer]

Ok, I see... that's interesting. Actually, though, Sugar is not a class with any instance methods on it, just a namespace (well, actually a function as well, but not a class).

The only instance methods I'm using (aside from on natives when in extended mode) is the Sugar chainables, but it seems that I'm already splitting them as you suggested between SugarNamespace and SugarChainable. Maybe though I should rename SugarNamespace to SugarChainableConstructor instead to make it more clear.

[andrewplummer]

I've hit another issue, this time potentially blocking. Sugar allows user defined functions (this is a major feature of the v2.0 update) that will appear on the global object like such:

Sugar.Array.defineInstance('foo', function() {});
Sugar.Array.foo(); // This function will now be available here

Strangely, though, it appears that Typescript index signatures can't handle this. I've pruned it down to the following declarations file:

declare module sugarjs {
  interface SugarArrayConstructor {
    [propName: string]: any;
  }
  interface Sugar {
    Array: SugarArrayConstructor;
  }
}
declare var Sugar: sugarjs.Sugar;

Doing this will throw the error: Property 'foo' does not exist on type 'SugarArrayConstructor'.
Is there some way to workaround this?

[vendethiel]

Not really. That's dynamic by nature, so not typeable.

[andrewplummer]

I'm not quite sure what you mean. Index signatures are supposed to allow dynamic access to objects such as not to throw a compile error. Looking into it further, it seems that this works with brackets but not dot syntax for a property. However this isn't really a workable solution for Sugar.

[vendethiel]

I don't know of a workaround. Property names are dynamic; so TS constrains them to []

[andrewplummer]

Ah I see what you mean now.

[trikadin]

You want to create dts as a standalone module or add it to main repo?

[trikadin]

How can I join to development to help you with this feature?

[andrewplummer]

Good timing! I've essentially just finished with the new gulp tsd task to generate a typescript declarations file! This has taken quite a long time, but it's pretty nice, as the task allows options as well. You can include/exclude modules or methods and also turn off the extended mode declarations if you want. The default declarations file will have them included.

So the question is now what? Is it best practices to include the declarations file directly in the Sugar repo? Also, can you link me to the best guide for contributing them? Is typings the newest/best place for this?

[andrewplummer]

One thing to note is that I will need to look into the issue with being unable to define new methods. Until that is resolved, these definitions aren't exactly complete. If typescript doesn't support that (accessing arbitrary properties through the dot syntax) then new method definition cannot be supported.

Also, due to typescript quirkiness with conflicting interfaces, extending Object.prototype cannot be supported. This is not recommended anyway, however, so I'm willing to consider it unsupported.

[trikadin]

IMHO, the best way is to include module (not extended) declaration file directly into Sugar repo, and add declaration for extended mode to typings.

[andrewplummer]

Ah I see, so split into 2 declarations? Something like:

sugar.d.ts
sugar-extended.d.ts

Maybe? But why only check one into the repo? Or better question, is there a good reason to check any into the repo when they can be built easily?

[trikadin]

Ah I see, so split into 2 declarations?

Yep)

Or better question, is there a good reason to check any into the repo when they can be built easily?

Well, I think you right -- there's nothing bad about to add them both, and add the 'typings' field to the package.json with value './sugar.d.ts'. So, when you use Sugar in your libs (without extending) -- TS automatically finds the declaration, and you don't need to do anything more. And when you write the standalone application and you want to use extended mode -- you just add the 'sugar-extended.d.ts' into your references, just like:

/// <reference path="./node_modules/sugar/sugar-extended.d.ts" />

Kinda long, but not the problem)

[andrewplummer]

OK that sounds pretty good. Is it customary to put them in the root directory or a subfolder? What about the file naming?

[trikadin]

Is it customary to put them in the root directory or a subfolder? What about the file naming?

No limitations, but usually .d.ts. files placed in the root directory and named as <package>.d.ts.

So, imho, sugar/sugar.d.ts and sugar/sugar-extended.d.ts. is the best choice.

[andrewplummer]

I've actually been using generics for this. Would this work:

isArray<T>(instance: any): instance is Array<T>;

Other generics can be derived from the ES6 typings...

[trikadin]

I've actually been using generics for this

Ts reports errors for your current definitions:

node_modules/sugar/sugar-extended.d.ts(364,46): error TS2677: A type predicate's type must be assignable to its parameter's type.

Would this work

Yes, it would, but you shouldn't use generics for this, because argument could be non-typed array, like [0, true, '2'], and, in fact, your method doesn't answers on question "is this a typed array?", only "is this an array"?

Typescript built-in es6.d.ts has the following definition for Array.isArray:

isArray(arg: any): arg is Array<any>;

[trikadin]

One more error:
https://github.com/andrewplummer/Sugar/blob/master/sugar-extended.d.ts#L48
Your Array.prototype.map definition:

map<U>(map: string|sugarjs.Array.mapFn, context?: any): U[];

sugarjs.Array.mapFn is generic - please, could you provide types to it? Like this:

map<U>(map: string|sugarjs.Array.mapFn<T, U>, context?: any): U[];

[andrewplummer]

Typescript built-in es6.d.ts has the following definition for Array.isArray

Ok fair enough... the rest makes sense too... Will check this out

[trikadin]

I'll try to carry out a full inspection of all the TS definitions in the nearest future and send PR.

[andrewplummer]

Thank you! If possible can you keep track of it somewhere so I can repro the errors?

[trikadin]

If possible can you keep track of it somewhere so I can repro the errors?

Sorry, I don't understand... What do you want?

[andrewplummer]

Ah sorry, I just meant if you can give me some code to help test the definitions I would appreciate it :)

[andrewplummer]

you shouldn't use generics for this, because argument could be non-typed array...

Ok I re-read this and I think I finally understand what you mean. Unfortunately I think this means that I'm overusing generics in a lot of places then. For example:

var squares = Array.construct(5, function(n) {
  return n * n;
});

This is a common use case for this method, but in theory construct could return anything, including a non-typed array. Right now in the definitions I have this:

construct<T>(n: number, map: (i: number) => any): T[];

However should probably be this:

construct(n: number, map: (i: number) => any): any[];

Would you agree?

[andrewplummer]

But actually I still find this confusing... If this is correct, then why do the native lib.es6.d.ts typings define interface Array<T> with a generic? A standard array may or may not be typed. I have been interpreting generics to mean "a subset of types", not just "a single type". In fact the docs that I have read have themselves said that any effectively turns off type checking and if possible use a generic instead, as there are certain checks it can still perform. I don't have a perfect understanding of this, so I don't know what checks generics provide over any, but I was simply trusting in the docs that it would be better...

[vendethiel]

I'd prefer construct<T>(n: number, (i: number) => T): T[];

If T needs to be any, that's no issue

[trikadin]

I agree with @vendethiel about construct.

A standard array may or may not be typed.

For non-typed arrays you can use smth like:

const bla: Array<any> = [1, '0', true, {}];

Or, if you have restricted subset of types, you may use unions:

const numAndStringArr: Array<number | string> = [1, '', 'bla'];

[trikadin]

But, in case of isArray you don't need the information about the type of arrays items. Same for isMap, isSet, etc.

[andrewplummer]

For non-typed arrays you can use smth like...Or, if you have restricted subset of types...

Right, but wouldn't the generic help to facilitate that? i.e. Array.construct<number | string>(...)

[vendethiel]

if the function has the return type number | string, it'll correctly get inferred when you pass it to Array.construct.

[andrewplummer]

Ahh ok this is making more sense now. Also ignore my above comment, I was misunderstanding something. However it leads me to another question. For the method fromQueryString, there is an optional callback that receives an object key and property. Right now the typings are like this:

fromQueryString<T, U>(str: string, options?: QueryStringParseOptions): Object;

interface QueryStringParseOptions {
  ...
  transform?: <T, U>(key: string, val: T, obj: Object) => U;
}

This should probably be this:

fromQueryString<T, U>(str: string, options?: QueryStringParseOptions<T, U>): Object;

interface QueryStringParseOptions<T, U> {
  ...
  transform?: <T, U>(key: string, val: T, obj: Object) => U;
}

...right? Does that look right to you overall?

[trikadin]

Yes, that's right. Maybe, it have a sense to read a doc about generics)
https://www.typescriptlang.org/docs/handbook/generics.html

[andrewplummer]

Oh believe me, I've been over that document many times now....

[andrewplummer]

Ok, I fixed the issues raised in the comments:

1. isArray, isString, etc. accept any instead of Object.
2. Callback functions like mapFn are now passed generics. The previous definitions were simply incorrect as type mapFn = <T>(...) but should have been type mapFn<T> = (...).
3. Options interfaces similarly accept and are now passed generics. One note, though is that my above comment was incorrect. It said transform?: <T, U>(...) but should have been transform?<T, U>: (...). I corrected this as well.
4. construct is now correctly using the generic for the map callback. This was simply an oversight.

[andrewplummer]

Please have a look. I will also have a more thorough look at how generics are being used in each method later. However I think these should probably be ok now, as I understand that any was mostly only applying to the type checking methods. However there may be a few more like construct which should be more strictly enforcing their generics. Thanks!

[andrewplummer]

Another issue raised with definitions... it seems that this won't work:

import SugarDate from 'sugar/date'

Having a look at how lodash does it, there seems to be a main index.d.ts file, and also one for each method as well. Sugar is similarly modular, so it makes sense that it should follow this pattern.

However this raises another question... lodash types are in the @types/lodash module. It seems that Sugar should follow suit with this as well, especially if a declarations file is required for not only each module but also each method as well.

Looking into this raised another point. Although the user claimed it work, it seems that ES6 import syntax import Sugar from 'sugar' isn't working as I am using this export = syntax now. This should probably be changed to export default Sugar, no? It seems though that these 2 types are incompatible. That means that if this is changed, the import = require('sugar') will no longer work. My only question is are there any side effects to making this kind of change? If not, then it seems it should be moved as the ES6 style seems to be the standard now.

[andrewplummer]

Update: It seems that export default Sugar is incompatible with node modules, so it will need to be required with import * as Sugar from 'sugar' or import Sugar = require('sugar'). In addition, I'm having trouble exporting the declarations for each method. The declarations look something like this:

declare namespace sugarjs {

  interface Sugar {
    Date: Date.Constructor;
  }

  namespace Date {

    interface Constructor {
      create(str?: string): Date;
    }
    
  }
}

declare module "sugar/date/create" {
  var create: sugarjs.Date.create; // this is the part that won't work
  export = create;
}

I've tried various different approaches with the module, but it doesn't seem to be able to access create in a way that works with ambient contexts (declares). The docs haven't been much help so far. Turning to stackoverflow....

[DaanDeMeyer]

Typescript does not pick up the sugar-extended types by default. Perhaps these could be put in a separate @jonathanhefner types package so Typescript can pick them up by default?

EDIT: I've found the issue. The Object.keys() method in the sugar-extended file uses generics when it shouldn't.

The current definition is this:

keys<T>(instance: Object): T[];

And it should be this:

keys(instance: Object): string[];

Actually this is a separate issue. The suggestion to make a separate @types package for the sugar-extended types still stands

[granteagon]

I'm getting an error from webpack using the sugar-date npm module that may be relevant to this conversation:

ERROR in [at-loader] ./wwwroot/app/index.ts:25:24 
    TS2306: File '/node_modules/sugar-date/sugar.d.ts' is not a module.

[diegodlh]

I'm getting an error from webpack using the sugar-date npm module

I'm getting the same error from the TypeScript compiler. Any news about this? It does seem to work with the sugar npm module. Thanks!

[vincentbernat]

The file is missing an export directive.

I wonder what the meaning of the end of sugar.d.ts is:

declare module "sugar" {
  const Sugar: sugarjs.Sugar;
  export = Sugar;
}

declare var Sugar: sugarjs.Sugar;

Maybe it's automatically translated somehow? Instead, using:

export const Date: sugarjs.Date.Constructor;

Seems to help. But then, I don't understand what the deal with constructors are. I replace interface Sugar with:

  interface Sugar {
    (opts?: ExtendOptions): Sugar;
    createNamespace(name: string): SugarNamespace;
    extend(opts?: ExtendOptions): Sugar;
    Array: ArrayConstructor;
    Date: Date.Constructor;
    Function: FunctionConstructor;
    Number: NumberConstructor;
    Object: ObjectConstructor;
    RegExp: RegExpConstructor;
    String: StringConstructor;
  }

I am quite new with Typescript, so maybe this is stupid.

[vincentbernat]

Okay, after digging a bit more in Typescript documentation, I think I was mislead. There is a simpler modification that would work:

declare module "sugar-date" {
  const Sugar: sugarjs.Sugar;
  export = Sugar;
}