pax-k / rn-async-storage-level

⚡️ An abstract-level implementation of AsyncStorage for React Native using TypeScript
3 stars 1 forks source link

Polyfills should be set in the user's project not in this package #1

Open shamilovtim opened 1 year ago

shamilovtim commented 1 year ago

Based on the conversation I saw in Discord I just wanted to call out that this repo shouldn't set any polyfills like btoa/atob, textencoder, and so on for the user of the package. It's fine to use those inside the example project, but they should not be set in the code of the package. For example you're providing buffer in dist/main.js but you should actually be relying on the user's global.buffer (or lack thereof, and then throw).

Polyfills can be handled as a matter of documentation, for example forewarning the user that they will need those polyfills and providing example code for how to use the polyfills.

In our react native projects at TBD we're using react-native-quick-crypto , react-native-quick-base64,fastestsmallesttextencoder, react-native-bignumber, @craftzdog/react-native-buffer, and stream-browserify. We are avoiding browserify packages in general, and JS based polyfills where possible. A native C++ implementation seems lacking for streams and text encoding in the community, which we hope can be addressed by us or the community at a later point.

pax-k commented 1 year ago

@shamilovtim Thank you for this!

Means I have to remove those polyfills, remove browserify, and let the user provide its own global polyfills, else throw errors. Can you please point me to a TBD repo where you're doing react-native stuff?

As for the missing C++ modules, maybe this is a starting point:

Unfortunately I don't have time to pursue this, but would be fun to try and pick it up when I can

shamilovtim commented 1 year ago

@pax-k we don't have the authorization to open that repo yet but I can get you anything you need code wise if you DM me on Discord