Adding typescript definition file

[singhjusraj]

Brief description of the changes [REQUIRED]

Adding TypeScript definitions allows integration with TypeScript based projects

What browsers and operating systems have you tested these changes on? [REQUIRED]

N/A

Are all automated tests passing? [REQUIRED]

I haven't written tests for this yet

Is this pull request against develop or some other non-master branch? [REQUIRED]

yes

[rnicholus]

I'll make this as "in progress" for now. Please do let me know when you have a question or need a code review.

[rnicholus]

Related #1688.

[CLAassistant]

All committers have signed the CLA.

[singhjusraj]

If you check the fine-uploader.spec.ts fle, If I use the 'new' keyword to instantiate FineUploader, TypeScript is complaining.
If I don't use the 'new' keyword, things seem to be okay. I'll do more testing once I'm done writing everything

[singhjusraj]

http://docs.fineuploader.com/branch/master/api/options-ui.html#thumbnails.placeholders
the notAvailablePath option is listed twice

[rnicholus]

Thanks for finding that issue in the docs. I'll push a fix for that next.

Regarding new - you definitely have to new up a qq.FineUploader instance. Not doing so will result in a function return type, and, as a result, you won't be able to access any API methods. Also, the internal code expects qq.FineUploader to be properly constructed, otherwise the value of this will be window instead of the qq.FineUploader instance.

Not sure if Typescript provides some magic that makes new unnecessary. I'm not very familiar with the language myself.

[singhjusraj]

http://docs.fineuploader.com/branch/master/api/options-azure.html#signature.endpoint
In description here where it says "The endpoint that Fine Uploader can use to send GET for a SAS before sending requests off to S3"
I think instead of S3 it should be Azure.

[rnicholus]

Thanks for catching that. Fixed!

[rnicholus]

Note that your branch should be based off of Fine Uploader develop

[singhjusraj]

I see that you have changed the base branch from master to develop.
I should have forked from develop.
Is there anything else I need to do to base off of develop branch?
In this thread I'm seeing this message
Add more commits by pushing to the master branch on SinghSukhdeep/fine-uploader.
Which branch should I push my changes now?

[rnicholus]

it probably doesn't matter much since, at the time you forked, master and develop were identical.

[singhjusraj]

http://docs.fineuploader.com/branch/master/api/qq.html#qq.each
the description for parameter iterable is incorrect.

[rnicholus]

I'd suggest leaving any of the qq util functions out. I'd like to remove those entirely in a future major release.

[singhjusraj]

Okay, well so far I've written about 80% of util functions already. I'll take them out then.
If there is anything else you have in mind which is to be removed in future releases, please let me know before I write it all.

[rnicholus]

Sorry. If you're almost done with util functions, you are free to proceed. I can just remove them once 6.0 is released, which may not be for a while anyway.

I'll fix the docs for qq.each too.

[rnicholus]

Just started to look at this again. Sorry for the delay, but I'm only able to pick up a few minutes here and there for the foreseeable future.

What exactly is typescript/fine-uploader.spec.ts?

[rnicholus]

Just started to look at the definition file. I can safely say that I'll never have enough time to review all of that, nor will I be able to verify since I don't use TS in any projects. So I'm included to just merge this through since no existing code was updated and this this is localized to a particular integration point that did not previously exist. But I'm wondering - how do projects pick up this definition file? How does TS locate this? Where does it have to reside inside of the package retrieved from npm?

This will resolve #1652, #1688, and #1689 AFAICT.

[singhjusraj]

The typescript/fine-uploader.spec.ts is just a test file showing the use of proper syntax in TS.
About how will TS pickup the def file please read this
Either we put the def file in a directory and point "types" property in package.json to that path or we just put the file at the root and name it index.d.ts or we publish it separately from the project under Github's DefinitelyTyped and then users can install the def file as npm install @types/fine-uploader.

I'm currently using it as if installed using third option.
The def file will be then located at node_modules/@types/fine-uploader/index.d.ts.
They do recommend the first approach though.

[rnicholus]

Fine with the first approach. To complete this:

• update to package.json as specified in your last message
• update to Makefile to ensure the proper TS files are included in the zip files & the npm package. It's probably best to include these in the /client/commonJs directory in the source tree. The contents of that dir will be included in the lib/ dir in the npm package.

I think it would be nice if a feature page was created or something somewhere in the docs was updated to reflect TS support.

Related: Are you interested in becoming a contributor for this project? Seems you now have a better understanding of the API than most at this point.

[singhjusraj]

As of TS v2.1, the first approach seems to be working properly (Wasn't working before, that's why I was using @types approach).
I'll review the def file once more and will let you know when I think everything is okay.

And yes I have no issues with becoming a contributor. What do I have to do in order to be so?

[singhjusraj]

My bad, I can get it working only by using @types approach.
TS compiler is complaining with other two ways, It has something to do with the way fine-uploader file structure is.
Works only if I put the def file at node_modules/@types/fine-uploader/index.d.ts.
I have to do some more digging on this

[rnicholus]

@SinghSukhdeep Was wondering if you have any updates on your progress, or if you need anything from me?

[singhjusraj]

@rnicholus sorry didn't get any time during the holidays and had some tight deadlines before that, will have some time this week to look at it.

[rnicholus]

No rush at all. Just checking in.

[rnicholus]

@SinghSukhdeep Just wanted to bring to your attention DefinitelyTyped/DefinitelyTyped#13768. It just came onto my radar a few days ago. It's not clear to me how this affects your efforts here. Was wondering if you had some comments. In particular, regarding this area of the DefinitelyTyped readme - https://github.com/DefinitelyTyped/DefinitelyTyped#removing-a-package.

[singhjusraj]

I looked at it, they are also preparing a similar definitions file as we are.
Maybe somehow we can merge their progress with ours and get someone to review our code.

About removing a package, once we publish fine-uploader with the definitions included, then they can deprecate that package, we don't have to do it.

In this pull I need to set the types property to point to our bundled declaration file in order to locate the definition file, which I haven't done yet as I'm unable to get it to work that way.
I've looked at some of the TS docs but haven't found a resolution to the problem yet. I'll keep looking

[ghost]

Hi @SinghSukhdeep, I'd be willing to help, but could you explain to me the way this library is used? On the DefinitelyTyped tests, everything uses a qq global variable. But the NPM package appears to have module.exports. Can this library be used as a module?
If it is not used as a module, you will need /// <reference types="fine-uploader" /> to include the file in the compilation. (Use the --listFiles compile option to see which files get included.) Modules in node_modules/@types get included automatically, but not all of those in node_modules.

[rnicholus]

The library can be used as a module. Examples/details at http://docs.fineuploader.com/branch/master/features/modules.html.

[rnicholus]

After looking at this closer, could it be that the definition file is not included with the packaged library, and that is the cause of this issue? The entire source tree is not published to npm. There is a build step that only includes the bundled code and a subset of the project root. This is something I mentioned in a comment a while back.

[singhjusraj]

@andy-ms okay got it, I also came to this resolution because I'm not using the library as a module and I was missing that triple slash directive.
Including the dependency like that does the job.
Thanks for your help.

Any chance someone can review our code before we publish the package?
I would really appreciate that.

[bradfordwagner]

@SinghSukhdeep I think you did some really nice thorough work here.

@rnicholus I think we need to set up CI for this TypeScript component. Treating the TypeScript definitions as first class is much better than having it maintained in the DefinitelyTyped repository, and I'd be happy to contribute. Would it be possible to have a feature branch that both @SinghSukhdeep and I could work out of?

[rnicholus]

I'd be happy to add you both as contributors to fine-uploader. Then, this PR could be merged into a local feature branch where work could continue. How does that sound?

[bradfordwagner]

Sounds great to me

[rnicholus]

I've sent both of you invitations. In order to bring this into the repo without merging it into develop (since it's not complete yet), I think the next steps should be:

1. Create a new branch in this repo based off of develop.
2. Change the target branch of this PR to the branch created in step 1 & merge it in.
3. Open up a new PR and begin to collaborate and make changes on the branch created in step 1. Please reference this PR in the new one.

[singhjusraj]

Everything done. New PR #1719