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

JimpInstance type can't be used consistently when working with Jimp instances #1337

Open
vntw opened this issue Sep 9, 2024 · 5 comments
Open

Comments

@vntw
Copy link

vntw commented Sep 9, 2024

Expected Behavior

Before v1, it was possible to pass around Jimp as a type and throw in the results of Jimp.read/Jimp.create/new Jimp. Example without TS errors:

import Jimp from "jimp"

const img1 = await Jimp.read("")
const img2 = await Jimp.create("")
const img3 = new Jimp("")

function test(j: Jimp) {}

test(img1) // works
test(img2) // works
test(img3) // works

See https://codesandbox.io/p/sandbox/g56q6v?file=%2Findex.mts

Current Behavior

#1330 already added an improvement via the JimpInstance type, but it seems like it doesn't work the same way (interchangeably).

import { Jimp, JimpInstance } from "jimp";

const inst = new Jimp({ width: 100, height: 100 });
const img = await Jimp.read("…");

function testInstance(j: JimpInstance) {}
function testRead(j: Awaited<ReturnType<typeof Jimp.read>>) {}

testInstance(img); // error
testRead(img);

testInstance(inst);
testRead(inst); // error

See https://codesandbox.io/p/sandbox/jimp-test-249ch8-pk8qdr?file=%2Findex.mts

Workaround for now is to use Awaited<ReturnType<typeof Jimp.read>>

Failure Information (for bugs)

Steps to Reproduce


Ref. #1328 (comment)

@phyzical
Copy link

can confirm the workaround works for me

@hipstersmoothie
Copy link
Collaborator

Would either of you like to contribute this via PR?

@hipstersmoothie
Copy link
Collaborator

There may be a way we can massage the types to not need this an just use JImp as a type. I don't have too much time to figure that out tough

@phyzical
Copy link

phyzical commented Oct 8, 2024

Im not sure how to solve it properly either TBH (not a huge TS user), i don't think hacking in another export is the right play either long term

@vntw
Copy link
Author

vntw commented Oct 21, 2024

There may be a way we can massage the types to not need this an just use JImp as a type.

That's what I had hoped, I tried to get sth. working after opening this issue but unfortunately that's where my TS skills stop apparently, sorry 😅

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

No branches or pull requests

3 participants