photostructure / exiftool-vendored.js

Fast, cross-platform Node.js access to ExifTool
https://photostructure.github.io/exiftool-vendored.js/
MIT License
431 stars 43 forks source link

`ExifTool.readRaw` incorrectly has return type `Promise<Tags>` #138

Closed silvio2402 closed 1 year ago

silvio2402 commented 1 year ago

Documentation/Type Issue Proper documentation and typing is very important. If for example a return value is wrongly typed, a developer might feel comfortable not checking the types at runtime which can cause issues.

The function at ExifTool.readRaw is noted to return field values of type string | number | string[]. However, the type of the return value is falsely defined as Promise<Tags> in the built ExifTool.d.ts file. I believe this is due to the ReadRawTask extending ExifToolTask<Tags>.

Could we create a type, for example:

type RawTags = Record<string, number | string | string[]>;

...and use it to extend ReadRawTask with ExifToolTask<RawTags>, which would come at the cost of editor autocompletions of tags?

But also the documentation for ExifTool.readRaw is wrong because I've seen it return boolean for some tags (for example FlashFired). Also, how does readRaw handle structs?

I'm getting the feeling that the actual type of RawTags is just anything json serializable, for which I would recommend this type:

type Literal = string | number | boolean;
type Json = Literal | { [key: string]: Json } | Json[];
type RawTags = Json;

Please let me know if I've missed anything. I hope we can resolve this issue. Thanks for the work!

mceachen commented 1 year ago

Thanks for this suggestion. I agree with you about how important good types are.

I'm getting the feeling that the actual type of RawTags is just anything json serializable

I think it should reflect what ExifTool actually renders. For the most part, that's just number | number[] | string, but there are a handful of boolean tags, and struct tags (like Face, Look, and CreatorContactInfo) that support deeper structures (the -struct argument is added by default for other readTags calls). So... yeah. It's arbitrary JSON.

We can at least tighten up the top level RawTags definition a bit, as we know the top Record is string-indexed.

/**
 * Loosely typed raw result from ExifTool
 *
 * @see https://github.com/photostructure/exiftool-vendored.js/issues/138
 */
export type RawTags = Record<string, Json>

I'm releasing v21.4 with this update now.