logging: Add rn warning and logging #798
Conversation
Eric-Warehime
requested review from
winder,
tzaffi,
jasonpaulos,
shiqizng and
algochoi
src/logging.ts
Outdated
| /** | ||
| * setLogLevel adjusts the minimum level of logs which are output by the SDK's logger | ||
| */ | ||
| export function setLogLevel(level: number) { |
There was a problem hiding this comment.
Is this somehow being called implicitly somewhere?
There was a problem hiding this comment.
Reading your PR description more carefully I see the purpose now. Perhaps we have enough static checks to see that this function is well formed. Regardless, a unit test can act as a sanity check.
There was a problem hiding this comment.
It's not being called anywhere, it's set that users can set the log level to turn off logs if they don't want to see a certain level.
There was a problem hiding this comment.
My only concern is whether or not we should document this behavior somehow. I guess the log is to make it self-documenting so I'm also OK with it as-is. I don't have any opinion on the logging library, but it seems like a sensible approach to providing this hint to the developers.
There was a problem hiding this comment.
Looks good. Just wary of adding a logging specific package when logging isn't prevalent in the app to begin with. I think this is the only place (outside maybe some examples) that a log function is called. Maybe worth just switching to console.warn() until there's a specific need for more custom sdk-wide logging. (unless there are plans in future PR's to introduce a lot more logging)
|
@Blackglade Yeah I can change to a console log. My inclination is to introduce more logging in the future, but if it's not something that people want/need then yeah minimizing the dependencies is best. |
Adds a log when we detect we're running in react-native which tells users they will need to polyfill crypto libraries.
The react-native project does not (and probably won't) provide any crypto functionality facebook/react-native#5049 so users need to polyfill things like
crypto.getRandomValuesif they want to do address generation.I've added a convenience function to determine if we're in react-native ( see facebook/react-native@e966cd1 )
Also added
tslogto provide logging cross platform and an exported function for users to adjust the log level if they don't want to see any logs.