what3words / w3w-node-wrapper

Node wrapper for the What3Words API
https://docs.what3words.com/api/v3/
MIT License
44 stars 10 forks source link

Return type of convertTo3wa() #52

Closed esride-nik closed 1 year ago

esride-nik commented 2 years ago

My VS Code says that the return type of convertTo3wa()is Promise<LocationGeoJsonResponse | LocationJsonResponse>, each of which expresses one square:

export interface LocationGeoJsonResponse {
    bbox: [number, number, number, number];
    geometry: {
        coordinates: number[];
        type: string;
    };
    type: string;
    properties: LocationProperties;
}

This would make sense to me, because if I enter an exact point, it must be within exactly one square. (...or doesn't it? Are the square borders overlapping?)

Anyway, this is what comes out: image

So it looks like the return type is more like a Promise of

{
    "features": LocationGeoJsonResponse[] | LocationJsonResponse[];
    "type": "FeatureCollection";
}

Am I right? If so, would you update your TS definitions?

c5haw commented 2 years ago

Hi @esride-nik,

Yes I can see that the type definition for the geojson response is incorrect. I've opened up a ticket for us to fix this. Thanks for raising this

c5haw commented 2 years ago

Hi @esride-nik,

This should be resolved now and we have released v4.0.7 which updates the type definitions as per #53.

esride-nik commented 2 years ago

Hi @c5haw, thanks for taking care of this.

I almost agree. These are your enhanced definitions:

    convertTo3wa(options: ConvertTo3waOptions): Promise<LocationJsonResponse>;
    convertTo3wa(options: ConvertTo3waOptions & {
        format?: 'json';
    }): Promise<LocationJsonResponse>;
    convertTo3wa(options: ConvertTo3waOptions & {
        format: 'geojson';
    }): Promise<FeatureCollectionResponse<LocationGeoJsonResponse>>;

What I see is that the compiler doesn't take the right overload, even when I pass in 'geojson' as format. This is probably because the compiler chooses the first matching overload Solution: When I delete the first line (without format restriction) in the definitions, my compiler finds the right overload when passing in 'geojson'.

One minor follow-up issue: format stands in a class variable in my code, which used to be of type 'json' | 'geojson'. Now, format must be of type 'geojson'. This is ok for me because it's hard-coded anyway, but in case you'd want to make it configurable and read it out from somewhere, you'd have a string or a union type like the one mentioned above. Right?

c5haw commented 1 year ago

Hi @esride-nik,

I can see the method overloading is not taking the effect that was desired. I have now got an open PR that should resolve this and correctly type hint based on the combination of parameters provided as expected.

And yes on your second point this change should also resolve that too.

c5haw commented 1 year ago

This has been autoclosed as part of the recent PR merge, but you can reopen if you are having any other issues @esride-nik.