Skip to content
This repository has been archived by the owner on Nov 20, 2018. It is now read-only.

Feature/typescript definitions #1719

Merged
merged 35 commits into from
Feb 13, 2017
Merged

Conversation

singhjusraj
Copy link
Member

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]

Need to write automated tests during code reviews or afterwards

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

Yes

@singhjusraj
Copy link
Member Author

PR #1689 Continued here

@singhjusraj
Copy link
Member Author

Ok, I'll take a look at qq.Promise and make changes to functions that are returning a promise. Later after 6.0 release, we'll update the def to return native Promise

@rnicholus
Copy link
Member

rnicholus commented Jan 31, 2017

Excellent! One more thing - will these defs be picked up when Fine Uploader is installed via npm? It doesn't look like they are even part of the npm package, since the Makefile's setup-dist recipe contains the logic for generating the package, and the typescript folder is new. You should be able to test before even publishing to npm by following these steps:

  1. make publish simulate=true
  2. cd _dist/5.14.0-beta1
  3. npm link
  4. Go to whatever directory/project you want to install Fine Uploader to
  5. npm link fine-uploader
  6. Make sure your TS project is able to see the defs.

@singhjusraj
Copy link
Member Author

Little typo in the docs here.
It should say Call this on a promise to indicate failure. instead of Call this on a promise to indicate success.

@singhjusraj
Copy link
Member Author

I'll work on including the Typescript folder in the build process whenever I get some time

@singhjusraj
Copy link
Member Author

@rnicholus I've updated Makefile to include typescript directory and has followed the steps you stated earlier to test changes. New project can see the defs no problem. Please review

@rnicholus rnicholus added this to the 5.14.0 milestone Feb 13, 2017
Copy link
Member

@rnicholus rnicholus left a comment

Choose a reason for hiding this comment

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

LGTM - This was a tremendous amount of work. Thanks a ton for sticking with this. Approved for merge, and this will be part of 5.14.0, initially 5.14.0-beta3.

@rnicholus rnicholus merged commit 1c7bb25 into develop Feb 13, 2017
@rnicholus rnicholus deleted the Feature/TypeScript-Definitions branch February 13, 2017 02:47
@rnicholus
Copy link
Member

Tweeted from the Fine Uploader account, but couldn't find your Twitter account. https://twitter.com/fineuploader/status/830975906852241412

@singhjusraj
Copy link
Member Author

singhjusraj commented Feb 13, 2017

Thanks for your prompt support.
My Twitter handle is Sukh9212.
Thanks for the tweet as well

@don-bluelinegrid
Copy link

There is still an index.d.ts missing, and as a result the d.ts files inside the /typescript directory are not being automatically found by TypeScript.

@don-bluelinegrid
Copy link

Also, there is no export statement in the typing file, so nothing is being picked up as a default export.

There needs to be a default export of qq.

@singhjusraj
Copy link
Member Author

@don-bluelinegrid see PR #1840
Current TS definitions do not support ES6 style import.
You will have to include the fine uploader js file in your html page and follow this file for syntax guidance

@don-bluelinegrid
Copy link

I am not going to do that.

I'm using TypeScript and webpack, and every module that's provided as an npm package can be imported using an import statement. I don't include any JS files in my index.html, and I don't plan to start doing that.

I read various PRs talking about TypeScript compatibility, and I see typescript d.ts files in the package. Why do these exist, if there is no TypeScript compatibility?

@singhjusraj
Copy link
Member Author

As I said earlier, we are providing the typescript definitions. Current way of consumption is as I wrote in my previous comment.
We are going to provide import support soon.
Just be patient.

@singhjusraj
Copy link
Member Author

#1840 has been merged and released as 5.15.0
Happy Importing !!

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants