haimkastner / unitsnet-js

A better way to hold unit variables and easily convert to the destination unit
https://www.npmjs.com/package/unitsnet-js
MIT License
23 stars 8 forks source link

Equally use singular for unit types as in the c# project #29

Closed Shentoza closed 6 months ago

Shentoza commented 6 months ago

We're using the unitsnet-js as a way to have the same information in our frontend, that we also have in our backend. However in this package all unitnames are in plural (e.g. Meters, Millimeters, Inches ) while in the c# project they are singular (e.g. Meter, Millimeter, Inch ) which makes conversion between our c# backend and our typescript frontend really painful. Is this a conscious decision?

haimkastner commented 6 months ago

Hi @Shentoza

Are you referring to the unit's enum? LengthUnits in the length unit case?

This enum convention is not the same as in the C#, and I do agree that it was better to be the same convention, but why is this so painful?

Can you please share a use case for why it's so matter?

I can think of a use case, when you want to pass the unit as part of an API response, for instance:

{
 "value": 10,
 "unit": "Meters"
}

Then in the consumer, you want to do the following:

const res = api_call()
const length = new Length(res.value, res.unit)

But, if this is the case, it doesn't matter what the enum key name is, but what the actual value is.

BTW, currently, the enums value of the units are random numbers (depending on the order, auto set number)

Shentoza commented 6 months ago

Because the object coming from my Backend is

{ 
    value: 10,
    unit: "Meter"
}

I'm using the enum name here to have a more speaking response in case anyone not using UnitsNet is calling my API, where the enum value of e.g. 12 would be confusing. Now i have to do a conversion where I map the singular response of the backend to the plural naming of the typescript enum, s.t. I can create an Length object, (since there is only an constructor(value: number, fromUnit?: LengthUnits); constructor I need the actual enum value. And if I want to send the object back to the backend, I need to convert it back again to the singular form.

I have seen that in the generator files ( unit-generator.ts ) in buildEnum it is specified to take the plural name, which should be easy to alter, no?

Shentoza commented 6 months ago

For now I defined a lookup table that converts the units to their singular equivalent

const enumToSingular = {
  [LengthUnits.Meters]: 'Meter',
  [LengthUnits.Miles]: 'Mile',
  [LengthUnits.Yards]: 'Yard',
[...]

which works but is more of a workaround

haimkastner commented 6 months ago

I understand, so basically if I will change the value of the enum from some random number to the unit non-plural name

Like this one https://github.com/haimkastner/unitsnet-js/blob/master/src/length.g.ts#L4

Meters = 'Meter',

It will work for you even if the enum key is still plural, like this:

const length = new Length(res.value, res.unit as LengthUnits)

I have seen that in the generator files ( unit-generator.ts ) in buildEnum it is specified to take the plural name, which should be easy to alter, no? Right, it's easy to change it, but it will break the package API (unless I will duplicate it and deprecate the old unit enumas I explain above).

Shentoza commented 6 months ago

I understand, so basically if I will change the value of the enum from some random number to the unit non-plural name

Like this one https://github.com/haimkastner/unitsnet-js/blob/master/src/length.g.ts#L4

Meters = 'Meter',

It will work for you even if the enum key is still plural, like this:

const length = new Length(res.value, res.unit as LengthUnits)

That would be great!

haimkastner commented 6 months ago

@Shentoza The new version 2.0.1 contains the fix from #30 published and ready in NPM.

haimkastner commented 6 months ago

@Shentoza Regarding your use-case WDYT about #31?

Shentoza commented 6 months ago

@Shentoza The new version 2.0.1 contains the fix from #30 published and ready in NPM.

Thank you so much. We're using yarn, is the package already pushed there as well? im taking a look into #31

haimkastner commented 6 months ago

@Shentoza Yes, AFAIK yarn is doing uplink to npm.

Shentoza commented 6 months ago

Yes, sorry I have to upgrade --latest since it's a new major version, which I forgot