feat: expose helpers for static models #311
Conversation
There was a problem hiding this comment.
These are nice additions!
Two comments:
-
If I could bike-shed for a moment,
staticfeels close, but also ambiguous due to the overloaded meaning in Swift. Hoping there might be a more explicit name. Some ideas: noOp, noOperation, identity, ignored, ignore, ignoringChanges/ignoringActions, immutable. -
Noticed this warning (Worker.swift, line 77):
passing argument of non-sendable type 'WorkerType.Output' into main actor-isolated context may introduce data races
We should mark
OutputasSendablesoon.
| } | ||
| } | ||
|
|
||
| extension Store { | ||
| public extension Store { |
There was a problem hiding this comment.
Please keep the access control on the individual methods.
There was a problem hiding this comment.
I agree, but the linter did this!
| @@ -5,6 +5,11 @@ | |||
| public struct ActionModel<State: ObservableState, Action>: ObservableModel, SingleActionModel { | |||
| public let accessor: StateAccessor<State> | |||
| public let sendAction: (Action) -> Void | |||
|
|
|||
| public init(accessor: StateAccessor<State>, sendAction: @escaping (Action) -> Void) { | |||
There was a problem hiding this comment.
are there any reasons we'd want to restrict clients' ability to create these types in general?
There was a problem hiding this comment.
To reduce the chance of folks using it wrong, mainly. You can't create something like a RenderContext either. The type's docs already hint at this, and I can copy it to the init as well.
There was a problem hiding this comment.
to be clear – you think the convenience of exposing the public constructor outweighs the chance of misuse?
| public extension ActionModel { | ||
| /// Creates a static model which ignores all sent values, suitable for static previews | ||
| /// or testing. | ||
| static func `static`(state: State) -> ActionModel<State, Action> { |
There was a problem hiding this comment.
per @g-mark's naming thoughts – this form of utility could perhaps be named 'just' if we want to lean into Combine's parlance
There was a problem hiding this comment.
Or follow Binding.constant(_:)'s lead.
| public extension ActionModel { | ||
| /// Creates a static model which ignores all sent values, suitable for static previews | ||
| /// or testing. | ||
| static func `static`(state: State) -> ActionModel<State, Action> { |
There was a problem hiding this comment.
Or follow Binding.constant(_:)'s lead.
watt
force-pushed
the
awatt/observablescreen-static-helpers
branch
from
b3d0dd3
to
13631b0
Compare
watt
force-pushed
the
awatt/observablescreen-static-helpers
branch
from
13631b0
to
d3b1e4b
Compare
Changed to
Feel free to take that on! I think it might take some integration work since it'll affect a lot of types. |
On it! |
Creating a static model is useful for testing, and the missing
public initonActionModelmakes that difficult to do. This PR adds that init along with a couple static factory methods for that use case.Checklist