Update typings for functions returning undefined #198
Conversation
| export function getAlpha3Code(name: string, lang: string): string; | ||
| export function getSimpleAlpha3Code(name: string, lang: string): string; | ||
| export function getName(alpha2orAlpha3orNumeric: string | number, lang: string): string | undefined; | ||
| export function getNames(lang: string): LocalizedCountryNames | undefined; |
There was a problem hiding this comment.
getNames does not return undefined. It returns an empty object?
There was a problem hiding this comment.
You're totally right.
exports.getNames = function (lang) {
var d = registeredLocales[lang.toLowerCase()];
if (d === undefined) {
return {};
}
return d;
};Well, we can't return {} and object types here, as there will be a lot of cumbersome service code. Though we can return Partial<LocalizedCountryNames>, so the type of getNames('en')['ru'] will be string | string[] | undefined. The other option is lang specification: for predefined lang values return LocalizedCountryNames, for the other return Partial<LocalizedCountryNames>.
At this step, I would prefer just Partial<LocalizedCountryNames>. What do you think?
There was a problem hiding this comment.
If that makes sense for a TypeScript dev I'm fine with it :) I'm not using TypeScript so I have no opinion and follow what you suggest.
There was a problem hiding this comment.
@ibarrae what would you prefer?
There was a problem hiding this comment.
Ok, I will implement Partial<...>.
There was a problem hiding this comment.
I don't see a problem with using {}. You just need to check if a key is present, for instance
const names = getNames("en");
if ("US" in names) {
// you can use it here
}
|
@xamgore I think we can merge this. Would you be able to resolve the conflicts? |
|
Yes, no problems. |
|
@xamgore one additional question: Do you have any experience with testing those type definition files? We had an issue last week where the file was "broken" and we did not notice because of the lack of a test. Let me know if you have any ideas in this area. |
|
There are at least two tools. The first one checks that some code in typescript passes the type check. The other allows to write assertions about types itself, like, assert the returning type of a function is |
|
@xamgore I see. If you don't mind adding any test that is better than no test I would be very happy :) |
|
I will make a new PR with some refined versions of these changes as the base; I've also written typescript typings test files before so I'll do one of those. |
|
That said i think that a test won't really help whether or not it's "correct", since the code itself isn't written in typescript the test can be just as buggy as the typing. These types aren't complex so I think a test doesn't get us much value. |
PR updates typings, so that they are typesafe: if garbage is passed in,
undefinedis returned. And a programmer will have to handle that situation somehow.