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

Missing fileTypeOptions for fileTypeFromStream function in index declaration file #715

Open
3 tasks done
sly7-7 opened this issue Dec 30, 2024 · 3 comments
Open
3 tasks done
Labels

Comments

@sly7-7
Copy link
Contributor

sly7-7 commented Dec 30, 2024

Description

The options with customDetectors are missing in the declaration file, see:

export function fileTypeFromStream(stream: AnyWebReadableStream<Uint8Array> | NodeReadableStream): Promise<FileTypeResult | undefined>;

Existing Issue Check

  • I have searched the existing issues and could not find any related to my problem.

ESM (ECMAScript Module) Requirement Acknowledgment

  • My project is an ESM project and my package.json contains the following entry: "type": "module".

File-Type Scope Acknowledgment

  • I understand that file-type detects binary file types and not text or other formats.
@sly7-7 sly7-7 added the bug label Dec 30, 2024
@sly7-7 sly7-7 changed the title Missing fileTypeOptions in index declaration file Missing fileTypeOptions for fileTypeFromStream function in index declaration file Dec 30, 2024
@sly7-7
Copy link
Contributor Author

sly7-7 commented Dec 30, 2024

Also, I've noted that the exported functions from core.js don't have the detectors in their signatures. Maybe this was on purpose ? In my use case then, I've used straight the FileTypeParser() constructor passing it a detector.
Please let me know if this is the right way, or if you would accept a pr with the detectors available in core.js.

@Borewit
Copy link
Collaborator

Borewit commented Jan 16, 2025

The options are not missing from the types, the options can only be passed via the constructor. Which may like or not like, but that is not a bug.

My philosophy behind was something like, if you do want to change the default behavior of file-type, you probably do not mind to call the constructor. That way you can also reuse the customized instance, for assessing multiple files.

To me it feels a bit odd if each function passes the options to the constructor. Then we also have repeat the documentation for it. But I also see it may appear a bit more convenient.

@sly7-7
Copy link
Contributor Author

sly7-7 commented Jan 17, 2025

Thank your for your answer, I understand your philosophy, and if I understand it well, then I'm doing what's intended 😃

One thing though, unless I'm missing something else (which can be very possible 😅 ), there is an inconsistency between index.js where fileTypeOptionsis in the signature

export async function fileTypeFromStream(stream, fileTypeOptions) {

whereas it does not appear in index.d.ts:

export function fileTypeFromStream(stream: AnyWebReadableStream<Uint8Array> | NodeReadableStream): Promise<FileTypeResult | undefined>;

Also I've just noticed an other small thing looking weird to me:

return (new FileTypeParser(fileTypeOptions)).fromFile(path, fileTypeOptions);

fileTypeOptions beeing passed to fromFile(), but

async fromFile(path) {

does not use it.

One more time, maybe I'm totally wrong and I just don't read the code correctly, but if I'm right, I could make a PR to remove these inconsistencies.

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

No branches or pull requests

2 participants