sindresorhus / file-type

Detect the file type of a file, stream, or data
MIT License
3.72k stars 354 forks source link

Changes on typing break with `stream.Readable` #675

Closed ajotaos closed 1 month ago

ajotaos commented 2 months ago

Description

It seems an update of a type library dependency where the input types for fileTypeFromStream come from, no longer allow to pass stream.Readable. This is the error I'm receiving:

Argument of type 'Readable' is not assignable to parameter of type 'AnyWebByteStream'.
  Type 'ReadableBase' is missing the following properties from type 'ReadableStream<Uint8Array>': locked, cancel, getReader, pipeThrough, and 3 more.ts(2345)

I would need to use Readable.toWeb to accommodate the typing but the library would crash with:

The argument 'stream' must be a byte stream. Received ReadableStream { locked: false, state: 'reada
ble', supportsBYOB: false }

Existing Issue Check

ESM (ECMAScript Module) Requirement Acknowledgment

File-Type Scope Acknowledgment

Borewit commented 1 month ago

It looks like your stream.Readable is not recognized as an instance of stream.Readable and hence the (wrong) assumption is made you passed a ReadableStream.

Can you give more details @ajotaos:

ajotaos commented 1 month ago

I'm using regular Node.js v20.

The stream comes from the AWS S3 library GetObject Output although the same result occurs for any readable instance I'm creating, even with const readable = new stream.Readable();

Also, I'm using typescript to type the input using import type { Readable } from 'node:stream'.

The ReadableStream I mentioned it's because that's what the type system is requiring me to pass to silence the type error but leads to a runtime error.

It's a codebase I have from work but the non-confidential portion of it goes like:

import { fileTypeFromStream } from 'file-type';

import type { Readable } from 'node:stream';

async function getMimeTypeFromStream(body: Readable) {
    const fileType = await fileTypeFromStream(body);
    return fileType?.mime ?? 'application/octet-stream';
}

Using:

"type-fest": "^4.21.0"

Borewit commented 1 month ago

Works just fine on my end:

node -v ➡️ v20.17.0

import { fileTypeFromStream } from 'file-type';
import * as stream from 'node:stream';

async function getMimeTypeFromStream(body) {
  const fileType = await fileTypeFromStream(body);
  return fileType?.mime ?? 'application/octet-stream';
}

// Create a custom Readable stream
const readable = new stream.Readable({
  read() {
    // Push some binary data (for example, a PNG file signature)
    this.push(Buffer.from([0x47, 0x49, 0x46]));
    // No more data
    this.push(null);
  }
});

// Get the mime type from the stream
(async () => {
  const result = await getMimeTypeFromStream(readable);
  console.log(result); // Expected output: "image/gif"
})();

➡️ image/gif

ajotaos commented 1 month ago

I'm talking about the type signature not allowing stream.Readable to be passed to fileTypeFromStream. The implementation allows it, but Typescript does not. @Borewit

Screenshot 2024-09-26 at 2 29 08 PM

Borewit commented 1 month ago

Works fine with TypeScript for me as well:

import { fileTypeFromStream } from 'file-type';
import { Readable } from 'node:stream';

async function getMimeTypeFromStream(body: Readable) {
    const fileType = await fileTypeFromStream(body);
    return fileType?.mime ?? 'application/octet-stream';
}

// Create a custom Readable stream
const readable = new Readable({
    read() {
        // Push some binary data (for example, a PNG file signature)
        this.push(Buffer.from([0x47, 0x49, 0x46]));
        // No more data
        this.push(null);
    }
});

// Get the mime type from the stream
(async () => {
    const result = await getMimeTypeFromStream(readable);
    console.log(result); // Expected output: "image/gif"
})();

Repo: https://github.com/Borewit/file-type-example/blob/master/ts-esm/test-readable.ts

ajotaos commented 1 month ago

The typing of fileTypeFromStream in your current latest release has this signature:

import type { AnyWebByteStream } from 'strtok3';
export function fileTypeFromStream(stream: AnyWebByteStream): Promise<FileTypeResult | undefined>;

Source: https://github.com/Borewit/strtok3/blob/e7c32adb7fa2e25735cb3ff3f8f39943a635a047/lib/core.ts#L8

strtok3.AnyWebByteStream is a re-export from the peek-readable library.

export { type AnyWebByteStream } from 'peek-readable';

Source: https://github.com/Borewit/strtok3/blob/e7c32adb7fa2e25735cb3ff3f8f39943a635a047/lib/core.ts#L8

And peek-readable. AnyWebByteStream has the signature:

import type { ReadableStream as NodeReadableStream } from 'node:stream/web';

export type AnyWebByteStream = NodeReadableStream<Uint8Array> | ReadableStream<Uint8Array>;

Source: https://github.com/Borewit/peek-readable/blob/b2fa00f40c0df4732208e107dcc2dca760cb22dc/lib/WebStreamReader.ts#L6

In this signature, node:stream.Readable does match that condition.

Borewit commented 1 month ago

This is the applicable type declaration::

https://github.com/sindresorhus/file-type/blob/0e91f7be09fbe7f1ac3928d96b7a23b611532d52/index.d.ts#L59

where NodeReadableStream is exactly the type you are looking for:

https://github.com/sindresorhus/file-type/blob/0e91f7be09fbe7f1ac3928d96b7a23b611532d52/index.d.ts#L5

Note that these typing are only available via the Node.specific entry point, as the node:stream is a Node specific dependency.

Crauzer commented 1 month ago

I have the same issue as @ajotaos above. I'm trying to verify the file type of a file being uploaded in a Remix JS upload handler. The data itself is AsyncIterable<Uint8Array> that gets converted into a node Readable

image

Borewit commented 1 month ago

I have the same issue as @ajotaos above. I'm trying to verify the file type of a file being uploaded in a Remix JS upload handler. The data itself is AsyncIterable that gets converted into a node Readable

@Crauzer, if you need to use the Node.js-specific functions from the file-type module, you must ensure that you're importing it server-side in your Remix app. Remix allows you to handle different environments, and Node.js functionality (like stream processing) is only available on the server side. If you attempt to import the module on the client side, it will resolve to the default export, which is browser-compatible and does not include Node.js functionality.

To resolve this, ensure that your import is happening in a server-only context (e.g., inside loaders or actions). If this approach doesn’t solve the problem, it could be worth raising the issue on Remix’s side.

Borewit commented 1 month ago

I close this issue under the assumption it is resolves, otherwise you may re-open.