Fix 0.8 typings, add type tests

[crutchcorn]

What's Changing and Why

This PR fixes issues with typings introduced by myself in #770 and fixes #785

As part of the discussion, we wanted to add TS tests. I have done so by using dtslint which will tests against all versions of TS from 2.8 and above (it could test down to 2.0, but due to specific features of our typings, TS 2.8 is needed)

Because dtslint tests only seems to against specific folders rather than using a regex, there are two seperate package.json commands to tests them. I would love to make them one command and add to test, but I figured that should have broader conversion with the maintainers of the project

What else might be affected

We might want to include this into our documentation for contributing (?)

Tasks

• Add tests
• Update Documentation
• Update jimp.d.ts
• Add SemVer Label

Published PR with canary version: 0.8.1-canary.786.262.0

[crutchcorn]

It seems there to still be issues somewhere, as exposed by tests:

TSError: ⨯ Unable to compile TypeScript:
packages/cli/src/cli.ts(62,58): error TS2345: Argument of type 'BaseJimp & { MIME_JPEG: "image/jpeg"; _quality: number; quality: (n: number, cb?: ImageCallback) => undefined; } & { 'image/jpeg': string; } & Dither & Blit & Rotate & Color & Blur & ... 18 more ... & { ...; }' is not assignable to parameter of type 'undefined'.
packages/cli/src/cli.ts(243,11): error TS2532: Object is possibly 'undefined'.

[crutchcorn]

It seems there is some undefined present in the typings that are throwing problems. I am unable to find it currently - will resume investigation tomorrow and enhanse the TS tests to fix this problem

[crutchcorn]

Please withold merging for now despite passed tests. I have some things I need to writeup - this has needed more consideration than I initially thought

[hipstersmoothie]

@crutchcorn Thanks for all your work!

[crutchcorn]

These typings have been a nightmare trying to figure out, not going to lie. Once again, I am so sincerely sorry that I didn't realize that sooner and that I didn't test things as thoroughly before. That said, the tests for the typings should be very thorough now and I have tested both the jimp mainline package as well as the configure function custom composing. All in all, as thorough as testing as I could reasonably do to try and ensure that it all works as-expected.

So, that's the good news - now for the not-so-good:

1. The typings have changed dramatically since the initial 0.8 typings (primarily for the better, the typings are much more centralized now [and simpler!], typing files for configure have been broken out to their own files for management, etc). I'm unsure if this is REALLLLY a problem, given that 0.8 initial release I cannot imagine anyone having actually used this version in their project long enough to rely on it's horribly broken types

2. TypeScript 3.1 users are going to basically be the only ones that can use these typings (at least for the mainline jimp package). The reasoning for this is due to some issues in prior versions of TypeScript that would more aggressively choose to use any as the typing for any sufficiently complex type (which our mainline jimp package type as-of-now THOROUGHLY qualifies as).

Essentially, point #2 means that - contrary to my prior statement in the initial PR - the big breaking change would be a requirement to use TS 3.1+ (TS is currently at 3.6RC) rather than 2.8 (which is what the minimum we'd need to be able to use even a minimal configure function due to features I'd used in the typing package).

Now, TS 3.1 added support for the typesVersions field in the package.json, so if there is a broad concern for preserving even lower versions of TS for the jimp package, what we could do is have a types/3.1 subfolder in the jimp package directory that has the index.d.ts file in THIS PR for the newer versions of TS and have the old (<0.7) typing index.d.ts file the default for older versions.

If we do end up doing this, I'd suggest we mark THAT typing file as depreciated and support it for at most a few more minor releases

One last thing of note:

The question regarding the newly created npm scripts:

• tsTest:custom

• tsTest:main

These files are the dtslint test files for the packages (the configure and the jimp mainline package). I think we should add them to the test script, but am unsure how we want to structure that. It seems there's a CI system I'm not familiar with, so I wanted to raise that as a question of how we'd want to add it to the automated test suite

Apologies if I end up having a delayed response - I've stayed up far too late working on this issue. I was wanting to get it wrapped up as soon as possible due to the breaking nature, but now is time for rest 😴

[patheticcockroach]

the big breaking change would be a requirement to use TS 3.1+ (TS is currently at 3.6RC)

Of course I can't speak for everyone, but I've been using Typescript for a while, and in my company we've always updated it pretty aggressively (I didn't even know 3.6 was just a RC). Requiring 3.1+ is probably very fine.

[crutchcorn]

Oh, apologies - 3.6 is stable as-of Aug 28.

I think the 3.1 requirement is relatively okay as I've experienced the same in every group I've written TS in, but it's tough to say as I lack statistics of new TS version adoption (and while popular Twitter user polls seem to agree, those are highly skewed and inaccurate).

I will mention that the pre-0.8 typings seem to support TS 2.1 so I certainly understand the gap between 2.1 and 3.1. (especially as TS releases are highly breaking [despite the single strict semver major bump]).

This is why I think having a depreciation notice and supporting older typings for a bit might be advantageous, at least until the newer TS versions have been stable for longer (Sep 27th, 2018 is the release date for TS 3.1). Particularly due to the relatively low short-term maintanance cost (just the jimp package, only needing a subfolder)

[hipstersmoothie]

I like the idea of a folder for the old types 👍 easy to remove when we want to

[crutchcorn]

Unfortunately, it'd have to either be a folder for the new types (again, only for the jimp package), or a folder for both (types/old, types/new?)

So then the question is:

1. Do we want one folder or two?
   1.a) What do we want the folder/s called?
2. Do we want to add a depreciation warning on the old typings?

When I add the old typings (I'll try to get this done by lunch. Our type tests should work fine for both versions) I'll try my best to add in the missing types for the plugins:

• Circle
• Fisheye
• Shadow
• Threshold

That were added to the typings (as mentioned by the initial #770 PR)

[hipstersmoothie]

one folder just for the old ones in case you want to use them. the new types should be the default

[hipstersmoothie]

Do we want to add a depreciation warning on the old typings?

If we can

[crutchcorn]

Right, and I agree the new types should be the default, but TypeScript automatically handles the import (due to the typesVersions field in the package.json) to provide the proper typing based on the version of TS used. That said, this feature only works on TS 3.1+ (as TS also provides it's language extensions to IDEs) so having two folders is probably the only way to go

I'll see if I can get a PR in this afternoon

[crutchcorn]

I am having the absolute HARDEST time getting typesVersions field to have consistent behavior with builds and dtslint. @weswigham I know you're not related to this PR, and only commented on the typings being broken in the TS repo, but I'd sincerely really love to get some additional input/feedback (like a review or just a confirmation that this works for you in some way) on this. I want to ensure that there's nothing structurally wrong with the things that've been setup here and have no idea who to ask for help with such complex typings/system (I couldn't find much in the way of an official TS chat community)

FWIW the build builds just fine on my machine (class or interface be darned) and I've been able to make brand new TS3.1 and TS2.8 projects, but there have been inconsistencies with:

• cannot find module 'jimp' during the npm run tsTest:main tests (for 3.1, never 2.8) (I suspect that yarn's resolution might be the cause of this)
• jimp being any or never in the newly created TS projects I'd used to test this (which doesn't seem to be happening in 01c6dc0, but the debugging process is slow and I'm not entirely confident in it) [once again, yarn MAY be the issue here]
• jimp imports being inconsistently able to be merged due to the 2.8 typings somehow merging with the Jimp from @jimp/core (I believe this is due to interface namespace polution, now that I think about it)

Unfortunately, I was unable to get moving the old types into one folder and the new types into another working well. The typesVersions seems to need a bit of a docs buff for examples and the only thing I was able to go off of was some examples from DefinitelyTyped that use the types/3.1 folder structure), but I'm having a hard time figuring out what was broken during my testing was due to yarn link having inconsistent module resolution and what was legitamately broken (if anything, anymore).

Alllll of that said - @hipstersmoothie I think this PR should be good-to-go FINALLY. Can we push a canary build of 0.8 to ensure that this PR has fixed the issue entirely so I can confirm on a published version to eliminate yarn link issues from being the cause of any issues?

[hipstersmoothie]

Ever push to a PR is published in a canary already. If you check the top comment (from you) the latest canary version is at the bottom

…

On Tue, Sep 10, 2019 at 5:31 PM Corbin Crutchley ***@***.***> wrote: I am having the absolute HARDEST time getting typesVersions field to have consistent behavior with builds and dtslint. @weswigham <https://github.com/weswigham> I know you're not related to this PR, and only commented on the typings being broken in the TS repo, but I'd sincerely really love to get some additional input/feedback (like a review or just a confirmation that this works for you in some way) on this, I feel like I'm flying absolutuely blind here and have no idea who to ask for help (the typings are complex enough that everyone I know to ask seemed unable to assist - I couldn't find much in the way of an official TS chat community) FWIW the build builds just fine on my machine (class or interface be darned) and I've been able to make brand new TS3.1 and TS2.8 projects, but there have been inconsistencies with: - cannot find module 'jimp' during the npm run tsTest:main tests (for 3.1, never 2.8) (I suspect that yarn's resolution might be the cause of this) - jimp being any or never in the newly created TS projects I'd used to test this (which doesn't *seem* to be happening in 01c6dc0 <01c6dc0>, but the debugging process is slow and I'm not entirely confident in it) [once again, yarn MAY be the issue here] - jimp imports being inconsistently able to be merged due to the 2.8 typings somehow merging with the Jimp from @jimp/core (I believe this is due to interface namespace polution, now that I think about it) Unfortunately, I was unable to get moving the old types into one folder and the new types into another working well. The typesVersions seems to need a bit of a docs buff for examples and the only thing I was able to go off of was some examples from DefinitelyTyped that use the types/3.1 folder structure), but I'm having a hard time figuring out what was broken during my testing was due to yarn link having inconsistent module resolution and what was legitamately broken (if anything, anymore). Alllll of that said - @hipstersmoothie <https://github.com/hipstersmoothie> I think this PR should be good-to-go *FINALLY*. Can we push a canary build of 0.8 to ensure that this PR has fixed the issue entirely so I can confirm on a published version to eliminate yarn link issues from being the cause of any issues? — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#786>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AAJDEBCUL5IX255JSBYMHALQJA377ANCNFSM4IUVJ3SA> .

[crutchcorn]

I just confirmed that this PR fixes typings for both TS 2.8 and 3.5 (the two versions I tested manually against) and works as-expected (loading new typings from the correct folder, etc) with the canary builds

[hipstersmoothie]

Cool! Can you change the versions back? (Unless you already did) I will merge this once that’s fixed

…

On Tue, Sep 10, 2019 at 6:23 PM Corbin Crutchley ***@***.***> wrote: I just confirmed that this PR fixes typings for both TS 2.8 and 3.5 (the two versions I tested manually against) and works as-expected (loading new typings from the correct folder, etc) with the canary builds — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#786>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AAJDEBF3PJRGNCBJMRSC44LQJBCABANCNFSM4IUVJ3SA> .

[crutchcorn]

I already reverted the versions (via force push to not muddy commit history more than needed)

[hipstersmoothie]

Good catch!

[hipstersmoothie]

@crutchcorn Thank you for all your hard work once again. This seems to have really come together and it's awesome that users of @jimp/custom can utilize this!

[hipstersmoothie]

🚀 PR was released in v0.8.1 🚀