Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ft: add typescript #27

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

shyakadavis
Copy link

@shyakadavis shyakadavis commented Oct 17, 2023

What does this PR do?

e.g.

Before, to access districts, it was:

const { Districts } = require('rwanda');

console.log(Districts('kigali')); // [ 'Gasabo', 'Kicukiro', 'Nyarugenge' ]
console.log(Districts('Kigali', 'South')); // [ 'Gasabo', 'Kicukiro', 'Nyarugenge', 'Huye',.... ]

But now, to ensure consistency and type-safety at least, you pass a single object param, whose properties are required, in order to maintain/retain familiarity with the package.

import { Districts } from "rwanda";

const districts = Districts({ provinces: "Kigali" }); // [ 'Gasabo', 'Kicukiro', 'Nyarugenge' ]
const districts = Districts({ provinces: ["Kigali", "North"] }); // [ 'Gasabo', 'Kicukiro', 'Nyarugenge', 'Huye',.... ]

Description of Task to be completed?

  • This will:
    • set up a pnpm workspace monorepo with:
      • an apps dir for the docs site, and
      • a packages dir which will host the actual rwanda package.
  • Versioning will be automated with the help of the changesets package
  • Testing will be made easier with Vitest
  • This will also drastically simply linting and formatting with the near-zero config toolchain, biome
  • Introduce pre-commit hooks for formatting and testing with husky
  • Re-write all internal methods.
    • Provinces
    • Districts
    • Sectors
    • Cells
    • Villages
  • Re-write all data in TS
    • East
    • Kigali
    • North
    • West
    • South
  • Write all tests

How should this be manually tested?

  • pnpm test in the root dir.
  • If you wish to test the package in a real app locally, navigate into the apps/www dir, hit pnpm i, and hit pnpm dev. You'll notice a bunch of type errors in the src/routes/+page.svelte, which are a demonstration of type violations. Correct them, and in localhost:5173, you should see the correctly rendered data.

Any background context you want to provide?

  • PNPM and Vite seemed the logical backbones for this. They have excellent D/X, imo.
  • Would have liked to make all param properties optional, i.e., one could just access the villages of a district or villages of a province, without having to pass all other params, only what you need, but I guess that would be considered in another release.
  • Since this P.R has gotten too broad, I plan to open another for the docs creation. That is, if the current one is accepted.
  • I didn't know the effect typing vars that hold tens-of-thousands of values would have on performance and/or bundle size, so I just left typing for provinces, districts, and sectors

Screenshots (if appropriate)

  • N/A

@shyakadavis shyakadavis marked this pull request as ready for review October 19, 2023 20:06
update workspace manifest file name

add vitest & biome

ch: use package inside the app
- add sveltekit app to test
- add vite-plugin-dts
- currently read package from the same namespace

add pre-commit hooks and test/lint/format scripts

update package meta data

ch: add changeset

ch: update workflows

add base types & data for Nyarugunga

ft: add `Districts` method

ft: add `Sectors` method
- add tests for provinces & districts

improve the `Sectors` method, and more tests

add `Cells` method and tests

ft: add `Villages` method & tests

ch: add Kicukiro sectors

add `Gasabo` data & export all sectors from `Kicukiro`

ch: add `Nyarugenge` sectors

complete `Kigali` province (districts & sectors)

temporarily disable district tests (until data is complete)

ch: add `East` province data (districts & sectors)

ch: add `North` province data (districts & sectors)

add `South` province
- districts & sectors
- remove exports of cells in `north` sectors

update ci workflow
- lint here and there also

wrap up init data transfer (ahwi 😮‍💨)

lint ✨

❓
- updated tests
- updates to the methods (making all, save the province, optional params)
- added a missing village

🧹
- improve methods (a tad bit simpler)
- check for some edge cases in tests
@shyakadavis shyakadavis force-pushed the feature/ts-support-and-new-docs branch from 8f91c09 to 3b31e88 Compare October 21, 2023 16:23
@shyakadavis shyakadavis changed the title ft: add typecript & new docs ft: add typecript May 28, 2024
@shyakadavis shyakadavis changed the title ft: add typecript ft: add typescript May 28, 2024
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.

Add TypeScript Type Definitions
1 participant