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

Add ts types #70

Merged
merged 1 commit into from
Jul 8, 2022
Merged

Add ts types #70

merged 1 commit into from
Jul 8, 2022

Conversation

bfischer1121
Copy link
Contributor

Why

Add typings for TypeScript users

How

  • Add country types to types.d.ts and reference it in package.json
  • Update README with a basic example

@stefangabos
Copy link
Owner

this looks deceivingly simple and beautiful!
but i am not familiar with all of this (i am old-school, yet to catch up with the world of react and its friends) :)
can you please make me an example on how would one use all of this as what is happening at line 41 in types.d.ts is evil magic

@bfischer1121
Copy link
Contributor Author

Ha! Well if you like technical puzzles and rabbit holes then it's an interesting path to go down...

Country describes the shape of data in data/countries/*/*
TranslatedCountry describes the data in data/countries/_combined/*

In javascript-speak, the file is doing this (where Omit and Record are built-in TS functions called generics):

function Omit(object, key) {
  delete object[key]
  return object
}

function Record(keys, value) {
  const record = {}

  keys.forEach(key => {
    record[key] = value
  })

  return record
}

export const LanguageCode = ['ar', 'bg']

export const Country = {
  id: 'number',
  alpha2: 'string',
  alpha3: 'string',
  name: 'string'
}

// { id: 'number', alpha2: 'string', alpha3: 'string', 'ar': 'string', 'bg': 'string' }
export const TranslatedCountry = { ...Omit(Country, 'name'), ...Record(LanguageCode, 'string') }

Someone using TypeScript can then validate the data they load (be it from JSON, CSV, XML, etc) like so:

import countries from 'world_countries_lists/data/countries/_combined/countries.json'
import { TranslatedCountry } from 'world_countries_lists'

countries.forEach((country: TranslatedCountry) => {
  // succeeds
  console.log(country.alpha2)
  console.log(country.ar)

  // fails in editor and during build time
  console.log(country.ca)
})

@stefangabos
Copy link
Owner

stefangabos commented Jul 7, 2022

ok, i think i got it. but

why do you need to describe the shape of data in data/countries/*/* when you are only using the combined version?
isn't import countries from 'world_countries_lists/data/countries/_combined/countries.json' enough to be able to access all data already and not having to have LanguageCode and also not needing to do any filtering on it?

also, in the pull request, you have
export const TranslatedCountry = { ...Omit(Country, 'name'), ...Record(LanguageCode, 'string') }
whereas in the example you have
export const TranslatedCountry = { ...Omit(Country, 'name') & ...Record(LanguageCode, 'string') }

@bfischer1121
Copy link
Contributor Author

Ahh yea the first example is just trying to translate the TS syntax to JS equivalent to make sense of what line 41 is doing... TS's interface & interface syntax is analogous to JS's { ...object, ...object }, where they're combining 2 entities into 1. Sorry for confusion.

For Country and LanguageCode, I tried to be liberal in what's exposed to outside devs. I'm personally using the combined file but if devs import individual language files they'll need Country. Language is useful for validating the list of languages this library supports. For example, an outside app/lib might want to validate mapping between this library's languages and their own, where if a new version adds or removes a language it lets them know to update their code with a break in the contract/build.

@stefangabos stefangabos merged commit febc4fd into stefangabos:master Jul 8, 2022
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.

2 participants