WP-3859 Make Events closable. Add EventsCollection base class. #73
Conversation
RavenNumber of Findings: 0 |
evanweible-wf
changed the title
evanweible-wf
force-pushed
the
disposable_events
branch
from
8a91146
to
d49bcd4
Compare
Codecov Report
@@ Coverage Diff @@
## master #73 +/- ##
==========================================
- Coverage 91.26% 90.03% -1.24%
==========================================
Files 3 4 +1
Lines 309 321 +12
==========================================
+ Hits 282 289 +7
- Misses 27 32 +5
Continue to review full report at Codecov.
|
|
+1 |
|
+1 |
|
+10 Things Tested
|
test/event_test.dart
Outdated
| expect(event.close(incorrectKey), throwsArgumentError); | ||
| }); | ||
|
|
||
| test('should now allow events to be dispatched after being closed', |
|
+1 |
|
@jayudey-wf ready for you |
|
QA +1
Merging into master. |
lib/src/event.dart
Outdated
| 'Event dispatch expected the "${_key.name}" key but received the "${key.name}" key.'); | ||
| 'Event dispatch expected the "${_key.name}" key but received the ' | ||
| '"${key.name}" key.'); | ||
| } | ||
| _sink.add(payload); |
There was a problem hiding this comment.
StreamController also defines the add() method.
There was a problem hiding this comment.
Fixed, using the _sink and _stream getters was unnecessary now that I'm storing the controller
|
As a followup, let's get the documentation updated : https://github.com/Workiva/w_module#events |
|
@dustinlessard-wf documentation updated, @jeroencranendonk-wf I addressed your comment, @trentgrover-wf I fixed the typo you caught |
|
+1 |
1 similar comment
|
+1 |
|
+10 Things Tested
|
|
QA +1
Merging into master. |
Issue
The
Eventclass offered no public API for disposing/closing, which means that the underlying StreamController could not be closed.Changes
Eventclosable via aclose()method similar toStreamController.close(), except that it is protected by theDispatchKey.EventsCollectionbase class for colocating relatedEventinstances that also makes disposal of said events easier via amanageEvent()method.LifecycleModuleclass to implement the latestDisposableManagerV3interface.Testing
Code Review
@Workiva/web-platform-pp
@Workiva/rich-app-platform-pp