Repository navigation
A list of suggested update to TypeScript's lib #49773
Description
lib Update Request
While working on uhyo/better-typescript-lib, I've found couples of issues in the built-in lib. While reflecting some of the changes there will cause compatibility issues and they are not included in the list below, others are worth reporting because either they are serious or they are not breaking changes. Here is the list:
es2015.collection.d.ts
-
WeakMapConstructor: The type ofentriesshould change fromreadonly [K, V][] | nulltoreadonly (readonly [K, V])[] | nullto keep it consistent withMapConstructor
es2015.core.d.ts
-
{Array<T>, ReadonlyArray<T>}.find:- The
thisparameter of the first parameter in the first overload is incorrectly typedvoid, which should be removed - Missing documentation for the second overload
- The
-
ArrayConstructor.from: The first parameter should be renamedsourceto keep consistency across multiple overloads from other lib files. Ines2015.iterable.d.tsit's writteniterable: Iterable<T> | ArrayLike<T>and that's misleading. It's also namedsourcein the spec text. (The documentation should be modify toan array-like or iterable objecttoo to keep it the same across all files) -
(Excluded unless Suggestion: one-sided or fine-grained type guards #15048 resolves)NumberConstructor.{isFinite, isInteger, isNaN, isSafeInteger}: Per the spec, the return type can be changed to the type predicatenumber is number -
ObjectConstructor.assign: All parameters but the first should be made writable ({ -readonly [P in keyof T]: T[P] }). -
ObjectConstructor.{getOwnPropertySymbols, setPrototypeOf}: 🟠 The first parameter should extends{}. See the es2017.object.d.ts section below -
String.normalize: 🟠 Per the spec, the second overload should be removed to prevent a runtime error. The first parameter should then be made optional because it's defaulted to"NFC".
es2015.iterable.d.ts
-
{ArrayConstructor, %TypedArray%Constructor}.from: See the es2015.core.d.ts section above
es2015.reflect.d.ts
Crossed out unless uhyo (@uhyo) thinks them worth changing and gives some illustrations.
-
Reflect.construct:newTargetcan be widen toany -
Reflect.getPrototypeOf: The return type can be narrowed toobject. It's impractical to get anullhere -
Reflect.setPrototypeOf:protocan be widen toany
es2015.symbol.wellknown.d.ts
-
(Fix already available in Fix definition ofArray<T>[Symbol.unscopables]: Should be a record.Array.prototype[Symbol.unscopables]#42566)
es2017.object.d.ts
-
Object.getOwnPropertyDescriptors: 🟠 Per the spec, these methods start with either aToObjectorRequireObjectCoercibleoperation, which throws aTypeErrorwith a nullish value. Starting from TypeScript 4.8 with Improve intersection reduction and CFA for truthy, equality, and typeof checks #49119,{}now precisely represents any nullish value. Thus, the first parameter of these methods should now be typed{}to prevent runtime errors.
es2019.array.d.ts
-
{Array<T>, ReadonlyArray<T>}.flatMap: TheThistype parameter should be removed to keep consistency with other array prototype methods in the current library
es2020.bigint.d.ts
-
%TypedArray%.{every, find, findIndex, some}: The return type of the first parameter can be widen toanyto keep consistency with other array prototype methods -
%TypedArray%Constructor.from: Missing documentation for the second overload. Also see the es2015.core.d.ts section above
es2021.string.d.ts
-
String.replaceAll: missing{ [Symbol.replace] }overloads. It should be defined exactly the same asString.replace(The documentation should be modify toA string or RegExp search valuetoo to keep it the same across all files)
es2022.object.d.ts
-
Object.hasOwn: Per the spec,ocan be widen to{}
es5.d.ts
-
ObjectConstructor.{getPrototypeOf, getOwnPropertyDescriptor, getOwnPropertyNames}: 🟠 The first parameter should extends{}. See the es2017.object.d.ts section above -
{CallableFunction, NewableFunction}.apply: Missing documentation for the second overload -
{CallableFunction, NewableFunction}.bind: Consider rewrite it like what we've done -
{Array<T>, ReadonlyArray<T>}.every: 🟠 The return type of the first overload can be typedthis is { [K in keyof this]: S }to support tuples.Unlike other solutions, this change is small enough that it should not break anything.It does affect some important tests, so a separate PR will be opened. -
ReadonlyArray<T>.map: 🟠 The return type can be typed{ -readonly [K in keyof this]: U }to support tuples. Will be in the follow-up PR -
Array<T>.map: 🟠 The return type can be typed{ [K in keyof this]: U }to support tuples. Will be in the follow-up PR -
{Array<T>, ReadonlyArray<T>}.{reduce, reduceRight}: Missing documentation for the second overload -
%TypedArray%.{find, findIndex}: The return type of the first parameter can be widen toanyto keep consistency with other array prototype methods -
%TypedArray%Constructor.from: See the es2015.core.d.ts section above
🟠 Breaking Changes (non-exhaustive)
If the TypeScript Team is fine with certain of these changes, I am happy to open a PR for it.
Thanks for reading till the end. That is all.
P.S. I am not sure if I shouldn't put all of the above into a single issue, but opening numerous issues to reflect all of these suggestions will be too many for me. I wonder how many PRs would be comfortable to the Team and how should I split it if I'm gonna work on this.
Activity
MartinJohns commentedon Jul 4, 2022
That would be a bad change. See this comment: #39090 (comment)
graphemecluster commentedon Jul 4, 2022
Martin Johns (@MartinJohns) Thanks for pointing that out. I've just crossed that item out. Are there any other inappropriate changes? My apologies if there're still any.
MartinJohns commentedon Jul 4, 2022
I didn't look thoroughly through it, just a few points here and there that raise eyebrows, like
The
UnionToIntersectiontype has been called an ill idea by the TypeScript team several times, and has no official support. You definitely won't see that in a standard library.See this comment: #41857 (comment)
Besides that it's very unclear which proposed changes would be breaking changes.
graphemecluster commentedon Jul 4, 2022
Martin Johns (@MartinJohns) Thanks again. I've marked some of them as breaking changes. I wonder if the changes to
{Array<T>, ReadonlyArray<T>}.{every, map}are also considered breaking.fatcerberus commentedon Jul 4, 2022
Surely it should be
object?{}basically means "anything object-coercible" which includes primitives, but it doesn't make much sense to set the prototype of a primitive number, for example. While it might not throw, it's not going to do anything useful, either.graphemecluster commentedon Jul 4, 2022
Bruce Pascoe (@fatcerberus) Yes. Originally I did put an ”at least“ in that line, but later I realized that consistency should be kept across all Object prototype methods. Moreover currently it is typed
anyand suddenly making itobjectmay cause inappropriate breaks in current codebases. Although altering it to{}is still a breaking change, I considered this appropriate even if it breaks current codebases because it may cause runtime errors.Josh-Cena commentedon Jul 4, 2022
I do see your doubts on these, but I'd still like to point out:
It has to have a
[[Construct]]internal slot, soFunctionseems like the best type we can haveReflect.getPrototypeOf(Object.create(null))?Reflect.setPrototypeOf({}, "1")is aTypeError.RyanCavanaugh commentedon Jul 5, 2022
Some of these seem OK and some are a little bit dicier. We can take a PR and evaluate case-by-case -- please be sure to include testcases that demonstrate what's being fixed.
wchargin commentedon May 13, 2024
This is checked and the issue is closed, but it is not actually fixed because #50451 was never merged. Can we reopen the issue or at least un-check the item to avoid confusion?