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

ui-keyring "Buffer is not defined" error with Webpack 5 #602

Closed
aolszewska opened this issue Jan 27, 2022 · 7 comments
Closed

ui-keyring "Buffer is not defined" error with Webpack 5 #602

aolszewska opened this issue Jan 27, 2022 · 7 comments
Labels
Support Tracks issues or requests related to troubleshooting, answering questions, and user assistance.

Comments

@aolszewska
Copy link

I have just migrated to Webpack 5 (with react-scripts 5.0.0 version). I have an issue using the ui-keyring lib.

In the browser console I get an error:

hid-framing.ts:19 Uncaught ReferenceError: Buffer is not defined
    at Object../node_modules/@ledgerhq/devices/lib/hid-framing.js (hid-framing.ts:19:1)
    at Object.options.factory (react refresh:6:1)
    at __webpack_require__ (bootstrap:24:1)
    at fn (hot module replacement:61:1)
    at Module../node_modules/@ledgerhq/hw-transport-webhid/lib-es/TransportWebHID.js (index.ts:316:1)
    at Module.options.factory (react refresh:6:1)
    at __webpack_require__ (bootstrap:24:1)
    at fn (hot module replacement:61:1)
    at Module../node_modules/@polkadot/hw-ledger-transports/browser.js (wrapBytes.js:9:1)
    at Module.options.factory (react refresh:6:1)

It is caused by this code:

import { keyring } from '@polkadot/ui-keyring'
...
keyring.loadAll(
  {
    isDevelopment: true,
    ss58Format: 0,
  }
)

The versions of @PolkaDot libs:

        "@polkadot/api": "7.5.1",
        "@polkadot/extension-dapp": "0.42.7",
        "@polkadot/keyring": "8.3.3",
        "@polkadot/networks": "8.3.3",
        "@polkadot/react-identicon": "0.89.3",
        "@polkadot/types": "7.5.1",
        "@polkadot/ui-keyring": "0.89.2",
        "@polkadot/ui-settings": "0.89.3",
        "@polkadot/util": "8.3.3",
        "@polkadot/util-crypto": "8.3.3",
@jacogr
Copy link
Member

jacogr commented Jan 27, 2022

This is really not a Webpack support forum, it is in the WP 5 CHANGELOG and migration guides -

@jacogr jacogr added the Support Tracks issues or requests related to troubleshooting, answering questions, and user assistance. label Jan 27, 2022
@aolszewska
Copy link
Author

aolszewska commented Jan 27, 2022

I have actually read the migration guides. I have also found this:
facebook/create-react-app#11756 (comment)
According to this point:

If the maintainers multiple times state that the package is NOT meant for usage in the front-end and close the issues - Then remove the package and find an alternative package better suited

I have revirewd the ledgerhq library. As of this issue and the linked PR, I guess they are not going to fix this issue.
So my question is:
Is polkadot-js/ui going to still use this package, although it does not conform to Webpack 5?

I have to stress that I had to migrate to Webpack 5 because of running into the issue described here in the first place. So I feel a little in a loop...
Additionally, I'm using CRA, so overriding webpack config is not straight forward without ejecting (which I would rather do not want to do).

@jacogr
Copy link
Member

jacogr commented Jan 27, 2022

Without the linked package, Ledger cannot work via WebUSB. There are no alternatives for Ledger+WebUSB support.

Webpack itself has done a great job in documenting their migration and the issues around it.

As for the linked API issue, it links to a working WP4 config - nobody forces anybody to upgrade. There is also the option to stay on older versions if you want to keep older tooling.

EDIT: For CRA, this comment could be useful to not eject - facebook/create-react-app#11756 (comment)

@jacogr
Copy link
Member

jacogr commented Jan 27, 2022

As an aside, what could be useful is an option to make the Ledger-compat opt-in instead of always applying it in the ui-keyring by default. If you believe it could add value, logging an issue for that to keep it on the radar would be good - it would break for those using it, but can be marked in the CHANGELOG with the steps to add the behavior.

PS: And yes, I absolutely despise Buffer dependencies (they give issue everywhere that is not Node), and have gone through great lengths to replace packages using it, e.g. the common repo has removed 99.9% of them - this is a case where such a replacement is not really available

@aolszewska
Copy link
Author

Thanks very much for your answers. Now I understand why this ledgerhq lib is essential. An opt-in would be an option, if I will not overcome the issue I'm facing, I will create an issue for that.
I will post here a solution for the problem once I found any :)
Thankd again!

@jacogr
Copy link
Member

jacogr commented Feb 1, 2022

1.0.1 has the Ledger export removed. I tested it against the example configs and the additional polyfil is not needed anymore, e.g. https://github.com/polkadot-js/ui/pull/606/files#diff-bb005282efb088567aa3972cb544c4e045b6a41e4297867336946bbb6e6d5a64L40

Closing this one with the above release in mind.

@jacogr jacogr closed this as completed Feb 1, 2022
@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 Feb 8, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Support Tracks issues or requests related to troubleshooting, answering questions, and user assistance.
Projects
None yet
Development

No branches or pull requests

3 participants