fetchCurrencies() #4
Conversation
There was a problem hiding this comment.
Kindly also remove the package-lock.json. we don't need it as we have yarn-lock.json
You should update readme with how to use the piece of code you added
Rem to do a commit to link this to the open issue, or tag it manually
In future, remember to request for reviews for PRs to be seen in time
| @@ -56,6 +57,28 @@ function getCities(filter) { | |||
| ) | |||
| } | |||
|
|
|||
| async function fetchCurrencies() { | |||
There was a problem hiding this comment.
the name should be descriptive to what the function does , getCountriesWithCurecy
| ); | ||
| countryCurrencies[country] = currencyData.currencies; | ||
| } catch (error) { | ||
| console.error(`Failed to fetch currency for ${country}:`, error); |
There was a problem hiding this comment.
I would rather suggest we return an empty currency field, rather than throw an error, this is to have consistent currency type|I'll prefer string | null to string | undefined
| @@ -1,4 +1,5 @@ | |||
| const countriesData = require("./country-cities.json") | |||
| const CurrencyExplorer = require('currency-explorer'); | |||
There was a problem hiding this comment.
this constant should be camel case
also we only need const { currencies } = require('currency-explorer');
also kindly lazy load this inside fetchCurrencies as it's only used there
|
@cliffgor could we also get the ability to get currency for a particular country? |
@cliffgor Added an async function to fetch currencies from the CurrenExplorer package and attach it to each country