appirio-tech / ng-iso-constants

MIT License
0 stars 2 forks source link

Expose the country code list for non-angular applications #13

Open maxceem opened 4 years ago

maxceem commented 4 years ago

Initially, we've been using this country code for an AngularJS application https://github.com/appirio-tech/accounts-app/blob/dev/package.json#L22.

But later, we need the same list of countries and codes to be used inside non-angular applications so we had to manually create a copy of the same country list:

It would be better if we could reuse the same list of countries and codes in these non-angular apps. So we can expose the list of countries as a simple array for non-angular apps from this package.

vikasrohit commented 4 years ago

@maxceem do you think using lookup-api makes sense to centralize these?

maxceem commented 4 years ago

@vikasrohit I think the list of countries with codes is static and almost never changes, so it would be better to store it in some static file to avoid extra API requests and keep the code easier.

vikasrohit commented 4 years ago

Yep, agreed. However, later I thought, can we treat it as starting of using lookup api for connect and related services? May be for now we can start with countries list but may be able to move more constants/lookup to this api? Its just a thought right now, no need to act on it.

maxceem commented 4 years ago

Hmm, I can see that lookup API supports countries and it would be nice to keep it in sinc with other Topcoder applications. Though on the other hand if we use API instead of a static file it would make all the related code much more complicated as we would have to take care of the loading process:

vikasrohit commented 4 years ago

Yes, agreed. However, we can avoid last point by making the react components dumb, I mean they can get the list of countries from props which I think we are doing in some of the components.

Anyway, lets not go with look up api for now. But at least we need to do followings:

  1. Fix all countries in ng-iso-constants
  2. Fix any component in react-components to use countries from props only i.e. no local copy of country list in react-component
  3. Fix any component in connect-app to use countries from ng-iso-constants only i.e. no local copy in connect-app

Further, for now keep referencing the ng-iso-constants wihout npm module i.e. keep referencing it using github link to the release tag.