Skip to content

Update bigint declaration file #38526

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.

Sign up for GitHub

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

Merged
merged 12 commits into from
Jun 22, 2020
Merged

Update bigint declaration file #38526

Show file tree
Hide file tree
Changes from 9 commits
Commits
Show all changes
12 commits
Select commit Hold shift + click to select a range
7d5942d
update toLocalString function signature
May 13, 2020
2f468d9
update test.
May 13, 2020
b11a746
fix lint
May 14, 2020
b1ad32c
follow review advice.
May 22, 2020
5592f60
format and better comment.
May 22, 2020
af547be
format
May 25, 2020
f959c5f
add case
May 25, 2020
e9410d6
Merge branch 'master' into update-bigint-declaration-file
May 25, 2020
bd99f0a
fix symbol.
May 25, 2020
bab6c28
Merge branch 'master' into update-bigint-declaration-file
ShuiRuTian Jun 19, 2020
abf01a3
remove subtype and string union in interface.
ShuiRuTian Jun 19, 2020
1b3f4f5
remove useless code.
ShuiRuTian Jun 22, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
105 changes: 104 additions & 1 deletion src/lib/es2020.bigint.d.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,3 +1,99 @@
interface BigIntToLocaleStringOptionsBase {
/**
* The locale matching algorithm to use.The default is "best fit". For information about this option, see the {@link https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Intl#Locale_negotiation Intl page}.
*/
localeMatcher?: "lookup" | "best fit" | string;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

"x" | string is currently reduced to string and the literals are thrown out. I think string is fine for now.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The feature request for supporting this is #33471

Copy link
Contributor Author

@ShuiRuTian ShuiRuTian May 22, 2020 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do I need to remove these "x", or just leave them here, waiting for the benefit of this feature request?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's a fairly complex feature and not likely to be in 4.0 or 4.1. We should stick with string for all these unions.

/**
* The formatting style to use , the default is "decimal".
*/
style?: "decimal" | "percent" | string;

numberingSystem?: "arab" | "arabext" | "bali" | "beng" | "deva" | "fullwide" | "gujr" | "guru" | "hanidec" | "khmr" | "knda" | "laoo" | "latn" | "limb" | "mlym" | "mong" | "mymr" | "orya" | "tamldec" | "telu" | "thai" | "tibt" | string;
/**
* The unit to use in unit formatting, Possible values are core unit identifiers, defined in UTS #35, Part 2, Section 6. A subset of units from the full list was selected for use in ECMAScript. Pairs of simple units can be concatenated with "-per-" to make a compound unit. There is no default value; if the style is "unit", the unit property must be provided.
*/
unit?: string;

/**
* The unit formatting style to use in unit formatting, the defaults is "short".
*/
unitDisplay?: "long" | "short" | "narrow" | string;

/**
* The currency to use in currency formatting. Possible values are the ISO 4217 currency codes, such as "USD" for the US dollar, "EUR" for the euro, or "CNY" for the Chinese RMB — see the Current currency & funds code list. There is no default value; if the style is "currency", the currency property must be provided. It is only used when [[Style]] has the value "currency".
*/
currency?: string;
/**
* How to display the currency in currency formatting. It is only used when [[Style]] has the value "currency". The default is "symbol".
*
* "symbol" to use a localized currency symbol such as €,
*
* "code" to use the ISO currency code,
*
* "name" to use a localized currency name such as "dollar"
*/
currencyDisplay?: "symbol" | "code" | "name" | string;

/**
* Whether to use grouping separators, such as thousands separators or thousand/lakh/crore separators. The default is true.
*/
useGrouping?: boolean;

/**
* The minimum number of integer digits to use. Possible values are from 1 to 21; the default is 1.
*/
minimumIntegerDigits?: 1 | 2 | 3 | 4 | 5 | 6 | 7 | 8 | 9 | 10 | 11 | 12 | 13 | 14 | 15 | 16 | 17 | 18 | 19 | 20 | 21;

/**
* The minimum number of fraction digits to use. Possible values are from 0 to 20; the default for plain number and percent formatting is 0; the default for currency formatting is the number of minor unit digits provided by the {@link http://www.currency-iso.org/en/home/tables/table-a1.html ISO 4217 currency codes list} (2 if the list doesn't provide that information).
*/
minimumFractionDigits?: 0 | 1 | 2 | 3 | 4 | 5 | 6 | 7 | 8 | 9 | 10 | 11 | 12 | 13 | 14 | 15 | 16 | 17 | 18 | 19 | 20;

/**
* The maximum number of fraction digits to use. Possible values are from 0 to 20; the default for plain number formatting is the larger of minimumFractionDigits and 3; the default for currency formatting is the larger of minimumFractionDigits and the number of minor unit digits provided by the {@link http://www.currency-iso.org/en/home/tables/table-a1.html ISO 4217 currency codes list} (2 if the list doesn't provide that information); the default for percent formatting is the larger of minimumFractionDigits and 0.
*/
maximumFractionDigits?: 0 | 1 | 2 | 3 | 4 | 5 | 6 | 7 | 8 | 9 | 10 | 11 | 12 | 13 | 14 | 15 | 16 | 17 | 18 | 19 | 20;

/**
* The minimum number of significant digits to use. Possible values are from 1 to 21; the default is 1.
*/
minimumSignificantDigits?: 1 | 2 | 3 | 4 | 5 | 6 | 7 | 8 | 9 | 10 | 11 | 12 | 13 | 14 | 15 | 16 | 17 | 18 | 19 | 20 | 21;
/**
* The maximum number of significant digits to use. Possible values are from 1 to 21; the default is 21.
*/
maximumSignificantDigits?: 1 | 2 | 3 | 4 | 5 | 6 | 7 | 8 | 9 | 10 | 11 | 12 | 13 | 14 | 15 | 16 | 17 | 18 | 19 | 20 | 21;

/**
* The formatting that should be displayed for the number, the defaults is "standard"
*
* "standard" plain number formatting
*
* "scientific" return the order-of-magnitude for formatted number.
*
* "engineering" return the exponent of ten when divisible by three
*
* "compact" string representing exponent, defaults is using the "short" form
*/
notation?: "standard" | "scientific" | "engineering" | "compact" | string;

/**
* used only when notation is "compact"
*/
compactDisplay?: "short" | "long" | string;
}

interface BigIntToLocaleStringOptionsStyleUnit extends Omit<BigIntToLocaleStringOptionsBase, "style"> {
style: "unit";
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

All you get for the overhead of these two interfaces+union is the ability to make unit appear only when style: "unit". Considering that every other field in BigIntToLocaleStringOptionsBase is optional, I think it's better to put both unit and currency in the base as optional too, then get rid of the subtypes.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah.
In fact, "unit" and "currency" are already in base class as optional.
I would like to use "unit" as example to explain why I did this:
If "style" is not "unit", the value of "unit" is arbitrary, but if "style" is "unit" and "unit" is undefined, there would be an error. With this, user could know the error while type checking.
So which side does the responsibility to avoid error belongs to? The user-side or ts-definition?
I prefer latter one, but both are reasonable.
Anyway, I would follow your final decision.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The tradeoff is that the standard library is more complicated and slower to check. I'd like to go with the simpler single-interface solution and let users avoid the error.

unit: string;
}

interface BigIntToLocaleStringOptionsStyleCurrency extends Omit<BigIntToLocaleStringOptionsBase, "style"> {
style: "currency";
currency: string;
}

type BigIntToLocaleStringOptions = BigIntToLocaleStringOptionsStyleUnit | BigIntToLocaleStringOptionsStyleCurrency | BigIntToLocaleStringOptionsBase;

interface BigInt {
/**
* Returns a string representation of an object.
Expand All @@ -6,7 +102,7 @@ interface BigInt {
toString(radix?: number): string;

/** Returns a string representation appropriate to the host environment's current locale. */
toLocaleString(): string;
toLocaleString(locales?: string, options?: BigIntToLocaleStringOptions): string;

/** Returns the primitive value of the specified object. */
valueOf(): bigint;
Expand Down Expand Up @@ -613,3 +709,10 @@ interface DataView {
*/
setBigUint64(byteOffset: number, value: bigint, littleEndian?: boolean): void;
}

declare namespace Intl{
interface NumberFormat {
format(value: number | bigint): string;
resolvedOptions(): ResolvedNumberFormatOptions;
}
}
21 changes: 15 additions & 6 deletions tests/baselines/reference/bigintWithLib.errors.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
tests/cases/compiler/bigintWithLib.ts(4,1): error TS2350: Only a void function can be called with the 'new' keyword.
tests/cases/compiler/bigintWithLib.ts(16,33): error TS2769: No overload matches this call.
tests/cases/compiler/bigintWithLib.ts(21,33): error TS2769: No overload matches this call.
Overload 1 of 3, '(length?: number): BigInt64Array', gave the following error.
Argument of type 'number[]' is not assignable to parameter of type 'number'.
Overload 2 of 3, '(array: Iterable<bigint>): BigInt64Array', gave the following error.
Expand All @@ -12,18 +12,18 @@ tests/cases/compiler/bigintWithLib.ts(16,33): error TS2769: No overload matches
Overload 3 of 3, '(buffer: ArrayBuffer | SharedArrayBuffer, byteOffset?: number, length?: number): BigInt64Array', gave the following error.
Argument of type 'number[]' is not assignable to parameter of type 'ArrayBuffer | SharedArrayBuffer'.
Type 'number[]' is missing the following properties from type 'SharedArrayBuffer': byteLength, [Symbol.species], [Symbol.toStringTag]
tests/cases/compiler/bigintWithLib.ts(21,13): error TS2540: Cannot assign to 'length' because it is a read-only property.
tests/cases/compiler/bigintWithLib.ts(28,35): error TS2769: No overload matches this call.
tests/cases/compiler/bigintWithLib.ts(26,13): error TS2540: Cannot assign to 'length' because it is a read-only property.
tests/cases/compiler/bigintWithLib.ts(33,35): error TS2769: No overload matches this call.
Overload 1 of 3, '(length?: number): BigUint64Array', gave the following error.
Argument of type 'number[]' is not assignable to parameter of type 'number'.
Overload 2 of 3, '(array: Iterable<bigint>): BigUint64Array', gave the following error.
Argument of type 'number[]' is not assignable to parameter of type 'Iterable<bigint>'.
Overload 3 of 3, '(buffer: ArrayBuffer | SharedArrayBuffer, byteOffset?: number, length?: number): BigUint64Array', gave the following error.
Argument of type 'number[]' is not assignable to parameter of type 'ArrayBuffer | SharedArrayBuffer'.
Type 'number[]' is not assignable to type 'SharedArrayBuffer'.
tests/cases/compiler/bigintWithLib.ts(33,13): error TS2540: Cannot assign to 'length' because it is a read-only property.
tests/cases/compiler/bigintWithLib.ts(40,25): error TS2345: Argument of type '-1' is not assignable to parameter of type 'bigint'.
tests/cases/compiler/bigintWithLib.ts(43,26): error TS2345: Argument of type '123' is not assignable to parameter of type 'bigint'.
tests/cases/compiler/bigintWithLib.ts(38,13): error TS2540: Cannot assign to 'length' because it is a read-only property.
tests/cases/compiler/bigintWithLib.ts(45,25): error TS2345: Argument of type '-1' is not assignable to parameter of type 'bigint'.
tests/cases/compiler/bigintWithLib.ts(48,26): error TS2345: Argument of type '123' is not assignable to parameter of type 'bigint'.


==== tests/cases/compiler/bigintWithLib.ts (7 errors) ====
Expand All @@ -39,6 +39,11 @@ tests/cases/compiler/bigintWithLib.ts(43,26): error TS2345: Argument of type '12
let stringVal: string = bigintVal.toString();
stringVal = bigintVal.toString(2);
stringVal = bigintVal.toLocaleString();
stringVal = bigintVal.toLocaleString('de-DE');
stringVal = bigintVal.toLocaleString('de-DE');
stringVal = bigintVal.toLocaleString('de-DE', { style: 'currency' }); // should error
stringVal = bigintVal.toLocaleString('de-DE', { style: 'unit' }); // should error
stringVal = bigintVal.toLocaleString('de-DE', { style: 'currency', currency: 'EUR' })

// Test BigInt64Array
let bigIntArray: BigInt64Array = new BigInt64Array();
Expand Down Expand Up @@ -113,4 +118,8 @@ tests/cases/compiler/bigintWithLib.ts(43,26): error TS2345: Argument of type '12
const x = -12n; // should emit as const x = -12n
const y: 12n = 12n; // should emit type 12n
let z = 12n; // should emit type bigint in declaration file

// Test Intl methods with new parameter type
new Intl.NumberFormat("fr").format(3000n);
new Intl.NumberFormat("fr").format(bigintVal);

17 changes: 17 additions & 0 deletions tests/baselines/reference/bigintWithLib.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,11 @@ bigintVal = bigintVal.valueOf();
let stringVal: string = bigintVal.toString();
stringVal = bigintVal.toString(2);
stringVal = bigintVal.toLocaleString();
stringVal = bigintVal.toLocaleString('de-DE');
stringVal = bigintVal.toLocaleString('de-DE');
stringVal = bigintVal.toLocaleString('de-DE', { style: 'currency' }); // should error
stringVal = bigintVal.toLocaleString('de-DE', { style: 'unit' }); // should error
stringVal = bigintVal.toLocaleString('de-DE', { style: 'currency', currency: 'EUR' })

// Test BigInt64Array
let bigIntArray: BigInt64Array = new BigInt64Array();
Expand Down Expand Up @@ -52,6 +57,10 @@ const w = 12n; // should emit as const w = 12n
const x = -12n; // should emit as const x = -12n
const y: 12n = 12n; // should emit type 12n
let z = 12n; // should emit type bigint in declaration file

// Test Intl methods with new parameter type
new Intl.NumberFormat("fr").format(3000n);
new Intl.NumberFormat("fr").format(bigintVal);


//// [bigintWithLib.js]
Expand All @@ -65,6 +74,11 @@ bigintVal = bigintVal.valueOf();
let stringVal = bigintVal.toString();
stringVal = bigintVal.toString(2);
stringVal = bigintVal.toLocaleString();
stringVal = bigintVal.toLocaleString('de-DE');
stringVal = bigintVal.toLocaleString('de-DE');
stringVal = bigintVal.toLocaleString('de-DE', { style: 'currency' }); // should error
stringVal = bigintVal.toLocaleString('de-DE', { style: 'unit' }); // should error
stringVal = bigintVal.toLocaleString('de-DE', { style: 'currency', currency: 'EUR' });
// Test BigInt64Array
let bigIntArray = new BigInt64Array();
bigIntArray = new BigInt64Array(10);
Expand Down Expand Up @@ -104,6 +118,9 @@ const w = 12n; // should emit as const w = 12n
const x = -12n; // should emit as const x = -12n
const y = 12n; // should emit type 12n
let z = 12n; // should emit type bigint in declaration file
// Test Intl methods with new parameter type
new Intl.NumberFormat("fr").format(3000n);
new Intl.NumberFormat("fr").format(bigintVal);


//// [bigintWithLib.d.ts]
Expand Down
Loading