Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This is a draft to inspect the diff. More details to come 🚧
Problem
As it stands today,
AnalyticsClient
has a constraint forAnalyticsData
. While the cases inAnalyticsData
are consistent with most analytics vendors, they do not account for other data types. In particular, thecase event(name: String, properties: [String: String] = [:]
case does not allow forInt
or other base types. These cases are a solid foundation and are applicable to most projects; however, they are not always necessary.Theory/Hypothesis/Guess?
Is it possible to provide a similar api as it stands today while also allowing
AnalyticsClient
to be generic allowing the consumer of this package to define it's own analytic event types.Changes
This PR is created to share a proof of concept and a demo to make
AnalyticsClient
generic across multiple data types. It includes a protocolAnalyticClientProtocol
with anassociatedtype DataType
. This associated type is created by the consumer of the package. Rather than define theliveValue
forDependencyKey
inside ofswift-composable-analytics
this value is also provided via the package consumer. SeeExample/ExampleCore/AnalyticsClient/LiveAnalytics.swift
. Once setup, the library behavior is similar to what it was before.Previously,
AnalyticsReducer
was responsible for mapping Actions to anAnalyticsData
and then sent via theanalyticsClient
. Now, a Reducer is created via the output of a function declared as an extension forAnalyticClientProtocol
.This way we can constrain the reducer to
DataType
associated with the protocol. Similar logic has been used to replaceMultipleAnalyticsReducer
.A full demo can be found in the
Example
directory. There you can see multiple implementations of the new api and run tests for each.Motivation
I have an appreciation for this library! I found it while looking for options for a work project and was impressed with the api. The ability to spin up a quick reducer in a similar way made the bridge between feature code and analytic code easier to understand and maintain. However, I faced the problems above and was forced to make changes for my analytics provider. I'm hoping this PR can allow for more flexibility for more individuals or teams.
Moving Forward
If this is a direction that this library wants to head in there will be required updates to the
README.md
and the current implementation is lacking in docs and could benefit from robust testing. I think rather than merging this intomain
we could create abeta
branch or some equivalent in order to move forward without any disruption to themain
branch.