Skip to content
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.

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

BigInt should be allowed to use as object key #136

Closed
fisker opened this issue Oct 28, 2020 · 8 comments · Fixed by #140
Closed

BigInt should be allowed to use as object key #136

fisker opened this issue Oct 28, 2020 · 8 comments · Fixed by #140

Comments

@fisker
Copy link
Collaborator

fisker commented Oct 28, 2020

require('meriyah').parse('{1n:1n}',{next:true})
Uncaught [ParseError [SyntaxError]: [1:4]: Unexpected token: ':'
] {
  index: 4,
  line: 1,
  column: 4,
  description: "[1:4]: Unexpected token: ':'",
  loc: { line: 1, column: 4 }
}
console.log({1n:1})
{ '1': 1 }
@fisker fisker changed the title BigInt should allow in object key BigInt should be allowed to use as object key Oct 28, 2020
@3cp
Copy link
Member

3cp commented Oct 28, 2020

What's your node/browser version?

⋊> ~/g/meriyah on jsx-fix ⨯ node
Welcome to Node.js v12.18.3.
Type ".help" for more information.
> console.log({1n:1})
console.log({1n:1})
             ^^

Uncaught SyntaxError: Unexpected number

@3cp
Copy link
Member

3cp commented Oct 28, 2020

node v15 works.

@KFlash
Copy link
Contributor

KFlash commented Oct 28, 2020

It's a valid case. BigInt can be a property key. Also for Class

@3cp
Copy link
Member

3cp commented Oct 28, 2020

I see where is missed.

} else if ((parser.token & Token.IsStringOrNumber) === Token.IsStringOrNumber) {
        key = parseLiteral(parser, context);

Our Token.IsStringOrNumber only applies to string and number, not bigint.
Should I add bigint to Token.IsStringOrNumber?

ecma spec seems indicating bigint can be used anywhere a normal number fits.

NumericLiteral::
  DecimalLiteral
  DecimalBigIntegerLiteral
  NonDecimalIntegerLiteral
  NonDecimalIntegerLiteralBigIntLiteralSuffix

@KFlash
Copy link
Contributor

KFlash commented Oct 28, 2020

Yes. It's easy to update token.ts

@3cp
Copy link
Member

3cp commented Oct 28, 2020

also need to update parseLiteral to check bigint.

@KFlash
Copy link
Contributor

KFlash commented Oct 28, 2020

why? How do you parse BigInt now? Just check for "if (token === Token.BigInt") return parseBigInt)

@3cp
Copy link
Member

3cp commented Oct 28, 2020

yes, I am adding the line to parseLiteral, if you add the line outside of parseLiteral, you need to add it in many places after Token.IsStringOrNumber check.

3cp added a commit that referenced this issue Oct 28, 2020
@3cp 3cp closed this as completed in #140 Oct 28, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants