Skip to content

Commit

Permalink
[Deps] pin hash-base to ~3.0, due to a breaking change
Browse files Browse the repository at this point in the history 
See browserify/hash-base#14 (comment)
  • Loading branch information
@ljharb
ljharb committed Mar 5, 2024
1 parent 168e16f commit 9e2bf12
Showing 1 changed file with 2 additions and 1 deletion.
3 changes: 2 additions & 1 deletion package.json
View file
Original file line number Diff line number Diff line change
Expand Up @@ -34,9 +34,10 @@
"create-hash": "^1.2.0",
"create-hmac": "^1.1.7",
"elliptic": "=6.5.1",
"hash-base": "~3.0",
"inherits": "^2.0.4",
"parse-asn1": "^5.1.6",
"readable-stream": "^3.6.2",
"readable-stream": "^2.3.8",
"safe-buffer": "^5.2.1"
},
"devDependencies": {
Expand Down

12 comments on commit 9e2bf12

@rjblopes
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@ljharb Why have you changed readable-stream version here? This is causing issues with crypto-browserify in browserify-sign.

@ljharb
Copy link
Member Author

@ljharb ljharb commented on 9e2bf12 Mar 13, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It was necessary to restore older node compat. See #86 (comment) - the existence of process won’t cause any issues with a working bundler.

@rjblopes
Copy link

@rjblopes rjblopes commented on 9e2bf12 Mar 13, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Expect when it's used in React Native/Metro where process is not available.

Personally, I don't think this change is worth. Adding compat support of very old Node version is causing major issues on other packages. We had to pin resolution to v4.2.2 to avoid all sort of issues in React Native.
@ljharb

@ljharb
Copy link
Member Author

@ljharb ljharb commented on 9e2bf12 Mar 13, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Then that just means that Metro is broken.

@huangmengjiao-hmj
Copy link

@huangmengjiao-hmj huangmengjiao-hmj commented on 9e2bf12 Mar 27, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can you tell me how to solve it？I use

browserify-sign+4.2.3.patch
but it has no effect.
@rjblopes

@ospfranco
Copy link

@ospfranco ospfranco commented on 9e2bf12 Apr 11, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just FYI. This indeed causes a regression on React Native where the process variable does not exist. People are patching it manually, thread here:

margelo/react-native-quick-crypto#242

Metro is not broken. It's just a runtime that does not have all the environment variables as the browser.

@ljharb
Copy link
Member Author

@ljharb ljharb commented on 9e2bf12 Apr 11, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@ospfranco if it's meant to consume node modules, and lacks process, it is broken. metro isn't a runtime, as i understand it, it's a bundler.

@ospfranco
Copy link

@ospfranco ospfranco commented on 9e2bf12 Apr 11, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry, wrote that in a hurry. Yes, metro is the bundler, has nothing to do with the current issue at hand. I meant RN as a runtime (internally uses JSC or Hermes but does not implement the full range of web APIs) is lacking the same APIs as web and node, so process is missing and probably won't be added any time soon. Would you be willing to revert this change or Node has priority for you? If you are not willing to revert this a large amount of users who depend on crypto-browserify (directly or indirectly) will come with the same request anyway, so we can at least patch it in our own modules manually.

@ljharb
Copy link
Member Author

@ljharb ljharb commented on 9e2bf12 Apr 11, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reverting this breaks node < 4. RN is using a build tool, so there should be many options available for RN users to support it.

@ospfranco
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

:S who uses Node 4?

The amount of work a release of React Native needs is massive and polyfilling a web api automatically will be hard if not impossible. I can already see your mind is set on this though. We might as well patch it however way we can. Thanks anyways.

@ljharb
Copy link
Member Author

@ljharb ljharb commented on 9e2bf12 Apr 11, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

process isn't a web API, it's a node api, and a maximal polyfill already exists https://www.npmjs.com/package/process

@pasquale95
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For whoever experiencing this problem on React-Native, I found a workaround which may be useful. In yarn.lock, replace this:

browserify-sign@^4.0.0:
  version "4.2.3"
  resolved "https://registry.yarnpkg.com/browserify-sign/-/browserify-sign-4.2.3.tgz#7afe4c01ec7ee59a89a558a4b75bd85ae62d4208"
  integrity sha512-JWCZW6SKhfhjJxO8Tyiiy+XYB7cqd2S5/+WeYHsKdNKFlCBhKbblba1A/HN/90YwtxKc8tCErjffZl++UNmGiw==
  dependencies:
    bn.js "^5.2.1"
    browserify-rsa "^4.1.0"
    create-hash "^1.2.0"
    create-hmac "^1.1.7"
    elliptic "^6.5.5"
    hash-base "~3.0"
    inherits "^2.0.4"
    parse-asn1 "^5.1.7"
    readable-stream "^2.3.8"
    safe-buffer "^5.2.1"

with this:

browserify-sign@^4.0.0:
  version "4.2.2"
  resolved "https://registry.yarnpkg.com/browserify-sign/-/browserify-sign-4.2.2.tgz#e78d4b69816d6e3dd1c747e64e9947f9ad79bc7e"
  integrity sha512-1rudGyeYY42Dk6texmv7c4VcQ0EsvVbLwZkA+AQB7SxvXxmcD93jcHie8bzecJ+ChDlmAm2Qyu0+Ccg5uhZXCg==
  dependencies:
    bn.js "^5.2.1"
    browserify-rsa "^4.1.0"
    create-hash "^1.2.0"
    create-hmac "^1.1.7"
    elliptic "^6.5.4"
    inherits "^2.0.4"
    parse-asn1 "^5.1.6"
    readable-stream "^3.6.2"
    safe-buffer "^5.2.1"

Run then a yarn install to fetch the latest package and you should have now crypto-browserify working with the browserify-sign version 4.2.2, which does not cause the issue anymore.

Please sign in to comment.