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

Regression on an undocumented usage of BigInt in 6.9.2 – Unexpected token: name (n) in file node_modules/@polkadot/util/bi/sqrt.js #1245

Closed
gre opened this issue Nov 25, 2021 · 5 comments
Labels
Bug Tracks issues causing errors or unintended behavior, critical to fix for reliability.

Comments

@gre
Copy link

gre commented Nov 25, 2021

Error: Unexpected token: name (n) in file node_modules/@polkadot/util/bi/sqrt.js at 6:38

It seems like this was unexpected, because not documented in Release notes.

We have two big projects to maintain at Ledger:

  • Ledger Live Desktop (ci results) – we use webpack and hard-source-webpack-plugin which is where we also seem to experience this issue. error: Do not know how to serialize a BigInt
  • Ledger Live Mobile (ci results) – Metro fails to parse that part. BigInt also won't be supported on React Native anyway and polyfilling it is to avoid ( ReferenceError: Can't find variable: BigInt facebook/react-native#28492 ) – we track on our side that a solution is to move to FB's Hermes engine, but not easy move.

Both our projects can't upgrade anymore to latest polkadot version, last time it happened was in August and one month later our support of polkadot broke in Ledger Live because there were no provided solution. I'm trying with this ticket to avoid this situation again. Please try to have more CI to test against more environments, because BigInt is not something we can easily move on in our side. 🙏 Thanks.

@jacogr jacogr transferred this issue from polkadot-js/api Nov 25, 2021
@jacogr
Copy link
Member

jacogr commented Nov 25, 2021

  1. That seems specific to that webpack plugin - why is it trying to serialize exports? (will need to look at their code there...)
  2. Yes, there should be no move to engines required - it should work on (a recent) expo. (As an aside - BigInt polyfills won't work, since it can't do things like 1n + 2n, so imho should not be added, the code should follow a happy path without them if/when they are used)

The second issue certainly should be addressed, the intent is/was only to use paths containing BI when available in the environment. Exports of constants/utilities (which are not used in the code atm), was in the CHANGELOG, see

https://github.com/polkadot-js/common/releases/tag/v7.9.1

that adds these exports -

https://github.com/polkadot-js/common/tree/39937bed29f88b43e7b4a0c2fa6c3960910742dc/packages/util/src/bi.

The fact that parsers create chaos on them as exports-only, not used anywhere internally, is certainly unintended.

As for more CI environments, PRs would be very, very welcome - I cannot even get RN setup on my local machine as it stands :) All changes are tested against web, Node and Electron (obviously without weird plugins as applied above - it boggles the mind). As much As I want to support the world, I cannot make all people happy - so if you are unhappy, contribute.

PS: The Unexpected token: name (n) in file node_modules/@polkadot/util/bi/sqrt.js here is great, it confirms my suspicions, actually have a fix for that in that will detect at runtime before definition.

@jacogr jacogr added -size-m Bug Tracks issues causing errors or unintended behavior, critical to fix for reliability. labels Nov 25, 2021
@jacogr
Copy link
Member

jacogr commented Nov 26, 2021

The 8.0.2 release, https://github.com/polkadot-js/common/releases, takes care of the 123n syntax with runtime detection (also in dependencies). It would certainly help with the RN parsing issues, if it has any effect on the 3rd party plugins mentioned above, unsure of. (Have not looked at their specific code to see what they do)

@jacogr
Copy link
Member

jacogr commented Nov 27, 2021

PR to fix the webpack plugin mzgoddard/hard-source-webpack-plugin#550

@jacogr
Copy link
Member

jacogr commented Dec 10, 2021

Closing - 8.0.2 resolved the n syntax, 8.1.2 provides shims for BigInt definitions.

@jacogr jacogr closed this as completed Dec 10, 2021
@polkadot-js-bot
Copy link

This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue if you think you have a related problem or query.

@polkadot-js polkadot-js locked as resolved and limited conversation to collaborators Dec 17, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Bug Tracks issues causing errors or unintended behavior, critical to fix for reliability.
Projects
None yet
Development

No branches or pull requests

3 participants