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

Cannot read property 'slice' of undefined #242

Closed
Tracked by #1
adrianofoschi opened this issue Mar 6, 2024 · 24 comments
Closed
Tracked by #1

Cannot read property 'slice' of undefined #242

adrianofoschi opened this issue Mar 6, 2024 · 24 comments

Comments

@adrianofoschi
Copy link

adrianofoschi commented Mar 6, 2024

Installed on new app created with Expo 50.
I am having this errors. Slice seems related to stream. Maybe I am missing something?

 ERROR  TypeError: Cannot read property 'slice' of undefined, js engine: hermes
 ERROR  Invariant Violation: "main" has not been registered. This can happen if:
* Metro (the local dev server) is run from the wrong folder. Check if Metro is running, stop it and restart it in the current project.
* A module failed to load due to an error and `AppRegistry.registerComponent` wasn't called., js engine: hermes

I tried with 'stream': 'stream-browserify' inside babel config and also patching cipher-base and hash-base replacing stream with readable stream. The error is always the same.

@grenos
Copy link

grenos commented Mar 6, 2024

Same here, Android is crashing on starup. This actually comes from crypto-browserify "^3.12.0" as it is a dependency of this library and browserify-sign where the error originates from, is a dependency of crypto-browserify

images showing the error reported by Metro and the file path to the actual library

Screenshot 2024-03-06 at 23 02 37 Screenshot 2024-03-06 at 23 03 08 Screenshot 2024-03-06 at 23 03 37

EDIT Same issue on iOS.

**EDIT 2 : ** I think this might be the MR on Browserify-sign that' causing the issue

@adrianofoschi
Copy link
Author

I opened an issue in browserify-sign
browserify/browserify-sign#86

In the meantime can we try to downgrade this dependency?

@grenos
Copy link

grenos commented Mar 7, 2024

A quick fix although a dirty one:

  1. Add the following patch browserify-sign+4.2.3.patch

  2. Good clean rm -rf node_modules && rm -rf yarn.lock && rm -rf android/app/build && cd ios && pod deintegrate && rm -rf Podfile.lock && cd ..

  3. Run yarn install && cd node_modules/browserify-sign && yarn install

  4. Go to your index.js / app.ts and add at the top of the file:

global.process = process || {}
global.process.browser = false

@beqramo
Copy link

beqramo commented Mar 12, 2024

simpler solution in package.json:

	"resolutions": {
		"browserify-sign": "4.2.2" 
	},

@nikhil-kataria
Copy link

nikhil-kataria commented Mar 13, 2024

@beqramo can you share exact steps? I have added the above snippet in package.json but still getting the same issue.

@beqramo
Copy link

beqramo commented Mar 13, 2024

@nikhil-kataria remove node modules and yarn.lock and run yarn again

@hanyufoodles
Copy link

To resolve stream to readable-stream I made it work by adding it in extraNodeModules options of the metro resolver config:

resolver: {
  extraNodeModules: {
    stream: require.resolve('readable-stream'),
  },
}

@huangmengjiao-hmj
Copy link

Does anyone have a solution?

@ApplyFrank
Copy link

@beqramo what do you mean by remove node js? If i'm using npm can I just delete package-lock.json ?

@ApplyFrank
Copy link

@grenos followed your steps but instead of yarn I used npm, still doesn't work, any insights you found when solving this ?

@grenos
Copy link

grenos commented Apr 3, 2024

@ApplyFrank

I ended up using @beqramo solution as mine was a quick hack at the time so I can continue working.

This is how it looks with yarn

    "resolutions": {
        "browserify-sign": "4.2.2"
    },

I think with npm you need to replace resolutions with overrides

check out this link

@ApplyFrank
Copy link

@grenos thank you so much, overrides in package.json works! Saved me so much frustration thank you for your quick reply !!!

@ospfranco
Copy link
Collaborator

@mrousavy @Szymon20000 This one is important, I just tried updating an app to the new arch and this started failing.

@mrousavy
Copy link
Member

I haven't encountered this yet, but if you have a fix for that that works in your app already could you submit a PR? we can get this merged and pushed asap. @ospfranco

@ospfranco
Copy link
Collaborator

The author of the commit refused to revert his change. It was done to keep compatibility with Node 4.

I do not have a PR, I solved by adding the resolution field into my package.json to force a different version of the subdependency:

"resolutions": {
	"browserify-sign": "4.2.2" 
},

This package might also be worth a shot:

https://www.npmjs.com/package/process

This one could be included as a dependency of quick-crypto and imported directly on the index.ts so that users don't have to manually install it. I don't have that much time to test and send a PR though. So leaving it for someone here to open a PR.

@Red3nzo
Copy link

Red3nzo commented Apr 15, 2024

Ran into the same issue as @adrianofoschi looked for hours thinking it was my HDKey dep but ended up being react-native-quick-crypto. @beqramo fix ended up compiling the application properly

@shamilovtim
Copy link
Contributor

This was resolved by #264

@shamilovtim
Copy link
Contributor

shamilovtim commented Apr 22, 2024

I would encourage folks to take a hard look at how much exposure they have to the projects ljharb has wormed his way into maintaining given how many times I have seen him pull these shenanigans on users. In this case thousands of people's projects were broken because he demanded we support Node v1 and pre-release Node RC versions.

Alternatives:
https://biomejs.dev/
https://github.com/antfu/eslint-plugin-import-x

@shubhamdeol
Copy link

I had to add below code into my package json to resolve above issue.
adding browserify-sign in resolutions did not fix the issue for me. This may be due to other peer dependencies using different version of readable-steam.

So if anyone face this issue can opt for my solution.
This would be really better if library resolves the issue instead of the workaround. No one uses node 3 with react native

"resolutions": {
"readable-stream": "^3.6.2"
}

@ljharb
Copy link

ljharb commented Apr 24, 2024

That’s not a fair characterization. The source of the breakage is that metro doesn’t provide process. My patch to fix a breaking change simply exposed it.

I’m looking into an alternative solution which wouldn’t require any of these workarounds, but in the meantime, i suggest you lobby metro to properly bundle node modules.

@boorad
Copy link
Collaborator

boorad commented Apr 26, 2024

@shamilovtim can we close this issue?

@shamilovtim
Copy link
Contributor

Yeah good to close

@castdrian
Copy link

at least push a release lol, having to pin a commit on the main branch is a bit suboptimal

@shamilovtim
Copy link
Contributor

at least push a release lol, having to pin a commit on the main branch is a bit suboptimal

It's fixed in the latest release candidate on npm (I think it's 0.7.0-rc2)

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