add typescript definitions #25

chillitom · 2018-11-05T09:39:16Z

should close #21

- needed to be auto-updated - seems to have mostly added carets to deps

- no semicolons, no newline at the top - also change props link to be on one line -- it's important enough that I don't think it should be in parens

- not sure if <object> makes sense, but it needed an argument

src/index.d.ts

agilgur5 · 2019-05-29T02:54:26Z

src/index.d.ts

@@ -16,7 +16,7 @@ export interface IOptions {

 // props specific to the React wrapper
 export interface SignatureCanvasProps extends IOptions {
-  canvasProps?: any,
+  canvasProps?: React.CanvasHTMLAttributes<object>,


should the generic type be <object>?

or maybe leave it as a generic as <T>?: React.CanvasHTMLAttributes<T>? I'm not totally sure why it's a generic or best practices around them in TS

Pretty sure it should be a generic as that's referencing not the object containing the HTML attributes, but the attributes themselves which can be of various types, giving the object various potential shapes

agilgur5 · 2019-05-29T02:56:15Z

src/index.d.ts

@@ -0,0 +1,22 @@
+// signature_pad's props 
+// (taken from https://github.com/szimek/signature_pad/blob/e2af461bca7ceb7cc5afb8349930908ca19f2f56/src/signature_pad.ts#L23)


should we import signature_pad's types directly from signature_pad? I haven't updated to the beta yet (as it's beta) so that might not be possible until then and this might be fine in the meantime

agilgur5 · 2019-05-29T02:56:52Z

src/index.d.ts

@@ -0,0 +1,22 @@
+// signature_pad's props 
+// (taken from https://github.com/szimek/signature_pad/blob/e2af461bca7ceb7cc5afb8349930908ca19f2f56/src/signature_pad.ts#L23)
+export interface IOptions {


should this be IOptions? or SignaturePadOptions? should it be (re-)exported?

package-lock.json

agilgur5 · 2019-05-29T02:59:06Z

Thanks for the (much requested) contribution @chillitom ! Took me a while to finally find the time to give this a proper review as there's a lot to consider here

Unfortunately, I'm not sure this should be merged. Aside from my comments in #21 (comment), which still stand, I also did some research since then and found that the TypeScript team themselves has explicitly said to publish types to DefinitelyTyped (@types) if the source code is not TS in https://www.typescriptlang.org/docs/handbook/declaration-files/publishing.html (c.f. inikulin/parse5#235 (comment) and inikulin/parse5#235 (comment)). This seems to lend a strong addition to my argument that "third-party typing makes more sense at this time".

I attempted to auto-generate the typings with lyft/react-javascript-to-typescript-transform to circumvent this argument, but not only is there quite a bit more work to do from this transform, it actually doesn't generate types, it transforms the entire JS into TS (with different formatting too). Its types are also not as specific as the ones from the signature_pad TS beta (which should always remain the same, if not just imported)

I added some comments & commits to the PR which made me a bit more familiar with TS, but I'm still not sure this should be merged (and my uncertainty in the comments certainly doesn't help 😅). Open to thoughts and counter-arguments

src/index.d.ts

benny-medflyt · 2019-08-19T16:30:01Z

I believe that this PR is missing the style prop, which is valid and will be passed along to the underlying signature implementation

agilgur5 · 2019-08-19T18:46:40Z

@benny-medflyt style is not an option of signature_pad and nor is it a direct prop of react-signature-canvas. You can only pass style through the prop canvasProps, which is already typed here.

agilgur5 · 2019-12-06T19:49:41Z

@remiroyc yes, as you can see, this PR is incomplete and has not been merged (and #21 is still open), therefore there are no types for this pure JS library. And as you can see above, I pretty much recommended the author of this PR submit these types to DefinitelyTyped (though there are still improvements to be made, some of which I made myself here).

As I wrote in #21 (comment), at the time this PR was written, I had never used TypeScript, so I could not reasonably maintain the typings myself, and no one stepped up to maintain them here or at DefinitelyTyped (including the author of this PR, who has not responded to comments).

I've now written a bit of TypeScript and actually do have a ts branch locally, and expect to have a TS re-write out in v1.1.0 before the new year. Now that there's 100% test coverage, that should also give us some confidence that the types are correct before publishing (might add some more tests too).

Was actually going to comment about this in #21 sometime soon, but guess I'll summarize some here. The big blockers are rewriting the 3.5 year old build system used here (it is still on webpack 1) and rewriting the example once the build system is changed (and possibly having JS and TS versions of it).

I was planning on using tsdx to make swapping the build system easier, but jaredpalmer/tsdx#165 is a big blocker for this library that uses default exports. Fortunately, I wrote a PR to solve that recently at jaredpalmer/tsdx#327, but I have no idea when that will be merged or what the hold on it is. I recently created a fork with my changes included and so am planning on using that similar to what I did in agilgur5/mst-persist#24.

For the example, I'm planning on just rewriting it with CRA, similar to the cra-example branch. Just need to figure out what to do about JS vs TS examples.

agilgur5 · 2019-12-23T05:21:35Z

Please read the discussion, review, comments, related issues, etc before making comments. Locking this due to multiple comments that have not.

agilgur5 · 2020-01-18T08:32:36Z

Closing this as @types/react-signature-canvas has been released thanks to @ksocha in #21 (comment) .

Per the above comments, this is the solution recommended by the TS team for third-party typings that are not generated from TS source code.

A TS rewrite is still in-progress and that will eventually replace the unofficial third-party typings with official, internal typings, planned for the 1.1.x release

agilgur5 · 2022-04-20T21:37:59Z

Noting here that the internal re-write, #42, has now been merged. Planning to release as 1.1.0-alpha.1 soon

agilgur5 and others added 6 commits May 28, 2019 21:22

(deps): update package-lock.json

ea9a873

- needed to be auto-updated - seems to have mostly added carets to deps

add @types/react by itself - fixup w/ 1st commit to PR

ae9fa04

add typescript definitions

f2536de

fix imports and make extra props optional

060fe9b

(format/types): use standard style for types

a2fae62

- no semicolons, no newline at the top - also change props link to be on one line -- it's important enough that I don't think it should be in parens

(types): canvasProps should be more specific than any

61e4c2f

- not sure if <object> makes sense, but it needed an argument

agilgur5 force-pushed the master branch from 29b0ae8 to 61e4c2f Compare May 29, 2019 02:47

agilgur5 requested changes May 29, 2019

View reviewed changes

agilgur5 reviewed May 29, 2019

View reviewed changes

src/index.d.ts Show resolved Hide resolved

This comment has been minimized.

Sign in to view

Repository owner locked and limited conversation to collaborators Dec 23, 2019

agilgur5 mentioned this pull request Dec 23, 2019

h.default is not a constructor error after adding unofficial TS definitions #38

Closed

ksocha mentioned this pull request Jan 4, 2020

Add TypeScript declaration file #21

Closed

agilgur5 mentioned this pull request Jan 4, 2020

[react-signature-canvas] Add definitions DefinitelyTyped/DefinitelyTyped#41396

Merged

12 tasks

agilgur5 closed this Jan 18, 2020

agilgur5 added the scope: types Related to type definitions label Jan 18, 2020

Repository owner unlocked this conversation Feb 7, 2020

agilgur5 mentioned this pull request Feb 7, 2020

TypeScript Rewrite #42

Merged

agilgur5 added the problem: stale Issue author has not responded label Mar 13, 2021

Repository owner locked as resolved and limited conversation to collaborators Mar 13, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

add typescript definitions #25

add typescript definitions #25

chillitom commented Nov 5, 2018

agilgur5 May 29, 2019

agilgur5 Jul 14, 2019

agilgur5 Aug 19, 2019

agilgur5 May 29, 2019

agilgur5 May 29, 2019

agilgur5 commented May 29, 2019 •

edited

Loading

benny-medflyt commented Aug 19, 2019

agilgur5 commented Aug 19, 2019 •

edited

Loading

This comment has been minimized.

agilgur5 commented Dec 6, 2019 •

edited

Loading

This comment has been minimized.

agilgur5 commented Dec 23, 2019 •

edited

Loading

agilgur5 commented Jan 18, 2020

agilgur5 commented Apr 20, 2022

		@@ -0,0 +1,22 @@
		// signature_pad's props
		// (taken from https://github.com/szimek/signature_pad/blob/e2af461bca7ceb7cc5afb8349930908ca19f2f56/src/signature_pad.ts#L23)

add typescript definitions #25

add typescript definitions #25

Conversation

chillitom commented Nov 5, 2018

agilgur5 May 29, 2019

Choose a reason for hiding this comment

agilgur5 Jul 14, 2019

Choose a reason for hiding this comment

agilgur5 Aug 19, 2019

Choose a reason for hiding this comment

agilgur5 May 29, 2019

Choose a reason for hiding this comment

agilgur5 May 29, 2019

Choose a reason for hiding this comment

agilgur5 commented May 29, 2019 • edited Loading

benny-medflyt commented Aug 19, 2019

agilgur5 commented Aug 19, 2019 • edited Loading

This comment has been minimized.

agilgur5 commented Dec 6, 2019 • edited Loading

This comment has been minimized.

agilgur5 commented Dec 23, 2019 • edited Loading

agilgur5 commented Jan 18, 2020

agilgur5 commented Apr 20, 2022

agilgur5 commented May 29, 2019 •

edited

Loading

agilgur5 commented Aug 19, 2019 •

edited

Loading

agilgur5 commented Dec 6, 2019 •

edited

Loading

agilgur5 commented Dec 23, 2019 •

edited

Loading