devongovett / exif-reader

A small EXIF image metadata reader
MIT License
145 stars 22 forks source link

Add typescript declaration file #28

Closed atombrenner closed 1 year ago

atombrenner commented 1 year ago

This PR adds a typescript definition file the the package, see #27

It allows typescript users to directly use this package without installing another dependency from DefinitelyTyped. It allows the maintainer to add/change/remove types and keep the type definitions in sync with the implementation.

In addition to the existing type annotation at DefinitelyTyped, most tags have now more specific types, e.g. ImageWidth is now of type number which is more precise than unknown.

The tag names were automatically generated from tag.js. During that processed I noticed, that tag.s uses the same tag name for different tags (e.g. the WhiteBalance tag). That makes typing hard, because at runtime it depends on the image which tag is returned. The last encountered one overwrites existing tags. I don't know if this is a bug or a feature, but in those cases we can only return a generic tag type, which is

type GenericTag = number | number[] | string | Buffer | null;
lovell commented 1 year ago

Thank you very much for the PR Christian.

/cc @akwodkiewicz as they provided the current definitions via https://github.com/DefinitelyTyped/DefinitelyTyped/pull/62942

atombrenner commented 1 year ago

Hey @lovell, thanks for the tip to use tsd to test the index.d.ts. It found a bug and and also helped improving the declaration file, it feels like TDD for typings :smile:

Handling the deprecation process seems pretty straight forward, so I can take care if @akwodkiewicz is not interested.

lovell commented 1 year ago

@atombrenner Should we close this PR and continue discussion using https://github.com/devongovett/exif-reader/pull/30 ?

atombrenner commented 1 year ago

@lovell I agree, as #30 is based on this PR and typing is nearer to standard tag names.

lovell commented 1 year ago

Closing as superseded by #30

akwodkiewicz commented 1 year ago

Sorry for joining this late to the conversation. My contribution to DT was a "path of least resistance"-approach to typings. I wanted to have the package behave correctly in my side project, and I decided to share this so that nobody will be forced to recreate the very same types themselves.

I was counting on somebody to upgrade my work when they realized there's more to be done, though 😄 It's great you decided to include the types within the package.

Is there anything I should do regarding DT (e.g. delete types from there)?

lovell commented 1 year ago

@akwodkiewicz Thanks for the background, feel free to review #30 if it might be of interest to you. Once that PR is merged and published then I'm sure any help to deprecate/remove the DT-based definitions would be most welcome.