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

Freeze empty from Array #979

Closed
sutarmin opened this issue Oct 20, 2019 · 2 comments
Closed

Freeze empty from Array #979

sutarmin opened this issue Oct 20, 2019 · 2 comments
Assignees

Comments

@sutarmin
Copy link

🚀 Feature request

Hey! I want to suggest freezing empty to avoid occasional mutation (e.g. by third-parties).

Current Behavior

import { empty } from 'fp-ts/lib/Array';

function foo(arr: Array<number>) {
    arr.push(1); // might be not that obvious
}

foo(empty); // Neither compile-time nor runtime error
// All the code that depends on `empty` is broken after that

Desired Behavior

import { empty } from 'fp-ts/lib/Array';

function foo(arr: Array<number>) {
    arr.push(1);
}

foo(empty); // No compile-time error, but runtime exception

Suggested Solution

const empty: Array<never> = Object.freeze([]);

Who does this impact? Who is this for?

My team several times faced subtle bugs caused by empty been mutated occasionally by third-party code. I think that confidence that empty won't be mutated worth possible runtime exception at development time.

Describe alternatives you've considered

  1. Adding readonly to empty's type - inconveniently restrict possible usages of empty cause you can't pass readonly Array<T> where you expect Array<T>
  2. Object.seal - less strict than Object.freeze, but basically is the same for our case
  3. Doing Object.freeze(empty) in every project - easy to forget about

Your environment

Software Version(s)
fp-ts v2.1.0
TypeScript v3.6.3
@raveclassic
Copy link
Collaborator

2 cents here - if we disable mutations on Array then return type of empty should be ReadonlyArray. Otherwise it's a broken contract.

@raveclassic
Copy link
Collaborator

related #544

gcanti added a commit that referenced this issue Jan 11, 2020
@gcanti gcanti self-assigned this Jan 11, 2020
@gcanti gcanti closed this as completed in 8c6a60b Jan 12, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants