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

Enhancement: improve fail-fast nature of constructor when null accidentally passed as input (a somewhat breaking change) #1768

Closed
ericblade opened this issue Jun 27, 2019 · 4 comments

Comments

@ericblade
Copy link

Discovered in relation to #1767 . Also appears same as long closed #510

sharp(undefined, { raw: ... }).png().toBuffer()

never resolves or rejects.

@lovell
Copy link
Owner

lovell commented Jun 27, 2019

Passing in an undefined input currently switches sharp into a WritableStream, in this case it would expect uncompressed, raw pixel data to be streamed in, the shape of which is defined by raw: ...

The above example is the same as using the sharp({ raw: ... })... syntax.

Are you expecting to use Stream-based input? If not, how are you planning to provide input data?

@ericblade
Copy link
Author

I had thought i was passing it a buffer, but i was accessing the wrong key of my object that held the buffers. :-) I thought that's what it was doing, was giving me a WritableStream, but I wasn't sure if it was valid to pass (undefined, { raw.. }) to it.

I'm thinking perhaps checking arguments.length then throwing if that doesn't match what would be expected? (ie, if there are two arguments, but the first one is undefined, it probably means the user made a mistake, rather than the user intended to create a WritableStream) .. that's the only way I can think of to achieve that without breaking the existing interface, anyway.

also, throwing on toBuffer() while there's nothing there? maybe?

@lovell
Copy link
Owner

lovell commented Jul 1, 2019

The two-param constructor-function that allows the first param to be null/undefined was retained for backwards compatibility but I agree that it is confusing. Let's mark this as an enhancement to have that usage throw in the next major release.

(toBuffer() can't synchronously throw as input is asynchronously provided.)

@lovell lovell changed the title toBuffer with no data never fulfills promise Enhancement: improve fail-fast nature of constructor when null accidentally passed as input (a somewhat breaking change) Jul 1, 2019
@lovell lovell added this to the v0.24.0 milestone Nov 13, 2019
@lovell
Copy link
Owner

lovell commented Jan 16, 2020

v0.24.0 is now available with your suggested API improvement, thank you.

@lovell lovell closed this as completed Jan 16, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants