photostructure / exiftool-vendored.js

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

Adding values to numericTags does not reflect in Tags type, nor is the function allowed to be properly extended to fix the return value #158

Closed djak250 closed 9 months ago

djak250 commented 9 months ago

Describe the bug exifTool.read<T> requires T to extend Tags. However, passing in numericTags on the ReadTaskOptions potentially modifies the expected value. For instance, FocalLength has a type of string in Tags, but when passed into numericTags, the actual value is a number. When trying to override the generic to allow this type to be reflected in the type checking, the compiler complains, since setting FocalLength to number (it's actual type) breaks the extension of Tags, which is required by the type signature of exifTool.read.

To Reproduce

type Image = { path: string }
type TagsWithNumericFocalLength = Omit<Tags, "FocalLength"> & {
  FocalLength?: number
}
const exifData = async (image: Image): Promise<TagsWithNumericFocalLength> => {
  const readTaskOptions: ReadTaskOptions = {
    ...DefaultReadTaskOptions,
    numericTags: DefaultReadTaskOptions.numericTags.concat(["FocalLength"]),
  }
  const tags = await exiftool.read<TagsWithNumericFocalLength>(
    image.path,
    undefined,
    readTaskOptions
  )
  return tags
}

Expected behavior const tags to be of type TagsWithNumericFocalLength

Actual behavior

Type 'TagsWithNumericFocalLength' does not satisfy the constraint 'Tags'.
  Types of property 'FocalLength' are incompatible.
    Type 'number' is not assignable to type 'string'.

due to overly strict type enforcement.

mceachen commented 9 months ago

The typing of Tags is only best-effort, and this is actually behaving as expected--if you're setting numeric values or any other advanced settings, I'd expect you to wrap ExifTool.read in some service layer, and Pick the Tag interface to taste.

If you've got a good idea for adding dynamic typing to Tag based on options inference (which would require the ExifTool class to pull in a generic type that then mutates .read's return value), know that I'm happy to review pull requests. Full disclosure, though: I'm a bit hesitant to add labyrinthine TypeScript signatures, as they can be a bit brittle on major TypeScript version upgrades.

djak250 commented 9 months ago

I don't know if it is possible to have the type update based on the dynamic properties of the ReadTaskInputs. But my main request is to remove the extends requirement for read<T extends Tags = Tags> to instead be read<T = Tags>. I understand it gives the end developer the opportunity to mistype things to not be a close match to the Tags interface, but if they're overriding the default generic, I think it's safe to say they're being intentional about the override.