Object.freeze overloads force the type to be recognized as an array (by the order of overloads)

[aghArdeshir]

lib Update Request

Configuration Check

My compilation target is es5 and my lib is ["dom", "dom.iterable", "esnext"].
(Which I believe does not matter in this context as the definition of freeze is inside lib.es5.d.ts solely.)

Missing / Incorrect Definition

Object.freeze

It is defined in a way that in IDEs it is always supposed the input is an array.

Sample Code

Object.freeze<{someKey: 'someValue'}>({/* the problem is with autocomplete here (in IDEs) */});

Documentation Link

https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Object/freeze

---

I believe the overload that forces arrays is defined in this PR:
https://github.com/microsoft/TypeScript/pull/12434/files

And I believe just removing it will suffice (and the user of Object.freeze needs to specify its an array in case its an array)

---

If it is confirmed, I would be happy to create the PR for it.

[Josh-Cena]

That seems to be an unusual development workflow though. Most of the time you only need to specify Object.freeze({ someKey: "someValue" }) without duplicating the type argument.

[aghArdeshir]

@Josh-Cena I have a SocialNetwork type:

type SocialNetwork = { name: string ; url: string; }

I need this type to be a kinda centric type and reusable. So in different places I can have different functions that produce this structure:

function getFacebook() {
  return Object.freeze<SocialNetwork>({});
}

when I write the function above, I expect my IDE to help me completing the object with suggesting required keywords. And later:

function getFacebook() {
  return Object.freeze<SocialNetwork>({name: 'Facebook', url: 'https://facebook.com/my-page' });
}

when I already have function above, I want any change in future to the actual SocialNetwork type, trigger errors all over the place, so I can easily refactor my code (e.g. change url -> URL) or add new keys and immediately know where to change (e.g. add a new key: displayName: 'My Page')

[Josh-Cena]

I see. That makes sense.

[MartinJohns]

When you use an explicit type annotation you get much better support:

function getFacebookExplicit(): Readonly<SocialNetwork> {
  return Object.freeze({  });
}

function getFacebookInferred() {
  return Object.freeze<SocialNetwork>({ });
}

instead of

instead of

[aghArdeshir]

@MartinJohns thanks 👍 That's actually very good. I agree with most parts.

Actually note that:

In the above screenshot you attached, the url is not autocompleted, IDE cannot suggest keys of the objects you pass into Object.freeze:

Also, from clean-code point of view and architecture point-of-view what you say is basically right, but the getFacebook is just an example.

The reason I opened this issue is to solve more complicated problems, e.g.:

type SocialNetwork = {
  url: string;
};

type CommonProfileHandle = {
  handle: string;
};

function getSocial(): SocialNetwork | CommonProfileHandle | null {
  if (someConditionIsCorrect) {
    return Object.freeze({
      /* here it is not deterministic */
    });
  } else if (someOtherConditionIsCorrect) {
    return Object.freeze({
      /* here it is not deterministic */
    });
  }

  return null;
}

Update
The thing is that Inferred types and Generic types are there for a reason and sometimes they needs to be used. So this issue is about those cases that people find themselves needing to use Generic Type on Object.freeze.

Update
Another thing is that explicit type annotation does not support automatic refactor (url -> URL)

[andrewbranch]

I think it probably makes sense to move overloads with no constraints on the type parameter after overloads that do have constraints. But even then, you can’t disambiguate a call with explicit type arguments between freeze<T>(o: T): Readonly<T> and freeze<T>(a: T[]): readonly T[]. This is a general limitation of overloads.

Accepting a PR so we can try this out and run extended tests to see if real-world usage breaks. No guarantee it will be mergeable.

[aghArdeshir]

Thanks @andrewbranch . Great news. I will try to see if I can understand this whole TypeScript thing and make a change.

(this does not mean I am reserving this issue to work on this)

[andrewbranch]

And I believe just removing it will suffice (and the user of Object.freeze needs to specify its an array in case its an array)

I missed this part of the suggestion by the way, but unless I’m missing something, I think you’re right. The array-to-readonly-array overload seems unnecessary, as it’s covered by freeze<T>(o: T): Readonly<T>.

[nicolas377]

Running some playground tests shows that real world usage shouldn't break. I'll open a PR so the TS team can test further.