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

[chore]: refactor some internal actions to use existential any #190

Merged
merged 1 commit into from
Feb 7, 2023

Conversation

jamieQ
Copy link
Contributor

@jamieQ jamieQ commented Feb 7, 2023

Description

since we now have access to primary associated types, there are a couple places internally where it likely makes sense to leverage the language's 'existential' boxing facilities over our own. to start, we'll update a couple places that pass an AnyWorkflowAction<T> that can now be replaced with any WorkflowAction<T>.

additional comments inline.

Checklist

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


/// 'Opens' the existential `any WorkflowAction<WorkflowType>` value
/// allowing the underlying conformance to be applied to the Workflow's State
func openAndApply<A: WorkflowAction>(
Copy link
Contributor Author

Choose a reason for hiding this comment

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

it's not entirely clear to me why this is necessary, or exactly why it works, but if you try to invoke action.apply(...) on the any WorkflowAction variable, you get a compiler error. this makes some amount of sense b/c an any WorkflowAction isn't itself a WorkflowAction. i did skim this and this to see if there are other options, but this was the only thing i've been able to get working thus far.

@@ -272,7 +272,7 @@ extension WorkflowNode.SubtreeManager {

fileprivate final class ReusableSink<Action: WorkflowAction>: AnyReusableSink where Action.WorkflowType == WorkflowType {
func handle(action: Action) {
let output = Output.update(AnyWorkflowAction(action), source: .external)
let output = Output.update(action, source: .external)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

this change will help the observation API be more useful as we will be able to expose the underlying conformance and not an AnyWorkflow wrapper

@jamieQ jamieQ marked this pull request as ready for review February 7, 2023 20:11
@jamieQ jamieQ requested a review from a team as a code owner February 7, 2023 20:11
@jamieQ jamieQ merged commit f9f5494 into main Feb 7, 2023
@jamieQ jamieQ deleted the jquadri/existential-actions branch February 7, 2023 21:34
jamieQ added a commit that referenced this pull request Feb 7, 2023
…rvers

* origin/main:
  [chore]: refactor some internal actions to use existential any (#190)
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.

2 participants