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

BSON: For React Native please polyfill crypto.getRandomValues #3714

Closed
cristianoccazinsp opened this issue May 5, 2021 · 30 comments · Fixed by #3745
Closed

BSON: For React Native please polyfill crypto.getRandomValues #3714

cristianoccazinsp opened this issue May 5, 2021 · 30 comments · Fixed by #3745
Assignees

Comments

@cristianoccazinsp
Copy link

cristianoccazinsp commented May 5, 2021

Goals

Regular use.

Expected Results

Regular use.

Actual Results

A warning started appearing after upgrading to Realm 10.4.0.

JSWarning
BSON: For React Native please polyfill crypto.getRandomValues, e.g. using: https://www.npmjs.com/package/react-native-get-random-values.

    src/conf.js:594:34 
    node_modules/bson/dist/bson.browser.umd.js:2617:18 insecureRandomBytes
    node_modules/bson/dist/bson.browser.umd.js:5182:40 
    node_modules/bson/dist/bson.browser.umd.js:14:9 createCommonjsModule
    node_modules/bson/dist/bson.browser.umd.js:5175:37 
    node_modules/bson/dist/bson.browser.umd.js:2:72 
    node_modules/bson/dist/bson.browser.umd.js:5:2 
    node_modules/metro/src/lib/polyfills/require.js:322:6 loadModuleImplementation
    node_modules/realm/lib/extensions.js:54:35 
    node_modules/realm/lib/index.js:64:24 
    node_modules/metro/src/lib/polyfills/require.js:322:6 loadModuleImplementation
    src/db/index.js:8 

Steps to Reproduce

Just use Realm (offline)

Code Sample

Version of Realm and Tooling

  • Realm JS SDK Version: 10.4.0
  • Node or React Native: React Native
  • Client OS & Version: All
  • Which debugger for React Native: None
@cristianoccazinsp
Copy link
Author

Looks like this started to happen after BSON upgraded from "4.2.3" to "4.3.0" (dependency of Realm) which is odd since Realm locks the version to "^4.2.0"

@steffenagger
Copy link
Contributor

steffenagger commented May 6, 2021

@cristianoccazinsp very sorry for the confusion - it's an oversight on my part in the latest changes to the BSON package.
This warning was only suppose to trigger, if/when generating a new ObjectId (or other random ids, which was the main focus of the update), without a propper polyfill.

A PR has been dispatched and will hopefully be out soon: mongodb/js-bson#435

@cristianoccazinsp
Copy link
Author

@steffenagger thank you! Looking forward to the update.

Do you happen to know if we should still install the recommended dependency when using Realm? It would be great for warnings to only happen in DEV also.

@steffenagger
Copy link
Contributor

@cristianoccazinsp If you're generating ObjectId (or the upcoming UUID, part of #3605) on the device, then yes this would be the recommended (this will also be mentioned in the docs soon).

The reason being that a part of ObjectId, and most of UUID is random, and React Native does not (yet) expose any cryptographically strong random value generation. (as Node and newer browsers currently do).

@steffenagger
Copy link
Contributor

@cristianoccazinsp in regards to your comment here: mongodb/js-bson#435 (comment) the warning should disappear if you install https://www.npmjs.com/package/react-native-get-random-values (and follow the instructions - it has to be loaded).

This is not a solution, by any means, but could perhaps be better than having false error reporting, for the time being?
Just a suggestion.

@cristianoccazinsp
Copy link
Author

@steffenagger thanks for the suggestion! I would rather wait than adding a dependency that will be pretty much unused.

Would it break Realm if I downgrade bson to 4.2.x directly in the lock file?

@steffenagger
Copy link
Contributor

@cristianoccazinsp for 10.4.0... No actually I don't believe it would.

@mednche
Copy link

mednche commented Nov 25, 2021

@steffenagger, I'm using RealmJS 10.10.1 and I still get this warning message when I use new ObjectID() from bson. I've tried to install the suggested react-native-get-random-values and load it in my index.js but the warning was still occurring so I've now uninstalled it. What's the current situation on this issue?

@kneth
Copy link
Contributor

kneth commented Nov 26, 2021

@mednche

Which version of the bson package do you use?

Can you provide a small code snippet which can reproduce the issue?

@mednche
Copy link

mednche commented Nov 26, 2021

@kneth

It looks like I don't have the bson package installed in npm list. Should I have installed it separately from the one in realm dependency?

Currently this is how I turn an id into an ObjectID to write to MongoDB (I'm using React Native):

import { ObjectId } from "bson";
newDoc = {_id : new ObjectId(_id), otherProperty : 1};
// ... insert newDoc to DB here

@steffenagger
Copy link
Contributor

I've tried to install the suggested react-native-get-random-values and load it in my index.js but the warning was still occurring so I've now uninstalled it. What's the current situation on this issue?

@mednche did you rebuild the app, after adding react-native-get-random-values? That could result in the behaviour you're describing (I think).
It looks like bson@4.4.1 is bundled with realm@10.10.1, so there shouldn't be a need to install it separately.

I'm not working at Realm anymore, so I'd probably refer you to @kneth, if further investigation is needed :)

@kneth
Copy link
Contributor

kneth commented Nov 30, 2021

@mednche

My simple app (with the code from #3714 (comment)) doesn't give me a warning. Please run npm list --depth 1 bson.

If you create a simple app and install BSON (npm install bson@4.4.1) instead of Realm, do you then see the warning too?

@mednche
Copy link

mednche commented Dec 3, 2021

@kneth, thanks for your reply.

So, I've run npm list --depth 1 bson and I get:

realm@10.10.1
└── bson@4.4.1

I've created a simple app and installed BSON (with npm install bson@4.4.1) then added this line in App.js:
const a = new ObjectId() (also had to add import { ObjectId } from "bson") at the top of App.js.
I get the same warning BSON: For React Native please polyfill crypto.getRandomValues, e.g. using: https://www.npmjs.com/package/react-native-get-random-values.

@xtianmpimbaza
Copy link

@kneth, thanks for your reply.

So, I've run npm list --depth 1 bson and I get:

realm@10.10.1 └── bson@4.4.1

I've created a simple app and installed BSON (with npm install bson@4.4.1) then added this line in App.js: const a = new ObjectId() (also had to add import { ObjectId } from "bson") at the top of App.js. I get the same warning BSON: For React Native please polyfill crypto.getRandomValues, e.g. using: https://www.npmjs.com/package/react-native-get-random-values.

I am getting the same and realm doesnt write after that warning. I have tried everything in vain. Did u get a work around?

@mednche
Copy link

mednche commented Dec 20, 2021

@xtianmpimbaza, nope, still haven't found a way around it. In my case, it doesn't prevent Realm writes though.

@kneth, any idea why those warnings are showing?

@andrelomba86
Copy link

I put import 'react-native-get-random-values' before import Realm and the warning stopped.

import 'react-native-get-random-values'
import Realm from 'realm'

@dleunig
Copy link

dleunig commented Mar 4, 2022

Thx @andrelomba86! This approach solved my problem.

@Joshfindit
Copy link

For anyone else who's finding this thread while being new to Realm (and possibly React Native):

The total steps as of now are:

  1. npm install react-native-get-random-values
  2. npx pod-install
  3. Add import 'react-native-get-random-values'; early on, and before import Realm from 'realm' (For example: in index.js or App.js)
  4. Rebuild and reinstall the app
  5. Now test it. You should no longer see the warning BSON: For React Native please polyfill...

@Joshfindit
Copy link

I'm struggling to understand why this is not a dependency and therefore showing up as an error to people who are using Realm inside React Native.

As I understand it, Realm requires that the ID has a portion that's random. Therefore any time a BSON ID is generated it requires random values (which is why the error). The only time a dev would not run in to this is if they are not creating any items on-device. Personally that sounds like a very low-chance edge case. (Excepting the case above which seems like CI/CD testing)

Pros to making it a dependency:

  • New users will have an easier time
  • Devs will not have to wait until they call new ObjectId() to come across this requirement
  • Since the bson package is not superficially concerned with the randomness, it's not even mentioned on https://www.npmjs.com/package/bson which causes a research dead-end

Cons:

  • Are there performance issues?
  • React Native may implement a source of randomness in future and make this obsolete
  • bson may get the randomness by proxy through the uuid package? (Reference: UUID convenience class mongodb/js-bson#425 but that is merged and I still had to use react-native-get-random-values)

Personally as a new user I think there's a strong case, but there are a lot of things I don't know about React Native, Realm, or the goals of the realm-js package so YMMV.

@kraenhansen
Copy link
Member

kraenhansen commented Jun 2, 2022

@Joshfindit thanks for the detailed analysis! We'll discuss this on our team meeting next week and I'll report back.

@lnorton89
Copy link

@Joshfindit thanks for the detailed analysis! We'll discuss this on our team meeting next week and I'll report back.

What is the status on this? As its been almost six months others are waiting on how to handle this...

@adhamhassan99
Copy link

this issue is still present right ? I can see it has been detected way back and there has been action to solve but fast forward to now and I still get the same warning ? Am I doing something wrong ?

@kneth
Copy link
Contributor

kneth commented May 23, 2023

@adhamhassan99 Please create a new issue with all details for us to reproduce it.

@adhamhassan99
Copy link

@adhamhassan99 Please create a new issue with all details for us to reproduce it.

@kneth I just did #5841

@bloggerklik

This comment was marked as spam.

@kraenhansen
Copy link
Member

kraenhansen commented Feb 17, 2024

It seems this is causing confusing to quite a lot of people, although we believe the instructions are straight forward, the need for this might not be clear and it seems we never clarified this.

The reason we are not adding the polyfill as a dependency of our package is three-fold:

  1. In general, we consider it bad practice for a library to apply a polyfill to the environment, since another library might be doing the same, which could result in conflicts and incompatibility between the two libraries.
  2. Not all users of Realm use BSON ObjectId or UUID. We want to help our users keep bundle size as low as possible, which wouldn't be possible if we include it optimistically.
  3. Since the polyfill has a native component, it needs to be available for auto-linking and to my knowledge, the community CLI will only detect and auto-link direct dependencies of an app. So even if we wanted, it might not even be possible to include it.

If you you're experiencing the warning, you have a few options:

@bloggerklik
Copy link

@kraenhansen Thank you for your answer. What can I use instead of ObjectId or UUID? In my case it can only be an int value that can be unique. For example, a unique id with +1 more than the other, such as 1, 2, 3... will do the job. Or I can ignore the warning if it won't cause a problem as you stated.

@kraenhansen
Copy link
Member

What can I use instead of ObjectId or UUID?

Ints could work as primary key, but many just use a string. Some even generate a cryptographically strong UUID using another library and simply store it in a string property.

@bloggerklik
Copy link

@kraenhansen I don't need a structure that would require me to have a very strong id. But I want it to be practical. Is there a way I can increase it automatically? For example, is there a method that I can add as default without dealing with queries?
... primaryKey: "id", properties: { id: { type: "int", default: /* Highest value in current database + 1*/ }, } ...

@kraenhansen
Copy link
Member

@bloggerklik since Realm is designed to be easy to use when synchronising data between clients, we don't currently support an "auto-increment" default on ints. The risk of two clients picking the same id and getting into conflicts because of that seems like a "foot-gun" that we don't want to actively encourage.

This is however not an issue if you're local only and you could probably use the feature of passing a function as default:

// ... Be aware, I haven't actually tested this code 🙈
properties: {
  id: {
    type: "int",
    default: () => realm.objects("Thing").max("id") + 1,
  },
}

Anyways, we're outside of the scope of this issue and I'll lock it for now to avoid pinging people that might be watching it.
Please feel free to open a discussion if you want to talk more about the options available.

@realm realm locked as resolved and limited conversation to collaborators Feb 19, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging a pull request may close this issue.