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
| ) | ||
| } | ||
|
|
||
| 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? |
2 participants
@cliffgor Added an async function to fetch currencies from the CurrenExplorer package and attach it to each country