Skip to content

Improve NearestPoint type for @turf/nearest-point so that it keeps the properties of the points in the FeatureCollection#2861

Closed
EmilJunker wants to merge 3 commits intoTurfjs:masterfrom
EmilJunker:patch-1
Closed

Improve NearestPoint type for @turf/nearest-point so that it keeps the properties of the points in the FeatureCollection#2861
EmilJunker wants to merge 3 commits intoTurfjs:masterfrom
EmilJunker:patch-1

Conversation

@EmilJunker
Copy link
Copy Markdown
Contributor

@EmilJunker EmilJunker commented Mar 19, 2025

Please provide the following when creating a PR:

This PR improves the NearestPoint type and the type definition for the nearestPoint function.

Consider the following example in TypeScript:

const coordinatesList = [[0, 0], [1, 1], [0, 1]];
const pointsList = coordinatesList.map((coordinate, i) => {
    return turf.point(coordinate, {
        name: `point${i}`
    });
});
const pointsCollection = turf.featureCollection(pointsList);
const myNearestPoint = turf.nearestPoint(turf.point([2, 2]), pointsCollection);

With the old behaviour myNearestPoint.properties.name is of type any according to TypeScript.

With the changes in this PR, the type of myNearestPoint.properties.name is correctly inferred as string.

This change should be non-breaking.

twelch
twelch requested changes Apr 3, 2025
View reviewed changes
Copy link
Copy Markdown
Collaborator

@twelch twelch left a comment

Choose a reason for hiding this comment

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

Thanks for the PR @EmilJunker! Your addition looks really close. I see a couple of errors if you run pnpm install at the top-level of your local repo.

index.ts(7,28): error TS2304: Cannot find name 'GeoJsonProperties'.

It looks like if you add an import at the top
import { Feature, FeatureCollection, Point, GeoJsonProperties } from "geojson";

and additionally extend P as well as assigning a default value:
function nearestPoint<P extends GeoJsonProperties = GeoJsonProperties>(

Then you should be in business. Could you make these changes and ping back for another check?

Be sure to run both pnpm install and pnpm test at the top level to make sure everything is passing per the guidelines

@EmilJunker EmilJunker closed this Apr 7, 2025
@EmilJunker EmilJunker deleted the patch-1 branch April 7, 2025 05:47
@EmilJunker EmilJunker restored the patch-1 branch April 7, 2025 05:48
@EmilJunker EmilJunker mentioned this pull request Apr 7, 2025
Merged
5 tasks
@EmilJunker
Copy link
Copy Markdown
Contributor Author

EmilJunker commented Apr 7, 2025

@twelch Thank you for your patience and guidance. I have opened a new pull request with the changes you requested: #2866

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@twelch twelch twelch requested changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@EmilJunker @twelch