polkadot-js / ui

Re-usable browser libraries and React UI components used inside the polkadot.{js} family of Polkadot and Substrate applications. Full documentation & examples available
https://polkadot.js.org/ui/
Apache License 2.0
121 stars 75 forks source link

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

Closed aolszewska closed 2 years ago

aolszewska commented 2 years ago

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 commented 2 years ago

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

aolszewska commented 2 years ago

I have actually read the migration guides. I have also found this: https://github.com/facebook/create-react-app/issues/11756#issuecomment-996464456 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 commented 2 years ago

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 - https://github.com/facebook/create-react-app/issues/11756#issuecomment-1016688275

jacogr commented 2 years ago

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 commented 2 years ago

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 commented 2 years ago

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.

polkadot-js-bot commented 2 years ago

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.