feat(geoutils): tree-shakeable getAdministrativeRegions function#130
feat(geoutils): tree-shakeable getAdministrativeRegions function#130
Conversation
|
There was a problem hiding this comment.
Pull request overview
This PR extracts getAdministrativeRegions into its own module to improve tree-shaking and updates exports/tests accordingly.
Changes:
- Added standalone
src/getAdministrativeRegions.ts(and tests) with Mount Athos filtering + level selection. - Updated
src/index.tsto re-exportgetAdministrativeRegionsfrom the new module. - Refactored
src/geoUtils.tsto consume the extracted function and removed the inlined implementation.
Reviewed changes
Copilot reviewed 6 out of 8 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| src/types.ts | Introduces shared geo options/types and a Locale alias used by exported option types. |
| src/index.ts | Re-exports getAdministrativeRegions from its own entry point for better tree-shaking. |
| src/getAdministrativeRegions.ts | New extracted implementation + data wiring. |
| src/getAdministrativeRegions.test.ts | New focused unit tests for getAdministrativeRegions. |
| src/geoUtils.ts | Removes embedded regions data/implementation; delegates to the extracted module. |
| src/data/administrative-regions-en.json | Adds/updates the English administrative regions dataset under src/ for module import. |
| src/tests/geoUtils.test.ts | Removes getAdministrativeRegions tests and updates admin-regions JSON import paths. |
You can also share your feedback on Copilot code review. Take the survey.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 6 out of 8 changed files in this pull request and generated 2 comments.
Comments suppressed due to low confidence (1)
src/geoUtils.ts:17
- Now that
AdministrativeRegionsOptionsis exported fromsrc/types.tsandgeoUtils.tsimportsgetAdministrativeRegions,geoUtils.tsstill defines its own localLocale/AdministrativeRegionsOptionstypes. This duplication can drift; consider importing and reusing the shared types instead of redefining them locally.
import { getAdministrativeRegions } from "./getAdministrativeRegions";
import type {
City,
Country,
GeographicRegion,
You can also share your feedback on Copilot code review. Take the survey.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 6 out of 8 changed files in this pull request and generated 4 comments.
You can also share your feedback on Copilot code review. Take the survey.
|
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 6 out of 8 changed files in this pull request and generated 4 comments.
You can also share your feedback on Copilot code review. Take the survey.
| // Geo utilities | ||
| export { getAdministrativeRegions } from "./getAdministrativeRegions"; | ||
| export { |
| export const MOUNT_ATHOS_REGION_ID = 14; | ||
|
|
||
| const administrativeRegions = { | ||
| el: administrativeRegionsEl, | ||
| en: administrativeRegionsEn, | ||
| } as const; | ||
|
|
||
| const administrativeRegionsWithoutMountAthos = { | ||
| el: administrativeRegions.el.filter(({ id }) => id !== MOUNT_ATHOS_REGION_ID), | ||
| en: administrativeRegions.en.filter(({ id }) => id !== MOUNT_ATHOS_REGION_ID), | ||
| }; |
| type Locale = "el" | "en"; | ||
|
|
||
| type AdministrativeRegionsOptions = { | ||
| locale?: Locale; | ||
| includeMountAthos?: boolean; | ||
| level?: "region" | "unit" | "municipality"; | ||
| }; |
| it("returns same reference for municipality level (performance optimization)", () => { | ||
| const result1 = getAdministrativeRegions(); | ||
| const result2 = getAdministrativeRegions(); | ||
|
|
||
| // Municipality level returns same reference for performance (documented as read-only) | ||
| expect(result1).toBe(result2); |
feat(geoutils): tree-shakeable getAdministrativeRegions function.