BigInt (Stage 3) [WIP] by wdhorton · Pull Request #6015 · babel/babel

wdhorton · 2017-07-26T01:49:21Z

Q	A
Patch: Bug Fix?	no
Major: Breaking Change?	no
Minor: New Feature?	yes
Deprecations?	no
Spec Compliancy?	no
Tests Added/Pass?
Fixed Tickets
License	MIT
Doc PR
Dependency Changes

I'm working on the transform for BigInt, and I wanted to put up a PR to get some feedback on the direction it's going.

This just works for adding two BigInts right now, obviously there are other operations, errors to be caught, edge cases, etc., etc. but I think if we can nail down the details of this case I should be able to move forward pretty quickly.

Main questions:
For @littledan:

does the transformed output (https://github.com/wdhorton/babel/blob/e538858d9e1ee6ff1e8315468e81e1344bc1d7dd/packages/babel-plugin-transform-bigint/test/fixtures/addition/expected.js) look like what you expect? Essentially we want to wrap the string values of the BigIntLiterals in the polyfill, and then add them (and I think we need a .add method until we get real support in the language, since we can't override the + operator in JS)

For babel people (@hzoo or others):

When I want to have the polyfill in the transformed file, should I be using import or require? I went with import because there's an addImport method and there doesn't seem to be an analogous state.file.addRequire method, but that breaks my tests unless I use transform-es2015-modules-commonjs as an additional plugin.
Should I be importing/requiring the BigInt polyfill as a variable, or should I be putting it on the global as a side effect of the import/require? How would that work if people use the BigInt constructor themselves in the code they want to transform?
Where should the code for the polyfill live? It's going to be a thin wrapper around an existing BigNumber library. Right now "bigint-polyfill" is just a placeholder name. Should it be in babel-helpers? babel-polyfill? A separate NPM package (that seems less than ideal for development purposes).
Any additional feedback on whether or not I'm on the right track with this would be much appreciated.

hzoo · 2017-07-26T11:19:22Z

Thanks for posting your current work, nice stuff! As a general thing you'll want to do some of the things in https://github.com/babel/babel/wiki/Adding-a-new-Proposal-to-Babel (generator to print it out, babel types to create a bigint)

hzoo · 2017-07-26T11:20:16Z

packages/babel-plugin-transform-bigint/src/index.js

+    visitor: {
+      Program: {
+        exit(path: NodePath, state: Object) {
+          state.file.addImport("bigint-polyfill", "default", "BigInt");


This is fine for now, we also use this for transform-runtime. I wouldn't call it a "polyfill" if you are just going to use the variable (then it's just the same as transform-runtime) rather than babel-polyfill.

hzoo · 2017-07-26T11:22:11Z

packages/babel-plugin-transform-bigint/src/index.js

+              t.ExpressionStatement(
+                t.CallExpression(
+                  t.MemberExpression(
+                    t.CallExpression(t.Identifier("_BigInt"), [


so here is where you can use an API method. This hardcodes it so if someone names another variable _BigInt this would have problems (unlikely but possible)

can use something like path.scope.generateUidIdentifier("BigInt"); But you want it to be the same variable as the import so don't need this actually.

EDIT: addImport actually returns the id

babel/packages/babel-core/src/transformation/file/index.js

Lines 181 to 212 in 28ae47a

addImport(

source: string,

imported: string,

name?: string = imported,

): Object {

const alias = `${source}:${imported}`;

let id = this.dynamicImportIds[alias];

if (!id) {

source = this.resolveModuleSource(source);

id = this.dynamicImportIds[alias] = this.scope.generateUidIdentifier(

name,

);

const specifiers = [];

if (imported === "*") {

specifiers.push(t.importNamespaceSpecifier(id));

} else if (imported === "default") {

specifiers.push(t.importDefaultSpecifier(id));

} else {

specifiers.push(t.importSpecifier(id, t.identifier(imported)));

}

const declar = t.importDeclaration(specifiers, t.stringLiteral(source));

declar._blockHoist = 3;

this.path.unshiftContainer("body", declar);

}

return id;

}

so you can use that as the identifier

littledan

cc @caiolima

littledan · 2017-07-26T11:37:48Z

packages/babel-plugin-transform-bigint/src/index.js

+          if (
+            path.node.left.type === "BigIntLiteral" &&
+            path.node.left.type === "BigIntLiteral"
+          ) {


Such conditionals will not generalize sufficiently. You need to be able to handle code like

function add(a, b) { return a + b; }

Functions like this are genuinely generic. The logic needs to be, instead, "whenever I see any infix operator, replace it by a function call to my polyfill library". This applies for + as well as === as well as |--they all need to implement the new dispatch algorithm.

I'd recommend that these expand into function calls, rather than method calls, since they may be called with any object or value as the first argument, not just BigInts.

Just complementing @littledan, we probably don't have path.node.*.type when one of the operands isn't a literal, so we need to have a check at runtime in all BigInt compatible BinaryExpressions if the kind of this operand is a BigInt or not.
Also IMHO, expanding into function calls will make our life easier to follow the spec and implement runtime errors due to invalid operand, such as 10n + 1.

Actually not that bad then since you don't need the if statement anymore? You just replace + with .add and so on?

Following Dan's example, what I have in mind is something like that:

function add(a, b) { var tmp; if (a instanceof _BingInt || b instanceof _BingInt) tmp = _BigIntAdd(a, b) else tmp = a + b; return tmp; }

PS.: The names are just placeholders

This might also not match the spec semantics. It's important to throw on Number + BigInt, cast to string on String + BigInt, ext.

Sorry if I wasn't 100% clear. I'm thinking in handle semantic cases and errors into _BigIntAdd(a, b). This function follows the implementation of https://tc39.github.io/proposal-bigint/#sec-additive-operators in JS. Does it make sense?

@hzoo probably we would like to have the "ifs" there to avoid checks at runtime of literals, for example.

littledan · 2017-07-26T12:05:18Z

packages/babel-plugin-transform-bigint/test/fixtures/addition/expected.js

@@ -0,0 +1,2 @@
+import _BigInt from "bigint-polyfill";


As for the polyfill library, I'd suggest starting with something like node-bigint. This has all the functions implemented, though it only works on Node, not the web. I don't think this is a problem, as the Babel implementation is more for exploration/demonstration purposes than actual deployment.

On top of whatever base library you start with, there will have to be significant things to match the form of the spec. This includes:

Dispatching appropriately on Number vs BigInt

Methods like asUintN, which you're unlikely to find elsewhere

Being a little creative with some functions, for example BigInt.prototype.valueOf should throw, to prevent implicit coercion to Number.

would be nice if we could use in the REPL but I guess that's not possible in this case (or we have to fake it)

I had been thinking of this for the polyfill so that we could get browser support as well https://github.com/MikeMcl/bignumber.js/ @littledan are there any drawbacks? i'm not really familiar with the different libraries in this space

Could even have an option for diff library if you wanted, if it passes the tests we make it's all good

@wdhorton Sure, go for it. That library may be slower, that's all, but working in browsers wouldn't be bad.

wdhorton · 2017-08-09T03:42:36Z

@hzoo @littledan made changes based on your feedback. I've got the addition case passing for BigInts, and not breaking for normal numbers.

A few more questions:

I added BigInt to babel-polyfill because I realized that, to match the spec, the constructor should be available in the global scope, so people can write:

BigInt('1223456')

in addition to the literal form 1223456n. I'd like to make sure that's the right idea, and if so, should I change anything about how I integrated it into babel-polyfill

I was still struggling to find the right place in the codebase for the function to check binary expressions. I started out trying to add it to babel-helpers, since that seemed like the natural place for helper functions, but I ran into an issue because if you include a helper, the code from that helper is run through the transform. To illustrate, the helper code:

(function(left, right) {
  if (left instanceof BigInt && right instanceof BigInt) {
    return left.plus(right);
  } else {
    return left + right;
  }
})

was transformed to this:

function _checkBinaryExpression(left, right) { 
  if (left instanceof BigInt && right instanceof BigInt) { 
    return left.plus(right); 
  } else { 
    return _checkBinaryExpression(left, right); 
  } 
}

which caused infinite recursion on non-BigInt numbers. I might be missing something, but it seems like I can't include BinaryExpressions in the helper code if the whole thing the transform does is change BinaryExpressions into calls to the helper.

So for now I just stuck it in another package (babel-check-binary-expressions), but I'd like to discuss what the best way to handle this is.

xtuc · 2017-08-09T15:29:17Z

packages/babel-check-binary-expressions/src/index.js

@@ -0,0 +1,8 @@
+export default function(left, right) {
+  // eslint-disable-next-line no-undef
+  if (left instanceof BigInt && right instanceof BigInt) {


Would it make sense to use window.BigInt (or global.BigInt) to avoid the no-undef rule?

yes, that makes sense. if I use global.BigInt, will it work in the browser and Node?

Yes sure, I was thinking about how it would be bundled but that's an unnecessary change anyway.

caiolima

That's a very good job!

caiolima · 2017-08-09T22:46:12Z

packages/babel-check-binary-expressions/src/index.js

+  if (left instanceof BigInt && right instanceof BigInt) {
+    return left.plus(right);
+  } else {
+    return left + right;


I think it's not spec compliant. You need to emit TypeError if Type(left) is different of Type(right). Check the semantics here https://tc39.github.io/proposal-bigint/#sec-additive-operators

In your current function, if left instanceof BigInt === true and left instanceof BigInt === false it will fallback to else path and probably will result wrong result.

caiolima · 2017-08-09T22:50:46Z

packages/babel-plugin-transform-bigint/test/fixtures/addition/add-two-bigints-exec/exec.js

@@ -0,0 +1 @@
+assert.equal((1000n + 200n).toString(), (new BigInt('1200')).toString())


Add test cases when you have BigInt + other primitive.

hzoo · 2017-08-10T04:12:54Z

packages/babel-polyfill/src/index.js


 import "core-js/shim";
 import "regenerator-runtime/runtime";
+import "./bigint/bigint";


I don't think we want to add this to babel-polyfill itself, since most people won't be using it and will ask about the size increase.

I guess it would be covered by preset-env and only adding if you use the preset/plugin but not sure how that would work really

fair point. where should i put it?

littledan · 2017-08-10T11:00:35Z

packages/babel-check-binary-expressions/src/index.js

+  }
+
+  if (isString(left) || isString(right)) {
+    return left.toString() + right.toString();


This is not quite right. For example, a user may monkey-patch Number.prototype.toString, but that behavior shouldn't be reflected here. Instead, do simply left + right.

In general: If you can try to follow the spec in order, one by one, you'll arrive at something closer to compliance. Spec for +.

littledan · 2017-08-10T11:01:50Z

packages/babel-check-binary-expressions/src/index.js

+  return x instanceof global.BigInt;
+}
+
+export default function(left, right, operator) {


I'm not sure why you take the operator as an argument, but then you just use +.

littledan · 2017-08-10T11:08:17Z

packages/babel-check-binary-expressions/src/index.js

+  }
+
+  if (isBigInt(left) && isBigInt(right)) {
    return left.plus(right);


Better to not use a method called plus but instead a function. The problem with a method is that it's easily accessible to users (who will be missing it in the real version). It's better to use a function, which we could refrain from exporting to them.

xtuc · 2017-08-11T08:08:17Z

packages/babel-check-binary-expressions/src/index.js

@@ -0,0 +1,26 @@
+import isNumber from "lodash/isNumber";
+import isString from "lodash/isString";


I think we can void the lodash dependency here, you can replace it with:

typeof x != "number" and typeof x != "string".

littledan · 2017-08-29T15:17:13Z

I'm wondering, how are you running this code? Do you have a BigInt library that you are using? Or are you just examining the generated output?

hzoo · 2017-08-29T15:18:34Z

maybe if you rebase you could test this with our new repl links, although not sure how it would add the polyfill until we figure out a way to import those

wdhorton · 2017-08-30T01:30:17Z

@littledan i just shuffled stuff around a bit, but I made a BigInt polyfill that wraps around the big-integer package. That part of the PR is here. In the test environment, Babel loads the polyfills, so that's what's on global.BigInt. The tests that are named exec.js are actually executing the code, so I'm using that to test that it runs, in addition to examining the generated output.

…and concatting on BigInt + String

…ferent bigint lib.

wdhorton · 2017-08-30T03:40:28Z

@hzoo I'm seeing this error: Error: [BABEL] /home/ubuntu/babel/packages/babel-plugin-transform-bigint/test/fixtures/subtraction/subtract-number-and-string/exec.js: Cannot find module '/home/ubuntu/babel/packages/babel-plugin-transform-bigint/node_modules/babel-helper-transform-fixture-test-runner/lib/../../babel-plugin-transform-bigint' from '/home/ubuntu/babel' on the Circle tests, but I can't really figure out what's going wrong because they're passing locally. Have you seen that error before?

undeadcat · 2017-09-19T21:23:45Z

Sorry to barge in, I came here from Googling the proposal, but JFYI (or am I mistaken?) this also seems to be missing support for library functions:

BigInt.asUintN(width, BigInt)
BigInt.asIntN(width, BigInt)
BigInt.parseInt(string[, radix])

described here and in the spec.

Yaffle · 2018-06-22T17:59:40Z

if someone is interested in ... - https://github.com/Yaffle/babel-plugin-transform-bigint

xtuc · 2018-06-22T18:23:55Z

@Yaffle that looks great. Have you tried to run it against the tests here? Would you mind helping us pushing this PR forward?

Yaffle · 2018-06-22T18:33:42Z

@xtuc, thanks, I didn't try to run it against tests. How can I help you with this PR?

littledan · 2018-06-24T13:34:03Z

@Yaffle Great work! At a skim, this repo looks generally correct.

I'd like to argue that this transform should be kept out of all presets unless explicitly included, as it makes fairly little sense to run in production with the general performance degradation that it causes.

xtuc · 2018-11-09T10:52:11Z

packages/babel-plugin-syntax-bigint/package.json

@@ -0,0 +1,13 @@
+{
+  "name": "babel-plugin-syntax-bigint",


The plugin-syntax-bigint is already on master, could we remove it from this PR?

ItamarGronich · 2023-08-02T11:51:55Z

@wdhorton Seems like this PR was abandoned. Is there a specific reason? any alternatives?
As far as i read, yours is the only complete transform solution for this.

wdhorton mentioned this pull request Jul 26, 2017

BigInt (Stage 3) babel/proposals#2

Open

hzoo reviewed Jul 26, 2017

View reviewed changes

littledan suggested changes Jul 26, 2017

View reviewed changes

tarcieri mentioned this pull request Jul 26, 2017

Support for TC39 "BigInt: Arbitrary precision integers in JavaScript" proposal microsoft/TypeScript#15096

Closed

hzoo mentioned this pull request Aug 8, 2017

July 2017 babel/proposals#19

Closed

10 tasks

xtuc reviewed Aug 9, 2017

View reviewed changes

caiolima suggested changes Aug 9, 2017

View reviewed changes

hzoo reviewed Aug 10, 2017

View reviewed changes

littledan reviewed Aug 10, 2017

View reviewed changes

xtuc reviewed Aug 11, 2017

View reviewed changes

wdhorton added 8 commits August 29, 2017 21:31

Make plugin for syntax.

7697f7b

Initial try at transforming addition

b9ebb37

Fixes based on PR feedback.

e04c59e

Add to babel-generator.

b17fb98

Fix package.json

be62403

Improve spec compliance for addition by throwing on BigInt + Number, …

2832bf5

…and concatting on BigInt + String

Add support for additional operators.

3c5d374

Separate BigInt polyfill from babel-polyfill package, and swap in dif…

1255f03

…ferent bigint lib.

wdhorton force-pushed the bigint branch from eab6b52 to 1255f03 Compare August 30, 2017 01:35

wdhorton added 3 commits August 29, 2017 21:53

Update usage to new bigint lib in polyfill

5623351

Start UnaryExpressions

acb6eea

Tests!

03e96c5

danez closed this Aug 31, 2017

danez reopened this Aug 31, 2017

danez changed the base branch from 7.0 to master August 31, 2017 17:01

hzoo changed the title ~~BigInt (Stage 2) [WIP]~~ BigInt (Stage 3) [WIP] Sep 5, 2017

hzoo added area: tc39 proposal PR: New Feature 🚀 A type of pull request used for our changelog categories labels Sep 14, 2017

hzoo mentioned this pull request Sep 28, 2017

Extensible Numeric Literals: Stage 1 babel/proposals#30

Open

hzoo added Spec: BigInt and removed area: tc39 proposal labels Oct 5, 2017

xtuc mentioned this pull request Feb 20, 2018

BigInt polyfill? zloirock/core-js#381

Closed

hzoo mentioned this pull request May 6, 2018

Missing generator for BigIntLiteral #7873

Closed

hzoo mentioned this pull request May 22, 2018

Bigint Support (no transform) #8006

Merged

aqrln mentioned this pull request Aug 10, 2018

Infinite integer, class Id metarhia/common#161

Closed

TehShrike mentioned this pull request Aug 10, 2018

Babel/polyfill tc39/proposal-bigint#165

Closed

xtuc reviewed Nov 9, 2018

View reviewed changes

pmdartus mentioned this pull request Dec 18, 2018

add support for BigInt salesforce/lwc#890

Open

JoshRosenstein mentioned this pull request Apr 26, 2019

feat: BigInt Support jestjs/jest#8382

Merged

2 tasks

	addImport(
	source: string,
	imported: string,
	name?: string = imported,
	): Object {
	const alias = `${source}:${imported}`;
	let id = this.dynamicImportIds[alias];

	if (!id) {
	source = this.resolveModuleSource(source);
	id = this.dynamicImportIds[alias] = this.scope.generateUidIdentifier(
	name,
	);

	const specifiers = [];

	if (imported === "*") {
	specifiers.push(t.importNamespaceSpecifier(id));
	} else if (imported === "default") {
	specifiers.push(t.importDefaultSpecifier(id));
	} else {
	specifiers.push(t.importSpecifier(id, t.identifier(imported)));
	}

	const declar = t.importDeclaration(specifiers, t.stringLiteral(source));
	declar._blockHoist = 3;

	this.path.unshiftContainer("body", declar);
	}

	return id;
	}

		@@ -0,0 +1 @@
		assert.equal((1000n + 200n).toString(), (new BigInt('1200')).toString())

		@@ -0,0 +1,26 @@
		import isNumber from "lodash/isNumber";
		import isString from "lodash/isString";

Uh oh!

Conversation

wdhorton commented Jul 26, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

hzoo commented Jul 26, 2017

Uh oh!

Choose a reason for hiding this comment

Uh oh!

hzoo Jul 26, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

littledan left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

caiolima Jul 26, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

wdhorton commented Aug 9, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

caiolima left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

hzoo Aug 10, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

littledan commented Aug 29, 2017

Uh oh!

hzoo commented Aug 29, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

wdhorton commented Aug 30, 2017

Uh oh!

wdhorton commented Aug 30, 2017

Uh oh!

undeadcat commented Sep 19, 2017

wdhorton commented Jul 26, 2017 •

edited

Loading

hzoo Jul 26, 2017 •

edited

Loading

caiolima Jul 26, 2017 •

edited

Loading

wdhorton commented Aug 9, 2017 •

edited

Loading

hzoo Aug 10, 2017 •

edited

Loading

hzoo commented Aug 29, 2017 •

edited

Loading