Refactor BindingUtils to enable the use of custom bindings#1184
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## v7.0.0 #1184 +/- ##
=========================================
Coverage ? 87.97%
=========================================
Files ? 120
Lines ? 3784
Branches ? 0
=========================================
Hits ? 3329
Misses ? 455
Partials ? 0 ☔ View full report in Codecov by Sentry. |
|
@ueman I will review that after the holidays, thank you for doing this btw. |
| if (binding == null) { | ||
| return; | ||
| } |
There was a problem hiding this comment.
I believe this should stay, otherwise, it won't work anyway, so bail out early.
| late SentryWidgetsBindingObserver _observer; | ||
| late SentryFlutterOptions _options; |
There was a problem hiding this comment.
Better to keep it nullable, so if close is called before call which is wrong, wouldn't throw, it's defensive programming against API misusing.
| - theme | ||
| - Removed isolate name from Dart context. It's now reported via the threads interface. It can be enabled via `options.attachThreads` | ||
| - Use `sentryClientName` instead of `sdk.identifier` ([#1135](https://github.com/getsentry/sentry-dart/pull/1135)) | ||
| - Refactor `BindingUtils` to `BindingWrapper` to enable the use of custom bindings ([#1184](https://github.com/getsentry/sentry-dart/pull/1184)) |
There was a problem hiding this comment.
If this is a refactor, we don't need an entry here, it's more about user-facing changes.
There was a problem hiding this comment.
I intend this to be user facing, although maybe it shouldn't be advertised.
Co-authored-by: Manoel Aranda Neto <[email protected]>
3 participants
📜 Description
This change enables the use of a custom WidgetsBinding. It doesn't introduce a custom binding though.
This change is done for the v7 branch, since this could be considered a breaking change.
💡 Motivation and Context
I'm experimenting with initializing a custom widgets binding to add even more observability. Currently, that's not really supported. That's why I propose the following refactoring.
The new interface is modelled after the methods of the
WidgetsBindingSentry needs. By using a custom implementation of theBindingWrapperand assigning it to the options, you can make use of a custom binding. The custom binding then gets correctly initialized and used by Sentry.💚 How did you test it?
This is covered by existing tests.
📝 Checklist
🔮 Next steps