Skip to content

Improve introspection for SignalProducerWorkflow actions #192

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

Merged
merged 1 commit into from
Feb 23, 2023

Conversation

amorde
Copy link
Member

@amorde amorde commented Feb 22, 2023

This change improves the usability of #168 and also makes it easier to debug when inspecting arbitrary workflow actions.

Below is an example of the string interpolation result of a SignalProducerWorkflow action before and after this change:

func workflowDidReceiveAction<Action: WorkflowAction>(
    _ action: Action,
    workflow: Action.WorkflowType,
    session: WorkflowSession
) {
    print("\(action)")
}

// before
"AnyWorkflowAction<SignalProducerWorkflow<Output>>(_apply: (Function))"
// after
"Action(output: OrdersSDKImpl.UpcomingOrdersMutationWorker.Output.ordersDidMutate)"

The primary reason the output was not useful before was the usage of AnyWorkflowAction(sendingOutput:) which wraps the output value in a closure so it becomes inaccessible.

public init(sendingOutput output: WorkflowType.Output) {
self = AnyWorkflowAction { state in
output
}
}

Checklist

  • Unit Tests
  • UI Tests
  • Snapshot Tests (iOS only)
  • I have made corresponding changes to the documentation

@CLAassistant
Copy link

CLAassistant commented Feb 22, 2023

CLA assistant check
All committers have signed the CLA.

@amorde amorde marked this pull request as ready for review February 22, 2023 04:09
@amorde amorde requested a review from a team as a code owner February 22, 2023 04:09
@@ -47,18 +47,27 @@ struct SignalProducerWorkflow<Value>: Workflow {
public typealias State = Void
public typealias Rendering = Void

struct Action: WorkflowAction {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this seems like a good change to improve the utility of type-level introspection. do you think repeating the associated workflow's type in the action's type itself would be valuable? i.e. SignalProducerWorkflowAction or something. or putting that info into the string via 'custom string convertible' or whatever the appropriate conformance may be.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I did consider that yeah. I'll go ahead and make that change.

I also considered pulling a generic SendOutputAction out as a standalone type, but wasn't sure if that was worth it

@amorde amorde force-pushed the amorde/signalproducer-action-observability branch from 595664e to f29b3d3 Compare February 23, 2023 02:00
@amorde amorde merged commit dbdcb96 into main Feb 23, 2023
@amorde amorde deleted the amorde/signalproducer-action-observability branch February 23, 2023 02:10
jamieQ added a commit that referenced this pull request Feb 25, 2023
…ow-workflow-conformance

* origin/main:
  Expose AnyScreen.wrappedScreen for inspection (#193)
  Improve introspection for SignalProducerWorkflow actions (#192)
  [release]: bump version to 2.2.0 & remove separate concurrency version (#191)
  [feat]: add runtime observation API (#168)
  [chore]: refactor some internal actions to use existential any (#190)
  [fix]: use weak reference to internal sinks when vending to clients (#189)
  [feat]: add primary associated types to more protocols (#188)
  [chore]: pre major version bump cleanup (#187)
  [chore]: bump minimum deployment & swift versions (#186)
  Abstract ViewEnvironment to shared framework (#185)
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.

5 participants