Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Draft] Feature/generic event types #7

Draft
wants to merge 8 commits into
base: main
Choose a base branch
from

Conversation

willbrandin
Copy link

@willbrandin willbrandin commented Feb 23, 2024

This is a draft to inspect the diff. More details to come 🚧

Problem

As it stands today, AnalyticsClient has a constraint for AnalyticsData. While the cases in AnalyticsData are consistent with most analytics vendors, they do not account for other data types. In particular, the case event(name: String, properties: [String: String] = [:] case does not allow for Int 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 protocol AnalyticClientProtocol with an associatedtype DataType. This associated type is created by the consumer of the package. Rather than define the liveValue for DependencyKey inside of swift-composable-analytics this value is also provided via the package consumer. See Example/ExampleCore/AnalyticsClient/LiveAnalytics.swift. Once setup, the library behavior is similar to what it was before.

Previously, AnalyticsReducer was responsible for mapping Actions to an AnalyticsData and then sent via the analyticsClient. Now, a Reducer is created via the output of a function declared as an extension for AnalyticClientProtocol.

public extension AnalyticClientProtocol {
    @inlinable
    func reduce<State, Action>(
        _ toAnalyticsData: @escaping (State, Action) -> DataType?
    ) -> some Reducer<State, Action> {
        Reduce<State, Action> { state, action in
            guard let event = toAnalyticsData(state, action) 
            else { return .none }
            
            return .run { _ in
                self.sendAnalytics(event)
            }
        }
    }
}

This way we can constrain the reducer to DataType associated with the protocol. Similar logic has been used to replace MultipleAnalyticsReducer.

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 into main we could create a beta branch or some equivalent in order to move forward without any disruption to the main branch.

Shamelessly used GPT for some of these. I made some tweaks but some adjustments may be necessary.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant