andrewplummer / Sugar

A Javascript library for working with native objects.
https://sugarjs.com/
MIT License
4.53k stars 306 forks source link

TypeScript type definitions #529

Open jonathanhefner opened 8 years ago

jonathanhefner commented 8 years ago

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 commented 8 years ago

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

ghost commented 8 years ago

@andrewplummer Any news on this?

hadnet commented 8 years ago

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

andrewplummer commented 8 years ago

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 commented 8 years ago

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 commented 7 years ago

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 commented 7 years ago

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:

Edit: Some typos and stuff I forgot.

trikadin commented 7 years ago

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 commented 7 years ago

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

andrewplummer commented 7 years ago

@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 commented 7 years ago

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 commented 7 years ago

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

andrewplummer commented 7 years ago

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 commented 7 years ago

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 commented 7 years ago

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

andrewplummer commented 7 years ago

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 commented 7 years ago

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

andrewplummer commented 7 years ago

Ah I see what you mean now.

trikadin commented 7 years ago

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

trikadin commented 7 years ago

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

andrewplummer commented 7 years ago

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 commented 7 years ago

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 commented 7 years ago

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 commented 7 years ago

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 commented 7 years ago

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 commented 7 years ago

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

trikadin commented 7 years ago

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 commented 7 years ago

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

andrewplummer commented 7 years ago

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 commented 7 years ago

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 commented 7 years ago

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 commented 7 years ago

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 commented 7 years ago

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 commented 7 years ago

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 commented 7 years ago

Makes sense! Will make this change!

andrewplummer commented 7 years ago

Ok! This is now updated.

trikadin commented 7 years ago

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

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

2016-11-11 17 23 19

andrewplummer commented 7 years ago

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

trikadin commented 7 years ago

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 commented 7 years ago

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

trikadin commented 7 years ago

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

trikadin commented 7 years ago

And you have to explicitly declare exports of this module.

trikadin commented 7 years ago

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

andrewplummer commented 7 years ago

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 commented 7 years ago

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 commented 7 years ago

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: image

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 commented 7 years ago

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 commented 7 years ago

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

trikadin commented 7 years ago

Oh, sorry, I see it now.

trikadin commented 7 years ago

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