LedgerHQ / ledger-live

Mono-repository for packages related to Ledger Live and its JavaScript ecosystem.
MIT License
431 stars 324 forks source link

Module not found '@ledgerhq/devices/hid-framing' / https://registry.yarnpkg.com/@ledgerhq%2fdevices/hid-framing returned a 405 #763

Closed dima-brook closed 2 years ago

dima-brook commented 2 years ago

Details

Describe the bug

After a clean installation of the latest version 7.0.0 the following error occurs:

Trace: Error: https://registry.yarnpkg.com/@ledgerhq%2fdevices/hid-framing: Request "https://registry.yarnpkg.com/@ledgerhq%2fdevices/hid-framing" returned a 405 at Request.params.callback [as _callback] (/Users/dimabrook/.yarn/lib/cli.js:67038:18) at Request.self.callback (/Users/dimabrook/.yarn/lib/cli.js:140883:22) at Request.emit (node:events:390:28) at Request. (/Users/dimabrook/.yarn/lib/cli.js:141855:10) at Request.emit (node:events:390:28) at IncomingMessage. (/Users/dimabrook/.yarn/lib/cli.js:141777:12) at Object.onceWrapper (node:events:509:28) at IncomingMessage.emit (node:events:402:35) at endReadableNT (node:internal/streams/readable:1343:12) at processTicksAndRejections (node:internal/process/task_queues:83:21)

Expected behavior

Should compile and build.

Additional context

Trying to run:

$ yarn add @ledgerhq/devices/hid-framing

Returns this error:

error An unexpected error occurred: "https://registry.yarnpkg.com/@ledgerhq%2fdevices/hid-framing: Request \"https://registry.yarnpkg.com/@ledgerhq%2fdevices/hid-framing\" returned a 405".

gre commented 2 years ago

@dima-brook yarn add @ledgerhq/devices should be the way to install that library, not yarn add @ledgerhq/devices/hid-framing

dima-brook commented 2 years ago
Screen Shot 2022-07-28 at 17 41 37

We've already found the issue. It is here: https://github.com/LedgerHQ/ledger-live/blob/cf80564ef12eef7b6919c52886cd91b673c54112/libs/ledgerjs/packages/hw-transport-webhid/src/TransportWebHID.ts#L7

Should be: import hidFraming from "@ledgerhq/devices/lib/hid-framing";

gre commented 2 years ago

@elbywan could you shine a light on this when you are back? 🙏 I think the import works inside our stack (changed by https://github.com/LedgerHQ/ledger-live/commit/235de21681ddb3447fd800a0309eb75ad763ea00) but causing issue outside.

elbywan commented 2 years ago

Hey @dima-brook @gre 👋,

We've already found the issue.

I don't think this is a problem, @ledgerhq/devices uses package exports which should map @ledgerhq/devices/hid-framing to @ledgerhq/devices/lib/hid-framing or @ledgerhq/devices/lib-es/hid-framing depending on the bundler configuration.

I tried to reproduce by creating a new app using create-react-app (since you are using react-app-rewired) and I managed to yarn build without issues.

I tried with adding either:

yarn add @ledgerhq/hw-transport-webhid
import TransportWebHID from "@ledgerhq/hw-transport-webhid";

or:

yarn add @ledgerhq/devices
import hidFraming from "@ledgerhq/devices/hid-framing";

My guess is that your stack relies on webpack 4 (or another random bundler) which does not support subpath exports. Upgrading to webpack 5 should make it work.

In any case I don't see any issues with the library itself.

oleg-savenok commented 2 years ago

You can use NormalModuleReplacementPlugin if you stuck with webpack 4.

plugins: [
    new webpack.NormalModuleReplacementPlugin(
        /@ledgerhq\/devices\/hid-framing/,
        '@ledgerhq/devices/lib/hid-framing'
    )
],
jordaaash commented 2 years ago

FWIW @ledgerhq/hw-transport and @ledgerhq/hw-transport-webhid work when pinned to the previous exact version 6.27.1 (published 2022-04-13), and this error is encountered with the latest release (6.27.2, published ~ a week ago, 2022-07-28).

alihalabyah commented 2 years ago

@jordansexton Tried that but it did not work

jordaaash commented 2 years ago

@alihalabyah pinning versions is tricky because the dependencies declare their transitive deps with ^.

Check out how we do this with exact versions and yarn resolutions here: https://github.com/solana-labs/wallet-adapter/commit/88f3576894702d8978172ad5673cd17a53148e68

This forces the resolved transitive deps to be pinned as well.

jordaaash commented 2 years ago

You'll know you did it right if you check your yarn.lock file and see only 6.27.1: https://github.com/solana-labs/wallet-adapter/blob/5437f957740e8651467b9eb74bf1d7ef77817f8f/yarn.lock#L1906-L1938

elbywan commented 2 years ago

So to sum it up.

Following this PR https://github.com/LedgerHQ/ledger-live/pull/364 @ledgerhq/device is now transpiled to commonjs and esm (in the /lib and /lib-es folders respectively).

Subpath exports have thus been added to use the same path (without /lib and /lib-es) in imports and allow consuming bundlers to declare which flavour of the dependency they want.

✅ For users of node.js@14+, webpack@5, esbuild, rollup, vite.js or any build tool that support subpath exports everything should be working fine.

⚠️ For users of other bundlers (parcel for instance - or webpack@4) you can either:

With webpack@4 adding the following line to the config should make it work:

resolve: {
  alias: {
    "@ledgerhq/devices": "@ledgerhq/devices/lib-es",
  },
},

With parcel, adding an alias to the package.json:

"alias": {
  "@ledgerhq/devices": "@ledgerhq/devices/lib-es"
}

Bottom line, I do not see anything wrong in the library from what has been posted so far.

Users stuck with webpack@4 or using other bundlers can add a single configuration line to map to the right path, and other users should upgrade anyway (there are only a few breaking changes between v4 and v5 - released 2 years ago).

If I missed something and you think the problem comes from anything other than what I posted feel free to comment the issue.

Otherwise please follow the instructions above.


The following demonstrate that the lib works fine with a compatible bundler.

// src/App.js
import TransportWebHID from "@ledgerhq/hw-transport-webhid";
console.log(TransportWebHID);

// src/polyfill.js
import { Buffer } from "buffer";
window.Buffer = Buffer;

// src/index.js
import "./polyfill";

Builds and prints:

Screenshot 2022-08-09 at 12 20 20
AlexeyAdoniev commented 2 years ago

Quick workaround for create-react-app(react-scripts v4) :

  1. package.json
"dependencies": {
    "react-app-rewired": "^2.2.1",
  },

 "scripts": {
    "start": "react-app-rewired start",
    "build": "react-app-rewired build",
  },
  1. config-overrides.js in root
module.exports = function override(webpackConfig) {

 webpackConfig.resolve.alias = {
    ...webpackConfig.resolve.alias,
    "@ledgerhq/devices/hid-framing": "@ledgerhq/devices/lib/hid-framing",
  };

}
ihorbond commented 2 years ago

Sounds like this should have been a breaking change, not just a minor patch.

elbywan commented 2 years ago

Sounds like this should have been a breaking change, not just a minor patch.

Yes, this is an oversight. @ledgerhq/devices did get a major bump but not the consuming hw-transport packages.

nebolsin commented 2 years ago

Just want to point out that the official Connecting an app tutorial uses parcel and is also broken now.

Adding parcel-style alias to the package.json seems to help:

{ 
  ...
  "alias": {
    "@ledgerhq/devices": "@ledgerhq/devices/lib-es"
  }
}
elbywan commented 2 years ago

@nebolsin Thanks, good catch 🙇 ! The tutorial needs to be updated.

@cfranceschi-ledger Sorry for the wild ping but I think you worked on this topic based on the file history. (cc: @fcipollone-ledger)

cfranceschi-ledger commented 2 years ago

Thank you @elbywan I will take this into account and modify the doc as soon as possible.

AmmarKhalid123 commented 2 years ago

Just a little correction in step 2:

  1. config-overrides.js in root
module.exports = function override(webpackConfig) {

 webpackConfig.resolve.alias = {
    ...webpackConfig.resolve.alias,
    "@ledgerhq/devices/hid-framing": "@ledgerhq/devices/lib/hid-framing",
  };

 return webpackConfig;
}
landabaso commented 2 years ago

This also broke the metro bundler for react-native which does not support subpath exports (yet).

If anyone gets that problem, you need to update babel.config.js to:

module.exports = {
  presets: ['module:metro-react-native-babel-preset'],
  plugins: [
    [
      'module-resolver',
      {
        root: ['.'],
        alias: {
          '@ledgerhq/devices/hid-framing': '@ledgerhq/devices/lib/hid-framing',
        },
      },
    ],
  ],
};

Remember then to reset the cache before bulding it again: npm start -- --reset-cache

Related: https://github.com/facebook/metro/issues/670

hbriese commented 1 year ago

This also broke the metro bundler for react-native which does not support subpath exports (yet).

If anyone gets that problem, you need to update babel.config.js to:

module.exports = {
  presets: ['module:metro-react-native-babel-preset'],
  plugins: [
    [
      'module-resolver',
      {
        root: ['.'],
        alias: {
          '@ledgerhq/devices/hid-framing': '@ledgerhq/devices/lib/hid-framing',
        },
      },
    ],
  ],
};

Remember then to reset the cache before bulding it again: npm start -- --reset-cache

Related: facebook/metro#670

As of react native 0.72 (expo 49) metro now supports package exports The (beta) feature needs to be enabled in your metro config