Add generic option type

[HansAarneLiblik]

What issue does this pull request resolve?
#704

What changes did you make?
Add generic Option through out the codebase

Is there anything that requires more attention while reviewing?

[HansAarneLiblik]

How can I build this and use it in my own project to test out that it actually works? 😆

[HansAarneLiblik]

@ericgio Can you guide me, on how can I build this (With types), so I can test it out on my own project?

[ericgio]

Hey @HansAarneLiblik you can run yarn build:types in the project root to build all the types, or just yarn build to build everything.

[ericgio]

Looking through your changes, you seem to be taking the approach that was somewhat discouraged on the issue thread, ie: propagating the option type throughout the whole codebase. While this technically correct, the concern raised on the thread is that it would add a lot of overhead to the maintenance of the project. I wonder if there's a lighter-weight approach, maybe something essentially like what DefinitelyTyped was doing where just the public interface is typed using the generic? 🤷

[HansAarneLiblik]

Sure, I'll take a look at DT, to see if that can instead be used... Don't know if types of that kind can be applied here or need to be a separate types package.

Regarding building - it didn't add types to the build folder together with dist. Need to take another look at it

[HansAarneLiblik]

Going forward with the DT way, won't that overwrite the types you've already defined here? Or will it just extend the current types?

[ericgio]

Going forward with the DT way, won't that overwrite the types you've already defined here? Or will it just extend the current types?

I think the idea would be to define a new public interface only. I'm not 100% sure on the implementation, but what I'm thinking of is something like

declare module 'react-bootstrap-typeahead' {
  export interface TypeaheadComponent<O> {
    labelKey: string | (option: O) => string;

    // Re-define public interface that relies on option type here.
  }
}

I would test this out by adding a similar declaration to a project that uses this library.