Skip to content

Added basic TypeScript typings#23

Merged
eshaz merged 7 commits into
eshaz:masterfrom
JohnWeisz:master
Jun 25, 2022
Merged

Added basic TypeScript typings#23
eshaz merged 7 commits into
eshaz:masterfrom
JohnWeisz:master

Conversation

@JohnWeisz
Copy link
Copy Markdown
Contributor

@JohnWeisz JohnWeisz commented Jun 18, 2022

This PR adds index.d.ts containing typings, but without descriptions (as of now). This allows codec-parser to be used in TypeScript projects nicely.

There are no changes to runtime code, these are merely declarations for IntelliSense, etc. to work better.

@CharlesSchimmel
Copy link
Copy Markdown

Could you also update format in package.json to include formatting TypeScript files, and then run npm format and push the changes? Just add --write **/*.ts to this line.

@CharlesSchimmel
Copy link
Copy Markdown

This is so helpful, thanks for doing this!

Comment thread index.d.ts Outdated

declare interface ICodecParserOptions
{
onCodec?: () => any;
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this can be narrowed to (string: codec) => {}. @eshaz onCodec is called with a string that is the name of the codec being used, and is expected to be an arbitrary action?

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it a union could be used here that is dependent on the passed in mimetype on instantiation.

  • When the mimetype is like aac or mpeg, then onCodec will be called with aac and mpeg respectively.
  • When the mimetype is like ogg, then onCodec may be called with one of flac, opus, or vorbis depending on the detected codec.

See: https://github.com/eshaz/codec-parser/search?q=onCodec and https://github.com/eshaz/codec-parser#properties

This is something I can add myself later too.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For now I added an enum to constrain codec to the possible codec strings.

Comment thread index.d.ts Outdated
@@ -0,0 +1,165 @@
declare module "codec-parser"
{
declare type OggMimeType = "application/ogg" | "audio/ogg";
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could these also be broken out into individual type aliases for each string?

Comment thread index.d.ts Outdated
Comment thread package.json Outdated
@eshaz
Copy link
Copy Markdown
Owner

eshaz commented Jun 18, 2022

Thanks @JohnWeisz for your contribution! I've been meaning to add types but you beat me to it :) Feel free to make updates based on the comments here if you want. Otherwise, I'll merge this into a branch and take care of the modifications.

@JohnWeisz
Copy link
Copy Markdown
Contributor Author

JohnWeisz commented Jun 19, 2022

Thanks, I pushed a few more changes, in particular to get formatting done and flesh out the onCodec callbacks.

Also I realized that, because of the type: "module" in package.json, we need to make the type declaration itself a module, so I also converted that to top-level module exports.

Note: I ran prettier locally and it essentially affected every .js file in the repo. I didn't push those.

@eshaz eshaz merged commit d85218c into eshaz:master Jun 25, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants