Skip to content

Conversation

@mfedderly
Copy link
Collaborator

No description provided.

path.join(__dirname, "test", "fixtures", "route2.geojson")
);

const options = { units: "miles" } as const;
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I think this had been broken when we changed the options to an object

Copy link
Member

Choose a reason for hiding this comment

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

Huh. So why wasn't the isObject call catching this? Or was bench just not being run by anyone?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yeah I think it was just broken. There's no actual testing making sure bench works. I don't think we really want to just burn CI time benchmarking though.

path.join(__dirname, "test", "fixtures", "route2.geojson")
);

const options = { units: "miles" } as const;
Copy link
Member

Choose a reason for hiding this comment

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

Huh. So why wasn't the isObject call catching this? Or was bench just not being run by anyone?

): Feature<LineString> {
// Optional parameters
options = options || {};
if (!isObject(options)) throw new Error("options is invalid");
Copy link
Member

Choose a reason for hiding this comment

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

Reckon we should set default units explicitly even if it effectively happens a lower level. Maybe we destructure similar to #2974?

const { units = "kilometers" } = options;

That'd also make sure we're not passing any rando options further down to distance or destination.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Good point about accidentally sending extra options down.


var coords;
var slice = [];
var coords: Position[];
Copy link
Member

Choose a reason for hiding this comment

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

Another couple of vars we could jettison.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Lets just apply eslint's no-var and prefer-const (in another PR)

@mfedderly mfedderly merged commit f111984 into master Dec 18, 2025
4 checks passed
@mfedderly mfedderly deleted the mf/line-slice-along-ts branch December 18, 2025 19:59
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