Open
Conversation
Owner
crhallberg
commented
Feb 4, 2023
- BREAKING: REMOVES MULTI RETURN.
There was a problem hiding this comment.
Pull Request Overview
This PR replaces the existing zip code lookup by location with a new kd‐tree implementation. Key changes include:
- Introducing a new kd‐tree implementation and updating the corresponding update and index modules.
- Removing the old multi-return functionality and updating tests accordingly.
- Bumping the package version and adding an esbuild configuration.
Reviewed Changes
Copilot reviewed 9 out of 9 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| update.mjs | New implementation using the kd‐tree for location searches. |
| update.js | Removal of the old zoned index approach. |
| test/by-location.js | Skipping the multi-return test in line with breaking changes. |
| src/kd-tree.mjs | New kd‐tree class implementation with private methods. |
| src/index.mjs | Updated lookup functions to use kd‐tree for location queries. |
| package.json | Version bump and script updates for new implementation. |
| esbuild.config.mjs | New configuration script for building the project. |
Comments suppressed due to low confidence (3)
test/by-location.js:15
- Since the multi-return functionality has been removed per the breaking changes, consider removing this skipped test altogether to avoid confusion.
it.skip('return multiple closest when asked', function() {
src/kd-tree.mjs:7
- The use of private class fields (e.g., '#leaf') requires language support beyond ES2018; consider updating the target environment in esbuild.config.mjs or adding appropriate transpilation support.
#leaf(item) {
src/index.mjs:41
- [nitpick] For consistency with the parsed floating-point values in the kd‐tree inserts, consider using parseFloat (or Number) for validating latitude and longitude in getByLocation.
if (typeof lat === "undefined" || isNaN(parseInt(lat, 10)) || typeof long === "undefined" || isNaN(parseInt(long, 10))) {
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.