BigInt constructor should not accept any type #38995

LongTengDao · 2020-06-09T16:57:33Z

InExtremaRes · 2020-06-10T00:49:36Z

Why is boolean and object allowed? Just compatibility? BigInt promotes every object to string before convertion and unlike Number (that returns NaN) it throws if the string doesn't represent a valid integer. I guess most of the time passing and object will be a mistake and for valid cases there is always the possibility of converting to string manually beforehand. Also the type object is very "loose" since it allows everything but a primitive, and I feel that most likely pass a function will be non intended, as opposed to call that function an pass the result, for instance.

Wouldn't be better allowing only string | number | bigint first and hope the world doesn't break because of the change?

LongTengDao · 2020-06-10T15:59:55Z

Why is boolean and object allowed? Just compatibility? BigInt promotes every object to string before convertion and unlike Number (that returns NaN) it throws if the string doesn't represent a valid integer. I guess most of the time passing and object will be a mistake and for valid cases there is always the possibility of converting to string manually beforehand. Also the type object is very "loose" since it allows everything but a primitive, and I feel that most likely pass a function will be non intended, as opposed to call that function an pass the result, for instance.

Wouldn't be better allowing only string | number | bigint first and hope the world doesn't break because of the change?

@DanielRosenwasser Any opinion?

DanielRosenwasser · 2020-06-11T01:25:21Z

This thing is just a conversion function from another type, so it should be somewhat permissive. The only thing that doesn't work is null and undefined which we're removing.

@typescript-bot test this
@typescript-bot user test this
@typescript-bot run dt

typescript-bot · 2020-06-11T01:25:24Z

Heya @DanielRosenwasser, I've started to run the parallelized Definitely Typed test suite on this PR at 759fff5. You can monitor the build here.

typescript-bot · 2020-06-11T01:25:24Z

Heya @DanielRosenwasser, I've started to run the parallelized community code test suite on this PR at 759fff5. You can monitor the build here.

typescript-bot · 2020-06-11T01:25:24Z

Heya @DanielRosenwasser, I've started to run the extended test suite on this PR at 759fff5. You can monitor the build here.

InExtremaRes · 2020-06-11T02:16:32Z

This thing is just a conversion function from another type, so it should be somewhat permissive.

Yeah, but other "primitive boxes" when called as a function does give a valid value of the primitive type:

Number({})  // -> NaN, a number
Boolean({}) // -> true, a boolean
String({})  // -> '[object Object]', an string
Symbol({})  // -> a new symbol
BigInt({})  // throws :(

Those maybe are just a side effect of the convertion to primitive and could or not be useful, but the type declaration doesn't lie. With BigInt most of the time an object will throw and in other cases TypeScript already prevents a "this is probably a mistake" scenario, even if the ES spec allows a value of any type; IMO this is a good opportunity of doing it as well to obtain a compile-time error instead of a runtime one, a dev can always call BigInt(anObject.toString()) or something like that if that is really the intention.

BTW I think Number() explicitly disallow symbols but it is typed as any as well.

LongTengDao · 2020-06-22T06:59:09Z

In consideration of below code also makes a TS error, maybe what InExtremaRes said is right?

const keyObj = {
	[Symbol.toPrimitive] () :'key' { return 'key'; },
	toString () :'key' { return 'key'; },
	valueOf () :'key' { return 'key'; },
};

( { 'key': 'value' } )[keyObj];

sandersn · 2020-09-04T16:17:45Z

@LongTengDao sorry for letting this get stale. The next step is to see how much (if any) of Definitely Typed packages and our user tests break with this change. I don't see PRs from the last run so I'm going to re-run:

@typescript-bot user test this
@typescript-bot run dt

Once they've run, report back on any failures.

typescript-bot · 2020-09-04T16:17:50Z

Heya @sandersn, I've started to run the parallelized Definitely Typed test suite on this PR at 759fff5. You can monitor the build here.

typescript-bot · 2020-09-04T16:17:50Z

Heya @sandersn, I've started to run the parallelized community code test suite on this PR at 759fff5. You can monitor the build here.

typescript-bot · 2020-09-04T17:10:04Z

The user suite test run you requested has finished and failed. I've opened a PR with the baseline diff from master.

fubar · 2021-02-03T21:11:06Z

@sandersn what's the status on this? We have bugs go into production because null and undefined are not rejected.

As for accepted values, BigInt({}) throws an error so object should not be included. The specs on accepted values are clear so I'm not sure why this is being discussed in the first place: https://tc39.es/ecma262/#sec-bigint-constructor and https://tc39.es/ecma262/#sec-tobigint @LongTengDao what's the reason you kept object in the list of accepted values? It sounded like the discussion concluded that it should be removed.

LongTengDao · 2021-02-04T12:37:30Z

@fubar One check fails after the commit which removed the object type, and I can't see why by the message (you can visit the red X to read it):

  Error: The configuration file "/home/runner/work/TypeScript/TypeScript/.github/codeql/codeql-configuration.yml" does not exist
      at getLocalConfig (/home/runner/work/_actions/github/codeql-action/v1/lib/config-utils.js:547:15)
      at loadConfig (/home/runner/work/_actions/github/codeql-action/v1/lib/config-utils.js:419:22)
      at Object.initConfig (/home/runner/work/_actions/github/codeql-action/v1/lib/config-utils.js:526:24)
      at Object.initConfig (/home/runner/work/_actions/github/codeql-action/v1/lib/init.js:29:38)
      at run (/home/runner/work/_actions/github/codeql-action/v1/lib/init-action.js:75:31)
      at processTicksAndRejections (internal/process/task_queues.js:93:5)
      at async runWrapper (/home/runner/work/_actions/github/codeql-action/v1/lib/init-action.js:123:9)

fubar · 2021-02-05T19:14:16Z

@LongTengDao your fork is well behind upstream, the file exists in upstream but not in your fork. Try pulling latest.

fubar · 2021-02-17T00:58:37Z

@sandersn can we get another look at this? Do those tests need to run again? The previous test runs don't have their output anymore, looks like they're too old. Thanks!

sandersn · 2021-02-24T23:12:07Z

@typescript-bot user test this
@typescript-bot run dt

typescript-bot · 2021-02-24T23:12:11Z

Heya @sandersn, I've started to run the parallelized community code test suite on this PR at f49fdca. You can monitor the build here.

typescript-bot · 2021-02-24T23:12:11Z

Heya @sandersn, I've started to run the parallelized Definitely Typed test suite on this PR at f49fdca. You can monitor the build here.

sandersn · 2021-03-11T00:59:00Z

@typescript-bot run dt

typescript-bot · 2021-03-11T00:59:04Z

Heya @sandersn, I've started to run the parallelized Definitely Typed test suite on this PR at 18b7cc9. You can monitor the build here.

sandersn · 2021-03-11T23:37:17Z

DT doesn't have any new errors (although I need to go and clean up the existing errors). The user tests were also clean back in September so I doubt anything has changed.

sandersn · 2021-03-12T00:51:25Z

This is now merged in #43204

fubar · 2021-03-12T04:45:58Z

Thank you @sandersn and @LongTengDao

Update es2020.bigint.d.ts

759fff5

LongTengDao mentioned this pull request Jul 15, 2020

BigInt constructor should not accept any type #38980

Closed

sandersn assigned DanielRosenwasser Sep 4, 2020

typescript-bot added the For Backlog Bug PRs that fix a backlog bug label Sep 4, 2020

typescript-bot mentioned this pull request Sep 4, 2020

🤖 User test baselines have changed for master typescript-bot/TypeScript#81

Open

Update es2020.bigint.d.ts

bdb6978

LongTengDao and others added 8 commits February 6, 2021 18:38

pull latest

48b6e5e

Update package-lock.json

957b7bc

Update package-lock.json

1b427f5

Update package-lock.json

80e517a

Update package-lock.json

3d795d4

Update package-lock.json

95c47c0

Update package-lock.json

766c934

Update package-lock.json

33a7f8a

typescript-bot added 4 commits February 19, 2021 06:22

Update package-lock.json

57514cf

Update package-lock.json

5f9935f

Update package-lock.json

0174643

Update package-lock.json

f49fdca

typescript-bot added 13 commits February 25, 2021 06:22

Update package-lock.json

6193d7b

Update package-lock.json

cb96a38

Update package-lock.json

67ff1be

Update package-lock.json

4528e82

Update package-lock.json

070d7f7

Update package-lock.json

ade3fa8

Update package-lock.json

33ed54a

Update package-lock.json

c90916d

Update package-lock.json

d15b499

Update package-lock.json

4b58a85

Update package-lock.json

86b7522

Update package-lock.json

7ddd97a

Update package-lock.json

18b7cc9

Update package-lock.json

2f1924b

sandersn approved these changes Mar 11, 2021

View reviewed changes

sandersn mentioned this pull request Mar 12, 2021

Bigint constructor forbids object null undefined #43204

Merged

sandersn closed this Mar 12, 2021

sandersn mentioned this pull request Mar 17, 2021

BigInt type constructor should not accept any type #40877

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

BigInt constructor should not accept any type #38995

BigInt constructor should not accept any type #38995

LongTengDao commented Jun 9, 2020

InExtremaRes commented Jun 10, 2020 •

edited

Loading

LongTengDao commented Jun 10, 2020

DanielRosenwasser commented Jun 11, 2020 •

edited

Loading

typescript-bot commented Jun 11, 2020 •

edited

Loading

typescript-bot commented Jun 11, 2020 •

edited

Loading

typescript-bot commented Jun 11, 2020 •

edited

Loading

InExtremaRes commented Jun 11, 2020

LongTengDao commented Jun 22, 2020 •

edited

Loading

sandersn commented Sep 4, 2020 •

edited

Loading

typescript-bot commented Sep 4, 2020 •

edited

Loading

typescript-bot commented Sep 4, 2020 •

edited

Loading

typescript-bot commented Sep 4, 2020

fubar commented Feb 3, 2021

LongTengDao commented Feb 4, 2021

fubar commented Feb 5, 2021

fubar commented Feb 17, 2021

sandersn commented Feb 24, 2021

typescript-bot commented Feb 24, 2021 •

edited

Loading

typescript-bot commented Feb 24, 2021 •

edited

Loading

sandersn commented Mar 11, 2021

typescript-bot commented Mar 11, 2021 •

edited

Loading

sandersn commented Mar 11, 2021

sandersn commented Mar 12, 2021

fubar commented Mar 12, 2021

BigInt constructor should not accept any type #38995

BigInt constructor should not accept any type #38995

Conversation

LongTengDao commented Jun 9, 2020

InExtremaRes commented Jun 10, 2020 • edited Loading

LongTengDao commented Jun 10, 2020

DanielRosenwasser commented Jun 11, 2020 • edited Loading

typescript-bot commented Jun 11, 2020 • edited Loading

typescript-bot commented Jun 11, 2020 • edited Loading

typescript-bot commented Jun 11, 2020 • edited Loading

InExtremaRes commented Jun 11, 2020

LongTengDao commented Jun 22, 2020 • edited Loading

sandersn commented Sep 4, 2020 • edited Loading

typescript-bot commented Sep 4, 2020 • edited Loading

typescript-bot commented Sep 4, 2020 • edited Loading

typescript-bot commented Sep 4, 2020

fubar commented Feb 3, 2021

LongTengDao commented Feb 4, 2021

fubar commented Feb 5, 2021

fubar commented Feb 17, 2021

sandersn commented Feb 24, 2021

typescript-bot commented Feb 24, 2021 • edited Loading

typescript-bot commented Feb 24, 2021 • edited Loading

sandersn commented Mar 11, 2021

typescript-bot commented Mar 11, 2021 • edited Loading

sandersn commented Mar 11, 2021

sandersn commented Mar 12, 2021

fubar commented Mar 12, 2021

InExtremaRes commented Jun 10, 2020 •

edited

Loading

DanielRosenwasser commented Jun 11, 2020 •

edited

Loading

typescript-bot commented Jun 11, 2020 •

edited

Loading

typescript-bot commented Jun 11, 2020 •

edited

Loading

typescript-bot commented Jun 11, 2020 •

edited

Loading

LongTengDao commented Jun 22, 2020 •

edited

Loading

sandersn commented Sep 4, 2020 •

edited

Loading

typescript-bot commented Sep 4, 2020 •

edited

Loading

typescript-bot commented Sep 4, 2020 •

edited

Loading

typescript-bot commented Feb 24, 2021 •

edited

Loading

typescript-bot commented Feb 24, 2021 •

edited

Loading

typescript-bot commented Mar 11, 2021 •

edited

Loading