Repository navigation
Conversation
|
🔗 Commit SHA: 7a70427 | Docs | View more details | Give us feedback! |
There was a problem hiding this comment.
Copilot review overview
🔵 Needs a closer look
Cross-package asynchronous lifecycle, weak-reference, and cancellation semantics warrant final human validation despite extensive coverage.
Review effort: Balanced
Findings: None
What changed in this PR
Adds retained first-install flag callbacks across Dart core and Flutter, including cancellation, documentation, examples, and comprehensive tests.
Changes:
- Adds immutable flag events and an event facade backed by weak client associations.
- Implements one-shot asynchronous delivery and cancellation in core and Flutter.
- Updates examples, documentation, and lifecycle/GC test coverage.
| File | Description |
|---|---|
packages/datadog_flags/lib/datadog_flags.dart |
Exports event APIs. |
packages/datadog_flags/lib/datadog_flags_internal.dart |
Exports the companion-package bridge. |
packages/datadog_flags/lib/src/default_flags_client.dart |
Registers core event delivery. |
packages/datadog_flags/lib/src/flags_client.dart |
Clarifies initialization documentation. |
packages/datadog_flags/lib/src/flags_client_event.dart |
Defines immutable event data. |
packages/datadog_flags/lib/src/flags_event_registry.dart |
Adds weak event-source associations. |
packages/datadog_flags/lib/src/flags_events.dart |
Adds the public facade and extension. |
packages/datadog_flags/lib/src/flags_repository.dart |
Retains and delivers the first event. |
packages/datadog_flags/lib/src/no_op_flags_client.dart |
Supports no-op registration. |
packages/datadog_flags/test/flags_events_compatibility_test.dart |
Tests legacy-client compatibility. |
packages/datadog_flags/test/flags_client_event_test.dart |
Tests event immutability and type mapping. |
packages/datadog_flags/test/first_flags_registration_test.dart |
Tests registration and cancellation. |
packages/datadog_flags/test/first_flags_callback_test.dart |
Tests installation callback behavior. |
packages/datadog_flags/test_vm/helpers/force_gc.dart |
Adds VM garbage-collection support. |
packages/datadog_flags/test_vm/first_flags_capture_test.dart |
Tests capture release and facade lifetime. |
packages/datadog_flags/example/bin/typed_evaluation.dart |
Demonstrates first-install callbacks. |
packages/datadog_flags/example/test/typed_evaluation_test.dart |
Tests the CLI example. |
packages/datadog_flags/example/pubspec.yaml |
Adds example test dependencies. |
packages/datadog_flags/README.md |
Documents the core API. |
packages/datadog_flags/CHANGELOG.md |
Records the core feature. |
packages/datadog_flags_flutter/lib/datadog_flags_flutter.dart |
Re-exports event APIs. |
packages/datadog_flags_flutter/lib/src/datadog_flags_plugin.dart |
Forwards registrations to core. |
packages/datadog_flags_flutter/test/helpers/first_flags_test_client.dart |
Adds a wrapper test factory. |
packages/datadog_flags_flutter/test/flags_events_compatibility_test.dart |
Tests Flutter compatibility. |
packages/datadog_flags_flutter/test/first_flags_registration_test.dart |
Tests forwarding and cancellation. |
packages/datadog_flags_flutter/test_vm/helpers/force_gc.dart |
Adds Flutter VM GC support. |
packages/datadog_flags_flutter/test_vm/first_flags_capture_test.dart |
Tests wrapper capture release. |
packages/datadog_flags_flutter/example/lib/main.dart |
Demonstrates callback lifecycle handling. |
packages/datadog_flags_flutter/example/test/first_flags_example_test.dart |
Tests the Flutter example. |
packages/datadog_flags_flutter/example/pubspec.yaml |
Adds example test dependencies. |
packages/datadog_flags_flutter/README.md |
Documents Flutter behavior and release coordination. |
packages/datadog_flags_flutter/CHANGELOG.md |
Records the Flutter feature. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
sameerank
left a comment
There was a problem hiding this comment.
Codex found one correctness issue but I assume it's non-blocking. Doesn't sound common to register and initialize in different Dart zones
The base branch was changed.
leoromanovsky
left a comment
There was a problem hiding this comment.
Didn't find any actionable defects.
Applications can register for the first accepted flags installation directly on the flags client, including after that installation has completed.
Each registration receives the retained first event once and returns an idempotent unregister function. The callback receives only
FlagsClientEvent, withtypeand nullable immutableflagsChanged; the implemented type isCONFIGURATION_CHANGED. Delivery always runs in a microtask in the registration zone. Cancellation releases captures and suppresses delivery until the callback starts, including while the Flutter delegate is resolving. The event contains all keys of the first accepted configuration, not one notification per flag or all flags on the server. Valid empty configurations notify with[]; rejected/failed/undecodable loads do not consume the signal. Registration and retained-event replay perform no SDK I/O. Evaluations read current assignments, not a snapshot pinned to the event; existing failed-fetch fallback behavior is unchanged.The API is a member of
DatadogFlagsClient, implemented directly by the core, no-op, and Flutter clients. There is no events facade, extension, registry, or runtime capability lookup. The event constructor is annotated@internal: it is excluded from generated API docs and external construction receives an analyzer warning. It remains in its existing library; there is no new public builder/factory. The copiedList.unmodifiablepayload is preserved.This is a source-breaking interface addition: custom implementations and test doubles must implement the method; decorators can forward it to their delegate. The SDK no-op client accepts registrations without emitting events.
The real CLI example registers with:
The real Flutter app registers in
initStateand unregisters indispose:Flutter forwards each registration once to the resolved core client. Registrations do not migrate across SDK re-enable; applications reacquire the shared client. Cache admission, CACHED provenance, evaluation telemetry, retained-event ownership, and exception isolation remain unchanged. Callback logging and failed-fetch retention changes are outside this PR.
CACHED #1203 is merged; this PR targets develop. The wrapper requires
datadog_flags: ^1.2.0for CACHED support. Before publishing the integration, publish core containing this direct API and ensure the wrapper minimum selects that release. Local path overrides prove companion-source compatibility, not registry availability. Release versions remain coordinated separately.Validation on the direct-client implementation: 157 tests passed (125 core, 24 Flutter wrapper including actual-core forwarding/RUM integration, four forced-GC capture-release tests, two CLI example tests, and two actual Flutter app tests). Both packages and both examples analyze cleanly; 56 Dart files pass formatting. Both packages generate API documentation with zero warnings/errors and no public event constructor. External probes through both barrels confirm
invalid_use_of_internal_memberfor construction and clean analysis for callback consumption. Core requiresmeta: ^1.3.0, where@internalwas introduced. The CLI executable/help and Flutter debug bundle build pass. Coverage includes isolated failed-load/valid-empty paths, cache-empty delivery before network completion, defensive source aliasing, retained keys versus current values after successful/failed context updates, replay with no fetch/persistence I/O, reentrant progress, synchronous Exception/Error/other thrown-object isolation, zone-value/async-error routing, microtask ordering, and independent unregister. Custom implementor fixtures compile through both public barrels. Facade-only tests were removed with the facade; callback capture-release coverage remains.The Flutter app tests use a dummy token and mocked HTTP. Native-device execution, live Datadog behavior, and current-head remote CI are not established by these local checks.