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

breaking changes in 4.2.3 #88

Closed
shamilovtim opened this issue Apr 20, 2024 · 8 comments
Closed

breaking changes in 4.2.3 #88

shamilovtim opened this issue Apr 20, 2024 · 8 comments

Comments

@shamilovtim
Copy link

shamilovtim commented Apr 20, 2024

  • crypto-browserify depends on this package transitively
  • this package shipped a breaking change in 4.2.3 by supporting node v1's APIs which modern projects don't support

please ship a major semver release for the 4.2.3 change and push a 4.2.4 that reverts the changes in 4.2.3.

@ljharb
Copy link
Member

ljharb commented Apr 20, 2024

Can you elaborate on the "breaking change"?

@shamilovtim
Copy link
Author

Can you elaborate on the "breaking change"?

There was a change made to intentionally support node v1 and this was transitively shipped into users projects. This was labeled as a patch release. We don't support node v1 nor do we intend to. This should not be a patch release.

@shamilovtim
Copy link
Author

Runtime crash is TypeError: Cannot read property 'slice' of undefined. We are pinning to 4.2.2 for now but cannot do that for downstream consumers of packages so a revert is needed here.

@ljharb
Copy link
Member

ljharb commented Apr 20, 2024

This is a node module; process always exists. The issue is that you’re using a bundler that is broken, and fails to provide process.env.

@shamilovtim
Copy link
Author

This is a node module; process always exists. The issue is that you’re using a bundler that is broken, and fails to provide process.env.

It worked before the patch release. The issue is that the published patch release to widen support to an ancient version of node broke modern node. Like many others I'm not interested in doing the labor to support that. Our users are on node 18+.

@ljharb
Copy link
Member

ljharb commented Apr 21, 2024

The breaking change was dropping older node versions; the fix was to restore them.

This has nothing to do with supporting old node versions, really - it's that metro is not properly bundling node modules by providing/shimming process. The only fix needed is to your RN build.

@ljharb ljharb closed this as not planned Won't fix, can't repro, duplicate, stale Apr 21, 2024
@shamilovtim
Copy link
Author

@ljharb The fix is clearly to remove or hard fork whatever you're maintaining since you lack judgement and respect for people's time. Toward that end I'm pulling crypto-browserify out of a dep and will be sure to audit the rest of our supply chain for these kinds of shenanigans since it's clear to me that you're doing this stuff as some sort of crusade.

@ljharb
Copy link
Member

ljharb commented Apr 21, 2024

@shamilovtim "going out of your way to remove a specific package" seems like a crusade to me :-)

I have plenty of respect for people's time - the amount of time wasted by bundlers failing to properly handle node builtins is massive, and tacitly endorsing it isn't helping anyone.

tillprochaska added a commit to alephdata/aleph that referenced this issue Jan 7, 2025
Version limited to <= 4.2.2 because 4.2.3 includes a breaking change: browserify/browserify-sign#88

The proper fix is to start using the WebCrypto API in the FtM TS lib so we don’t have to rely on `crypto-browserify` anymore.
tillprochaska added a commit to alephdata/aleph that referenced this issue Jan 7, 2025
Version limited to <= 4.2.2 because 4.2.3 includes a breaking change: browserify/browserify-sign#88

The proper fix is to start using the WebCrypto API in the FtM TS lib so we don’t have to rely on `crypto-browserify` anymore.
tillprochaska added a commit to alephdata/aleph that referenced this issue Jan 7, 2025
Version limited to <= 4.2.2 because 4.2.3 includes a breaking change: browserify/browserify-sign#88

The proper fix is to start using the WebCrypto API in the FtM TS lib so we don’t have to rely on `crypto-browserify` anymore.
tillprochaska added a commit to alephdata/aleph that referenced this issue Jan 8, 2025
* Bump axios

* Bump d3-color

* Bump multiple development dependencies / CRA

* Bump browserify-sign

Version limited to <= 4.2.2 because 4.2.3 includes a breaking change: browserify/browserify-sign#88

The proper fix is to start using the WebCrypto API in the FtM TS lib so we don’t have to rely on `crypto-browserify` anymore.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants