Consider drop of readonly modifiers

Question

Consider drop of readonly modifiers

Opened this issue 2 years ago · 8 comments

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.

Answer 1 · 2023-01-10T12:51:03.000Z

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?

Answer 2 · 2023-01-10T12:53:27.000Z

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

Answer 3 · 2023-01-10T13:36:17.000Z

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

Answer 4 · 2023-01-31T23:09:39.000Z

@Nodonisko does that work as intended on your end?

Answer 5 · 2023-02-03T18:33:16.000Z

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

Answer 6 · 2023-02-03T18:43:38.000Z

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?

Answer 7 · 2023-03-07T00:02:15.000Z

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!

Answer 8 · 2024-01-09T18:05:53.000Z

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