Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Properly split constructor and instance types #867

Merged

Conversation

forivall
Copy link
Contributor

@forivall forivall commented Mar 28, 2020

Fixes #803

What's Changing and Why

Currently, the typings indicate that one can do the following:

import Jimp = require('jimp');
const jimpInst: Jimp = new Jimp(0, 0);
// Typings indicate that this is 'image/jpeg'
jimpInst.MIME_JPEG

But actually running it (in a repl) shows

> var Jimp = require('jimp');
undefined
> var jimpInst = new Jimp(0, 0);
undefined
> jimpInst.MIME_JPEG
undefined

This change fixes that, and splits the instance type from the class constructor type.

What else might be affected

The jpeg plugin's constants typing was incorrect, idk about other plugins (that one was caught as it was used in my code)

Tasks

  • Add tests
  • Update Documentation
  • Update jimp.d.ts
  • Add SemVer Label
📦 Published PR as canary version: 0.9.9-canary.867.792.0

✨ Test out this PR locally via:

npm install @jimp/cli@0.9.9-canary.867.792.0
npm install @jimp/core@0.9.9-canary.867.792.0
npm install @jimp/custom@0.9.9-canary.867.792.0
npm install jimp@0.9.9-canary.867.792.0
npm install @jimp/plugin-blit@0.9.9-canary.867.792.0
npm install @jimp/plugin-blur@0.9.9-canary.867.792.0
npm install @jimp/plugin-circle@0.9.9-canary.867.792.0
npm install @jimp/plugin-color@0.9.9-canary.867.792.0
npm install @jimp/plugin-contain@0.9.9-canary.867.792.0
npm install @jimp/plugin-cover@0.9.9-canary.867.792.0
npm install @jimp/plugin-crop@0.9.9-canary.867.792.0
npm install @jimp/plugin-displace@0.9.9-canary.867.792.0
npm install @jimp/plugin-dither@0.9.9-canary.867.792.0
npm install @jimp/plugin-fisheye@0.9.9-canary.867.792.0
npm install @jimp/plugin-flip@0.9.9-canary.867.792.0
npm install @jimp/plugin-gaussian@0.9.9-canary.867.792.0
npm install @jimp/plugin-invert@0.9.9-canary.867.792.0
npm install @jimp/plugin-mask@0.9.9-canary.867.792.0
npm install @jimp/plugin-normalize@0.9.9-canary.867.792.0
npm install @jimp/plugin-print@0.9.9-canary.867.792.0
npm install @jimp/plugin-resize@0.9.9-canary.867.792.0
npm install @jimp/plugin-rotate@0.9.9-canary.867.792.0
npm install @jimp/plugin-scale@0.9.9-canary.867.792.0
npm install @jimp/plugin-shadow@0.9.9-canary.867.792.0
npm install @jimp/plugin-threshold@0.9.9-canary.867.792.0
npm install @jimp/plugins@0.9.9-canary.867.792.0
npm install @jimp/test-utils@0.9.9-canary.867.792.0
npm install @jimp/bmp@0.9.9-canary.867.792.0
npm install @jimp/gif@0.9.9-canary.867.792.0
npm install @jimp/jpeg@0.9.9-canary.867.792.0
npm install @jimp/png@0.9.9-canary.867.792.0
npm install @jimp/tiff@0.9.9-canary.867.792.0
npm install @jimp/types@0.9.9-canary.867.792.0
npm install @jimp/utils@0.9.9-canary.867.792.0
# or 
yarn add @jimp/cli@0.9.9-canary.867.792.0
yarn add @jimp/core@0.9.9-canary.867.792.0
yarn add @jimp/custom@0.9.9-canary.867.792.0
yarn add jimp@0.9.9-canary.867.792.0
yarn add @jimp/plugin-blit@0.9.9-canary.867.792.0
yarn add @jimp/plugin-blur@0.9.9-canary.867.792.0
yarn add @jimp/plugin-circle@0.9.9-canary.867.792.0
yarn add @jimp/plugin-color@0.9.9-canary.867.792.0
yarn add @jimp/plugin-contain@0.9.9-canary.867.792.0
yarn add @jimp/plugin-cover@0.9.9-canary.867.792.0
yarn add @jimp/plugin-crop@0.9.9-canary.867.792.0
yarn add @jimp/plugin-displace@0.9.9-canary.867.792.0
yarn add @jimp/plugin-dither@0.9.9-canary.867.792.0
yarn add @jimp/plugin-fisheye@0.9.9-canary.867.792.0
yarn add @jimp/plugin-flip@0.9.9-canary.867.792.0
yarn add @jimp/plugin-gaussian@0.9.9-canary.867.792.0
yarn add @jimp/plugin-invert@0.9.9-canary.867.792.0
yarn add @jimp/plugin-mask@0.9.9-canary.867.792.0
yarn add @jimp/plugin-normalize@0.9.9-canary.867.792.0
yarn add @jimp/plugin-print@0.9.9-canary.867.792.0
yarn add @jimp/plugin-resize@0.9.9-canary.867.792.0
yarn add @jimp/plugin-rotate@0.9.9-canary.867.792.0
yarn add @jimp/plugin-scale@0.9.9-canary.867.792.0
yarn add @jimp/plugin-shadow@0.9.9-canary.867.792.0
yarn add @jimp/plugin-threshold@0.9.9-canary.867.792.0
yarn add @jimp/plugins@0.9.9-canary.867.792.0
yarn add @jimp/test-utils@0.9.9-canary.867.792.0
yarn add @jimp/bmp@0.9.9-canary.867.792.0
yarn add @jimp/gif@0.9.9-canary.867.792.0
yarn add @jimp/jpeg@0.9.9-canary.867.792.0
yarn add @jimp/png@0.9.9-canary.867.792.0
yarn add @jimp/tiff@0.9.9-canary.867.792.0
yarn add @jimp/types@0.9.9-canary.867.792.0
yarn add @jimp/utils@0.9.9-canary.867.792.0

@crutchcorn
Copy link
Member

Additional to the install error present in CI, can we add some tests to the types testing file:

https://github.com/oliver-moran/jimp/blob/master/packages/jimp/types/ts3.1/test.ts

In order to properly test both constructor and static methods?

@forivall
Copy link
Contributor Author

Yeah, i'm working through those tests and discovering other stuff that should be in the typings (like decoders)

@forivall
Copy link
Contributor Author

Tests are now passing locally for me. Ready for review.

@forivall
Copy link
Contributor Author

forivall commented Mar 28, 2020

Whoops, i didn't know that tsTest isn't run with yarn test. Customs now erroring out on ts3.1 but passing on 3.2; fully passing the main tsTest.

Let me know if bumping the ts requirement to 3.2 is fine.

@crutchcorn
Copy link
Member

crutchcorn commented Mar 29, 2020

I'm so sorry it took me so long to review @forivall. Your last commit just so happened to exceed the time I had for the day (which I didn't make clearer beforehand).

FWIW I audibly said "Holy shit she did great" when reviewing the one section that I was initially hesitant on. This looks awesome to me! Let's bump the TS requirement to 3.2, rename the ts3.1 files to ts3.2 and get this approved up! :D

@forivall
Copy link
Contributor Author

I figured out how to fix the typescript 3.1 types without bumping the version requirements.

@crutchcorn
Copy link
Member

Even better! Lookin' great. @hipstersmoothie, this should be good-to-go! Easily a minor release :D

@hipstersmoothie hipstersmoothie added the minor Increment the minor version when merged label Mar 30, 2020
@hipstersmoothie hipstersmoothie merged commit e1d05da into jimp-dev:master Mar 30, 2020
@hipstersmoothie
Copy link
Collaborator

🚀 PR was released in v0.10.0 🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
minor Increment the minor version when merged released This issue/pull request has been released.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

TypeScript: Jimp types are not corresponding to Jimp implementation
3 participants