Improve default logger for realm/react

[takameyer]

What, How & Why?

The default logger could be a bit smarter. This will print a contextual log message with the level and use either console.error, console.warn and console.log based on the relevant level. This also sets a default log level to WARN.

☑️ ToDos

• 📝 Changelog entry
• 📝 Compatibility label is updated or copied from previous entry
• 📝 Update COMPATIBILITY.md
• 🚦 Tests
• 🔀 Executed flexible sync tests locally if modifying flexible sync
• 📦 Updated internal package version in consuming package.jsons (if updating internal packages)
• 📱 Check the React Native/other sample apps work if necessary
• 📝 Public documentation PR created or is not necessary
• 💥 Breaking label has been applied or is not necessary

If this PR adds or changes public API's:

• typescript definitions file is updated
• jsdoc files updated

[kraenhansen]

Why not add this to the realm package directly? I think a good default logger would be valuable more generally.

[takameyer]

Why not add this to the realm package directly?

I've wanted to do this for a while, so I have no argument against not doing it directly.
Also, I'm pretty sure @papafe is going to do this with the unified logger. This is to help people using React Native and Realm today.

[kraenhansen]

I see. If @papafe is already solving this for v12 with the unified logger, (btw, what issue is is tracking that?), I'd suggest holding back on this, as we want to be sure loading @realm/react won't override the default he'll be implementing, right?

[takameyer]

@realm/react won't override the default

The unified logger will be at a different entry point and this would turn into a no-op. So this will just be backwards compatible. I want to still ensure v11 users get the advantages of the @realm/react updates.

what issue is is tracking that?

Unified Logger Issue

[kraenhansen]

As I read the last comment on the unified logger issue, the Realm.App.Sync.setLogger will change to call Realm.setLogger and with this PR merged @realm/react will override whatever default the realm package injects on import, right? What am I missing? To me this change will interfere with the future default behavior of realm@v12.

[takameyer]

@kraenhansen How would you recommend we support both versions of logging? With this change, users wouldn't even have to update realm to get default logger. After unified logging is realised, we can make this support both.

Realm.App.Sync.setLogger will change to call Realm.setLogger

This will be a breaking change then right? We should probably make unified logging a v13 change if Realm.App.Sync.setLogger doesn't work anymore.

[kneth]

My comment is only minor (mostly enhancing readability) and can be ignored

[kneth]

Why not use an object literal? And let it default to DEFALT_LOG_LEVEL?

[elle-j]

I like the split off the various console methods! (Apologies for late review, this went under my radar)

[elle-j]

NumericLogLevel.All and NumericLogLevel.Off as well 👍

[takameyer]

I may be closing this, as the effort @papafe is currently doing will probably nullify this, as well as the logLevel and logger properties of the AppProvider. Part of me does want to release this, just so people that haven't updated to v12 can reap the benefits, although I'm unsure if this comes into conflict with the recent core versions.