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]

Yep that sounds good... I'll make the necessary changes. Also which typings repos should I check them into?

[andrewplummer]

whew! 😌

I was worried for a second about splitting out the two files, as the extended forms still reference callbacks and options interfaces that exist in the base declarations file, but it seems that they can reference each other even across different files, so I think this should work!

[andrewplummer]

OK, in addition to the main declarations in the repo, I have also added declarations for each of the modularized npm packages (sugar-array, sugar-date, etc). These will be generated each release and contain only the declarations relevant to the module!

[andrewplummer]

So, according to this guide, the preferred way of publishing the types is to put them in the repos and add the "types" field to package.json, which I have done.

Is this all I need to do? If so, I think we can close this one out...

[trikadin]

OK, in addition to the main declarations in the repo, I have also added declarations for each of the modularized npm packages (sugar-array, sugar-date, etc)

Not bad)

Is this all I need to do? If so, I think we can close this one out...

Please, use a Type Guards for 'Object.is'

You can read more about Type Guards here.

[andrewplummer]

Ok, so from what I gathered:

isDate(instance: Object): boolean;

becomes:

isDate(instance: Object): instance is Date;

? I guess the return boolean value is implied in the type guard?

[trikadin]

becomes: isDate(instance: Object): instance is Date;

Yes. The same is for 'isFunction', 'isString', 'isNumber', etc.

I guess the return boolean value is implied in the type guard?

Yep.

[andrewplummer]

Makes sense! Will make this change!

[andrewplummer]

Ok! This is now updated.

[trikadin]

You forgot to add reference to the sugar.d.ts in extended declaration, something like:

/// <reference path="./sugar.d.ts" />

[andrewplummer]

Oh, I thought the user would include both separately, but I guess this might be better?

[trikadin]

sugarjs declared in sugar.d.ts, and sugar-extended.d.ts using it, so you have to import or reference sugar.d.ts in it.

[andrewplummer]

Ah ok that's a good point since sugar-extended does have explicit references. Will update this!

[trikadin]

Why sugar.d.ts declare module called sugarjs? It should declare module sugar.

[trikadin]

And you have to explicitly declare exports of this module.

[trikadin]

You can look at the good example of module declaration here.

[andrewplummer]

So, I named it sugarjs to give it a more unique name. Are collisions not a thing here? That said, it works fine as the naming seems not to matter once it reaches the final variable declaration. If we can be sure that there will not be collisions then I will rename this. As for exports, I have tested it now and it is working fine, so what does declaring the exports do? Is there a way I can test?

[andrewplummer]

While we're on the topic, module and namespace seem to show no difference in usage. Is there a correct way to use one vs the other?

[trikadin]

So, I named it sugarjs to give it a more unique name.

The name of the declared module should be equal to package.json name and it should have explicit exports. Now it doesn't, and i've got this error:

While we're on the topic, module and namespace seem to show no difference in usage. Is there a correct way to use one vs the other?

namespace is like "internal module". Maybe, this link will help you to understand module declaration: http://www.typescriptlang.org/docs/handbook/modules.html#working-with-other-javascript-libraries

Is there a way I can test?

https://gist.github.com/trikadin/bfa72f3b77c7b3e648cf3c1accbd0106

[andrewplummer]

OK... I've read through the guide, and I see what you mean now. I also get the difference between namespaces and modules. It does appear that the sugarjs name doesn't matter as an internal namespace, but I see now that a module named "sugar" is also required with the proper exports. So, this is what I have set up, and it seems to be working in all cases:

declare namespace sugarjs {

  // Top level interfaces...

  namespace Array {

    // Array interfaces...

  }

}

declare var Sugar: sugarjs.Sugar;

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

This satisfies both the "import Sugar = " node form, as well as explicitly referencing the declarations file for non-commonjs use.

Let me know what you think.

[trikadin]

Please, give me a link at the commit, so I can test it on my projects.

[trikadin]

Oh, sorry, I see it now.

[trikadin]

Works fine, thanks a lot! Will waiting for release on npm.

[trikadin]

One thing: https://github.com/andrewplummer/Sugar/blob/d165e89c15794dab62dc6ea5124e9375fe21f765/sugar.d.ts#L934

It should be instance is string. string referenced for primitive type, and String referenced for instance of constructor String. Example:

const primitive: string = 'foobar';

const instance: String = new String('foobar');

The same is for number and boolean.

[andrewplummer]

Hmm. I see what you mean, and I definitely intended the primitive, but technically this method will accept either. Is that possible?

[andrewplummer]

So, I can't really figure out a way to test this, but I naively tried string | String and it doesn't seem to throw an error... what do you think?

[trikadin]

technically this method will accept either.

Hmm. You use something like this to check types?

Object.prototype.toString.call('bla');

I never used the primitive types wrappers explicitly, and afaik this is considered as a bad practice, and it have some caveats. For example:

if (new Boolean(false)) {
    console.log('lol'); // will be printed
}

Maybe, it have sense to use more stricter checking?

[trikadin]

So, I can't really figure out a way to test this, but I naively tried string | String and it doesn't seem to throw an error... what do you think?

Left the string only)

[andrewplummer]

You use something like this to check types? Object.prototype.toString.call('bla');

Exactly.

I never used the primitive types wrappers explicitly, and afaik this is considered as a bad practice,

I agree, so in that sense, maybe it's not necessary, but I wanted to make it as accurate to the actual code as possible.

Left the string only

Sorry I didn't understand... you mean only use the primitive? I'm ok with this, so if that's what you think then let's do it!

[trikadin]

I wanted to make it as accurate to the actual code as possible.

In this case, you shouldn't: https://www.typescriptlang.org/docs/handbook/declaration-files/do-s-and-don-ts.html

Don’t ever use the types Number, String, Boolean, or Object.

[andrewplummer]

OK fair enough!

[andrewplummer]

Are we good then? I'm going to wait for a final approval from you so that I don't have to keep bumping npm version numbers :)

[trikadin]

Seems ok) Thank you a lot)

[andrewplummer]

It seems that Webpack is using some other way of importing the declarations files than /// ... I'm moving that to this thread to help figure it out...

[qvazzler]

@andrewplummer Right, so I don't think ///'referencing (reference paths) is the "latest and greatest" recommendation, it's more of the old way to import typescript definitions, which even though it is not officially deprecated, is not really preferred the way things are going with typings right now (granted they've made a LOT of changes in the past year or so).

See here for some larger list of reflections:
https://github.com/Microsoft/TypeScript/issues/2242#issuecomment-83694181

And here for a good answer to this question of using reference paths or not: http://stackoverflow.com/a/34121620/2136136

I don't know everything about the definitions and best way to include them, but I do know that there is a new thing called @types in npm, where you install each definition as its own package. https://www.npmjs.com/~types

I think we can keep the definition to being included in the project, but it would probably be more "sane" to move it to the definitelytyped/@types repos so other users can help contribute more easily without your intervention. Of course this may be a double-edged sword.

Also yes, something needs to be done about ES6 module compliance, because right now something's off and I cannot yet figure out why.. Compare it to the moment.d.ts file for example, there are some clear differences.

[andrewplummer]

@qvazzler Apologies, I did know about the referencing here, in fact it was the same issue that @trikadin raised a few comments up. It seems that this is in fact addressed with the latest typings, I was just waiting to release them to give it a bit of time to settle before bumping the version. This seems like an opportune time, so I will release them now!

[andrewplummer]

OK, there were a couple other issues with this, but I think I got them sorted out. For reference, the updated version is 2.0.4.

Note that if you are going to use extended mode, then you will still need to reference "node_modules/sugar/sugar-extended.d.ts" using the /// form. Other than that it should work.

[trikadin]

Awesome, congratulations!

[qvazzler]

@andrewplummer So I can confirm that Sugar is now loading properly with the following:

import * as Sugar from "sugar";

Which is excellent news! I also want to highlight that I am using the latest beta packages of typescript and webpack, so you are looking great.

"webpack": "2.1.0-beta.27",
"typescript": "next" //2.2.0-dev.20161117

Without much insight in this package yet, I do think leaving anything that requires reference tags (///) is probably not going to be a popular thing for typescript users. With that said, really looking forward to use this package. :-) Thanks!

[andrewplummer]

Well, Sugar's original form -- in fact perhaps the reason it exists at all -- is in extending native prototypes. This is now optional, so the typings that support that need to be optional as well. If there is another way to allow the optional loading of typings instead of referencing with /// then I'm all for that, but this doesn't seem to exist as far as I can tell.

[qvazzler]

Maybe it's because the extended functions actually belong in a separate js file? That way we could import either as needed.

I just realized, that I actually want to use the extended functions.. Like minutesAgo for example.

[andrewplummer]

Extended mode is not separated in this way, nor can it be. It's a feature that's part of the sugar-core module that allows any defined method to be extended across to its corresponding native prototype.

[qvazzler]

Ok, but then why are these functions like minutesAgo not available in the Sugar Date object?

[andrewplummer]

It should be! I see it in the declarations file now as well... It should be called like this:

Sugar.Date.minutesAgo(date)

If this isn't working can you provide a repro maybe?

[qvazzler]

Sorry... I did not realize this function was static and not part of the created/initialized Sugar.Date object. This is working just fine.

Sugar.Date.minutesAgo(refreshTime);

I assumed I would be able to do

refreshTime.minutesAgo(/*Maybe some relative date here, otherwise now()*/)

[andrewplummer]

Well it actually should be both, so I'm still curious! Essentially you should be able to do this:

Sugar.Date.minutesAgo(d); // Static
new Sugar.Date(d).minutesAgo(); // Chainable

Sugar.extend(); // Extend methods to prototypes
new Date().minutesAgo();  // Extended

Let me know if any of those don't work!

[qvazzler]

I think I see what you mean now.. The Date object in Sugar is just a regular Date object, not a special "Sugar"-Date object. So enabling the extended mode, will indeed prototype this Date object with Sugar's extraneous methods.

While I, who have not included the extended typescript definitions (because it's outside the declared conventions in my project), will get an error by doing this.

        Sugar.Date.minutesAgo(refreshTime); // Static: No error
        new Sugar.Date(refreshTime).minutesAgo(); // Chainable: No error

        Sugar.extend(); // Extend methods to prototypes: No error
        new Date().minutesAgo();  // Extended: Getting "Property minutesAgo() does not exist on type 'Date' in compiler

[andrewplummer]

The Date object in Sugar is just a regular Date object

Well, it depends. The short version is that Sugar instance methods have 3 modes of use:

1. Static - Instances (vanilla JS objects) are passed as the first argument to static methods in the global namespace and return other vanilla JS objects. (Think Lodash/Underscore, but with namespaces).

2. Chainable - Special objects that are created via the namespaces themselves (Sugar.Date()). Instance methods are called directly on them as if they were on the prototype. Return values are also chainables (wrapped), and the underlying vanilla JS objects are accessed with the raw property. Essentially the same as _.chain() in Lodash/Underscore.

3. Extended - Extends the native prototypes. Methods are called directly on them. This is the only mode that needs to be opted into. Which methods are extended can be controlled.

Hope that makes sense...

While I...will get an error by doing this.

Yes, that's exactly right.

[andrewplummer]

I'm going to close this issue for now to get it off my plate. Thanks for all the help, especially @trikadin. I may actually delve into Typescript more after this :) . For now, I feel that this should work for most cases, but there definitely may be a few quirks, so please feel free to re-open.

[trikadin]

Sorry, but I found some errors(

https://github.com/andrewplummer/Sugar/blob/master/sugar-extended.d.ts#L357

Please, replace it with:

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

Same for this, replace with:

isMap(instance: any): instance is Map<any, any>;

And this:

isSet(instance: any): instance is Set<any>;

And, you shouldn't use 'Object' instead of 'any'. isArray can accepts string, boolean, etc. Please, could you check your dts and replace Object to any whenever needed?

[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