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

Not working with expo #468

Open
2ico opened this issue Feb 20, 2024 · 22 comments
Open

Not working with expo #468

2ico opened this issue Feb 20, 2024 · 22 comments

Comments

@2ico
Copy link

2ico commented Feb 20, 2024

Expo SDK: 49.0.0
After following the instructions at https://github.com/ai/nanoid#react-native , I get:

Android Bundling failed 403ms
The package at "node_modules/nanoid/index.js" attempted to import the Node standard library module "node:crypto".
It failed because the native React runtime does not include the Node standard library.
@ai
Copy link
Owner

ai commented Feb 20, 2024

@2ico
Copy link
Author

2ico commented Feb 20, 2024

Yes, I did

@ai
Copy link
Owner

ai commented Feb 20, 2024

Sorry, I have no idea or experience with Expo.

Try to ask Expo community.

@2ico
Copy link
Author

2ico commented Feb 20, 2024

I think the issue is here:
https://github.com/ai/nanoid/blob/61a087715a21d971dc9154c5726c818db0e42b27/index.js#L1C1-L1C50
The module imports the entire webCrypto from node:crypto.

In version 3 only the required function getRandomBytesAsync was imported:

let { getRandomBytesAsync } = require('expo-random')

@ai
Copy link
Owner

ai commented Feb 20, 2024

Nope, those are different files. Sync and async.

@2ico
Copy link
Author

2ico commented Feb 20, 2024

Async was removed in v5, right?

@ai
Copy link
Owner

ai commented Feb 20, 2024

Yes

@2ico
Copy link
Author

2ico commented Feb 20, 2024

So what is the status for expo compatibility?
I see someone was using expo here: #415

@ai
Copy link
Owner

ai commented Feb 20, 2024

Yes, it should work. This doc was written by Expo user.

@2ico
Copy link
Author

2ico commented Feb 20, 2024

Can I contact them?

@ai
Copy link
Owner

ai commented Feb 20, 2024

I don’t think it is polite. You should use StackOverflow and similar things if you want ask others to do your work.

Try also import index.browser.js version. Maybe react-native-get-random-values polyfill has issue with import.

@2ico
Copy link
Author

2ico commented Feb 20, 2024

I don't think it's impolite to contact the maintainer of a library.

@ai
Copy link
Owner

ai commented Feb 20, 2024

I am the maintainer. You want to contact some other developer who send PR to the library.

@2ico
Copy link
Author

2ico commented Feb 20, 2024

Anyway, how can I do this?

Try also import index.browser.js version. Maybe react-native-get-random-values polyfill has issue with import.

Could not find a declaration file for module 'nanoid/index.browser'. '/home/george/Documents/GAIO/expo/archie-expo/node_modules/nanoid/index.browser.js' implicitly has an 'any' type.
  Try `npm i --save-dev @types/nanoid` if it exists or add a new declaration (.d.ts) file containing `declare module 'nanoid/index.browser';`

@ai
Copy link
Owner

ai commented Feb 20, 2024

This is just a type warning not an error.

Does it works?

@2ico
Copy link
Author

2ico commented Feb 20, 2024

It works, with @ts-ignore

@ai
Copy link
Owner

ai commented Feb 20, 2024

Try the new Nano ID 5.0.6 with normal import { nanoid } from 'nanoid

@2ico
Copy link
Author

2ico commented Feb 20, 2024

It doesn't work, unfortunately. It's still using './index.js'.
Thanks for the quick attempt!

@2ico
Copy link
Author

2ico commented Feb 20, 2024

And I guess the release should include a ./index.browser.d.ts file to solve the error I got with TS.

@ai
Copy link
Owner

ai commented Feb 20, 2024

And I guess the release should include a ./index.browser.d.ts file to solve the error I got with TS.

Separeted manual import for browser version is not a long-term solution because you can't change import inside other libraries.

It was only a test.

@steida
Copy link

steida commented Apr 28, 2024

I tried this patch but without a success

diff --git a/package.json b/package.json
index 0d10ada4dce6c02a339399e71c82860c70f3d12b..4fb2ab8de9093eae1bc590cd738019e5b40d0dfb 100644
--- a/package.json
+++ b/package.json
@@ -24,6 +24,7 @@
   "exports": {
     ".": {
       "browser": "./index.browser.js",
+      "react-native": "./index.browser.js",
       "default": "./index.js"
     },
     "./non-secure": "./non-secure/index.js",

Why RN doesn't pick browser.js is a mystery.

@danielsht86
Copy link

In case anyone is still running into this and this is helpful. We had this problem and had to do 2 separate things to get crypto resolved correctly in different situations:

  1. We had to do the metro.config.js adjustment to properly resolve the module in case it was being explicitly imported:
config.resolver.resolveRequest = (context, moduleName, platform) => {
  if (moduleName === 'crypto' || moduleName === 'node:crypto') {
    // when importing crypto, resolve to react-native-quick-crypto
    return context.resolveRequest(context, 'react-native-quick-crypto', platform);
  }
  // otherwise chain to the standard Metro resolver.
  return context.resolveRequest(context, moduleName, platform);
};

(note, we added the node:crypto clause in case it mattered for some imports, although we never confirmed this was helpful for any particular case.

  1. We also had to override global.crypto since index.browser.js was being imported by default in our case
import { install } from 'react-native-quick-crypto';
install();

With both of these, we covered all our bases

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

4 participants