Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Explain why Readonly array is not good idea #987

Closed
steida opened this issue Oct 26, 2019 · 7 comments
Closed

Explain why Readonly array is not good idea #987

steida opened this issue Oct 26, 2019 · 7 comments

Comments

@steida
Copy link
Contributor

steida commented Oct 26, 2019

📖 Documentation

Readonly arrays are great to prevent accidental mutations via sort method, but PITA when used with code requiring normal Array. Long story short, it's: Use it everywhere, nowhere, or cast all the time.

I didn't find any other frictionless solution from the DX perspective than not using readonly arrays at all.

I believe it should be mentioned in the documentation.

@raveclassic
Copy link
Collaborator

It's ok to accept ReadonlyArray or even ArrayLike as an argument but things get worse when using them as a return type.

I think the best solution so far is to accept ArrayLike and return Array if a function constructs a new Array.

Note: we have the same problem in most mostjs/core#296 (comment)

@steida steida closed this as completed Nov 14, 2019
@steida steida reopened this Dec 17, 2019
@steida
Copy link
Contributor Author

steida commented Dec 17, 2019

@gcanti ReadonlyArray really should be banned in fp-ts ecosystem. It's only pain without any measurable benefit. I just spend another hour with export interface Context extends ReadonlyArray<ContextEntry> { trying to figure out why I can not use fp-ts Array. Not I have to cast (ugly) or call slice(0) (ugly too).

@gcanti
Copy link
Owner

gcanti commented Dec 18, 2019

ReadonlyArray really should be banned in fp-ts ecosystem

In practice it already is (except for io-ts's Context, for legacy reasons)

@andrei-zhidkov
Copy link

andrei-zhidkov commented Jan 10, 2020

Sorry, but I don't get why the use of ReadonlyArray (a built-in type) should be discouraged in the fp-ts docs.
Besides, I'm not sure I understand the reason to artificially limit the ReadonlyArray usage, given that the functions in Array.ts are pure.

ReadonlyArrays are great, and I'd be glad to submit a PR to support them more (either with overloads to return ReadonlyArrays when operating on ReadonlyArrays or using ArrayLike and returning Arrays as @raveclassic suggested) if that's ok with the maintainers.

@steida
Copy link
Contributor Author

steida commented Jan 10, 2020

"ReadonlyArrays are great" if everything is read-only array otherwise poor developers have to cast them all the time. If fp-ts people are happy with it casting all the time or not using mutable arrays at all, then ok.

@SRachamim
Copy link
Contributor

SRachamim commented Jun 3, 2020

In our projects we're enforcing using readonly and ReadonlyArray with functional linting rules.

@gcanti
Copy link
Owner

gcanti commented Jun 3, 2020

Actually I'm considering to drop support for mutable modules in v3, #1226

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants