mobily / ts-belt

🔧 Fast, modern, and practical utility library for FP in TypeScript.
https://mobily.github.io/ts-belt
MIT License
1.1k stars 30 forks source link

Consider drop of readonly modifiers #65

Open Nodonisko opened 1 year ago

Nodonisko commented 1 year ago

readonly is constant source of pain, because everytime where code from ts-belt come to contact with normal JS code it throws type error. It happens us a lot because we are using it in legacy codebase a even if you do thing like simple A.map you need always wrap result in F.toMutable.

If I decide to refactor our code to work with it, I need to place readonly literally everywhere. That will break TS in our tests because of our fixtures which are mutable. Same problem is that everywhere where code come to touch with 3rd party libraries I will throw some errors about readonly again.

And last and probably strongest argument is this nice thread on Twitter with some great examples why it's even not that safe >>> https://twitter.com/MichaelArnaldi/status/1601535981949456385

I know this is will produce big change (but shouldn't be breaking) and final resolution is on you @mobily but please consider dropping it for v4.

mobily commented 1 year ago

hello @Nodonisko 👋 thanks for the suggestion, and I will definitely consider it, in the meantime, have you tried this https://mobily.github.io/ts-belt/docs/getting-started/config#immutable-vs-mutable?

Nodonisko commented 1 year ago

@mobily Cool I didn't know about that config! I will try it :)

mobily commented 1 year ago

and the moment it supports arrays only, but I think adding support for objects doesn't require much effort

mobily commented 1 year ago

@Nodonisko does that work as intended on your end?

Nodonisko commented 1 year ago

@mobily It's better, but as you mentioned without object support there are still places where it throws a lot.

Nodonisko commented 1 year ago

I think I can help you with this one, but I have troubles to understand how to contribute. For example for Array, there are two files:

/Array/Array.ts
/Array/index.ts

In index.ts there seems to be type definitions for all functions from Array.res but there is also file Array.ts where is only small subset of type definitions for these functions. So which files I can change?

lieberkind commented 1 year ago

and the moment it supports arrays only, but I think adding support for objects doesn't require much effort

@mobily Adding a mutability option for objects would be a great help! At my company we're also trying to introduce ts-belt for into a large codebase and are getting hit by having to run F.toMutable a lot.

Thanks for the great work!

adrian-gierakowski commented 8 months ago

to add my two cents: it's best if function arguments are immutable, but return values not. This allows for seamless integration with libraries which are imprecise in their types and use mutable types in function arguments, event though they do not mutate them