@cycle/state Reducer to expect defined state?

[mightyiam]

cyclejs/state/src/types.ts

Line 2 in 313762d

export type Reducer<T> = (state: T | undefined) => T | undefined;

Is this exported Reducer meant to be imported and used in apps? I find that in my app I would like to have a reducer type where state is always defined:

type Reducer<T> = (state: T) => T

Could we have a separate API for instantiating state, somehow, so that a Reducer could assume positive existence of state?

[jvanbruegge]

I am actually currently working on the new isolate + state and yes this is a major pet peeve of mine. But I have no idea how to fix that, I would be very glad for ideas

[mightyiam]

Awesome. Keep up the good work. The idea I have is to separate initialization of state from reducing of state. That way a reducer can assume initialized state.

It seems that the only place where initialization of state could occur in that case is at the call to withState.

So perhaps

withState(main, initial, name)

This raises the question whether it is appropriate for the state of a component in this paradigm to be initialized by its parent rather than by itself.

I would say that it is.

[staltz]

These reducers are defined by your app and composed in streams, so that you know the order of emissions of those reducers in the stream reducer$ that gets passed to the sink. So all you need to do is put the initialReducer in a startWith operator, and then by definition all the other reducers can assume that the prev: State argument is defined.

[jvanbruegge]

The problem is more about the types I think, how can we make the stateSource not return a possibly undefined state

[staltz]

Hmm, but does that happen? I don't recall having to do state$.filter(isDefined). StateSource is a generic on S, with MemoryStream<S>

[mightyiam]

These reducers are defined by your app and composed in streams, so that you know the order of emissions of those reducers in the stream reducer$ that gets passed to the sink. So all you need to do is put the initialReducer in a startWith operator, and then by definition all the other reducers can assume that the prev: State argument is defined.

That all pertains to runtime. This issue is regarding the TypeScript types.

The problem is more about the types I think, how can we make the stateSource not return a possibly undefined state

I'm not sure what the challenge in that is. If an initialized state is required in withState then it can be assumed that the state source will always be defined.

Hmm, but does that happen? I don't recall having to do state$.filter(isDefined). StateSource is a generic on S, with MemoryStream<S>

Perhaps strictFunctionTypes: false allows that?

[staltz]

If an initialized state is required in withState

That's not going to happen for a tiny issue such as this one. Initial state is application logic, so it belongs inside main, not outside of it. Plus, it would be a breaking change, and we want to avoid these.

[wclr]

I use this signature for state reducer:

type Reducer<T> = (state: T) => T

[gomain]

I suggest a new constructor withInitialState something like this:

const withIntiailState = (main, initialState: T, name) => {
  const sandwiched = (sources) => {
    const mainSinks = main(sources)
    return {
     ...mainSinks,
     [name]: (mainSinks[name] as Stream<Reducer<T>>).startWith(_ => initailState)
    }
  }
  return withState(sandwiched, name)
}

sandwiched wraps the main component and in turn is wrapped with withState

[mightyiam]

Hmm, but does that happen? I don't recall having to do state$.filter(isDefined). StateSource is a generic on S, with MemoryStream<S>

Sure. If the logic is such that the initial undefined state is not emitted then the typing of the source is correct and there's no problem with the source.

The problem is in the sink type. The reducer demanded by the TypeScript type of the sink is the one that expects a possibly undefined state parameter. When a sink of type Stream<(T: State) => State> is provided, it passes type checking only when strictFunctionTypes: false.

When strictFunctionTypes: true, the sink would have to be at least Stream<(T: State | undefined) => State>.

That is why I claim that there's a type problem here. And the type problem exposes a missed opportunity of using type checking to make sure that reducers are always called with initialized state.

Currently, to conform with the sink types (with strictFunctionTypes: true), the component must provide a reducer sink that is of type Stream<(T: State | undefined) => State>. And so I have to write handling of undefined or do some (undesired) type assertions.

If an initialized state is required in withState

That's not going to happen for a tiny issue such as this one. Initial state is application logic, so it belongs inside main, not outside of it. Plus, it would be a breaking change, and we want to avoid these.

Initial state does feel like application logic, but also the mere having of state feels like application logic. And the mere having of state is removed outside the component to be the concern of withState. So, along with it, should go state initialization.

The concept that reduction and initialization of state is application logic makes sense to me. In most programs, state is initialized along with the possible type declaration of it, as soon as it exists. In current @cycle/state, state is practically initialized with the value undefined.

If it is so important to refrain from such a breaking change as the one I suggested, here is a proposal:

export function withState<
  So extends OSo<T, N>,
  Si extends OSi<T, N>,
  T = any,
  N extends string = 'state'
>(main: MainFn<So, Si>, name?: N): MainWithState<So, Si, T, N>
export function withState<
  So extends OSo<T, N>,
  Si extends OSi<T, N>,
  T = any,
  N extends string = 'state'
>(main: MainFn<So, Si>, initializer?: () => T, name?: N): MainWithState<So, Si, T, N>
export function withState<
  So extends OSo<T, N>,
  Si extends OSi<T, N>,
  T = any,
  N extends string = 'state'
>(main: MainFn<So, Si>, b?: (() => T) | N, c?: N): MainWithState<So, Si, T, N> {
  const [initializer, name] = typeof b === 'function'
    ? [b, c ?? 'state' as N]
    : [null, b ?? 'state' as N]
  // ...
}

The down-side of this proposal is that the initial state must be provided as a function that returns the initial state.

Another proposal is just a new function that provides a better alternative to withState. Perhaps withState could be deprecated for a long time.

withInitializedState(main, initialState, name)
// alternative names
withInitialState(main, initialState, name)
stateful(main, initialState, name)

[staltz]

This is a TypeScript problem, and it shouldn't leak out to JavaScript users of Cycle.js.

Viable solutions are:

• Don't use strictFunctionTypes
• Handle undefined inputs in every single reducer you create (after all, you chose strictFunctionTypes)
• Figure out new typings (not new runtime APIs) that fix this issue
• Fix TypeScript to allow us to express these cases
• Fix stream types to allow describing the first emission type, versus the subsequent emission types, then make the state sink require a type for a Stream where the first emission is (u: undefined) => State and the subsequent emissions are Reducer
  • This idea wouldn't be immediately merged in, though, it would have to be thought out carefully and avoid other breaking changes

Inviable solutions are:

• Introduce withInitialState and deprecate withState
  • This issue @cycle/state Reducer to expect defined state? #948 is not critical enough to require all other @cycle/state users to migrate to a new API, issue @cycle/state Reducer to expect defined state? #948 is a corner case

[wclr]

@mightyiam did you try how something like ((T: State) => State) | ((T: undefined) => State) would work?

[gomain]

Fix stream types to allow describing the first emission type, versus the subsequent emission types, then make the state sink require a type for a Stream where the first emission is (u: undefined) => State and the subsequent emissions are Reducer

This would be ideal, but still a breaking change. Current implementation allows the usecase of passing a state initializer (_: undefined) => State (set the whole state) or even (_: undefined) => undefined (reset the state) anytime. It's probably safe to assume this feature is being exploited.

Inviable solutions are:

* Introduce `withInitialState` and deprecate `withState`

withState need not be deprecated at all. The, if, introduced withInitialState (possibly along with its Reducer<T> helper type) could coexist for users who prefer the stricter types. From an extra/different module even.

[wclr]

I suggest a new constructor withInitialState

Setting an initial state is an app logic realm, should be done inside Main.

[mightyiam]

Viable solutions are:

• Don't use strictFunctionTypes

There is an opportunity to provide more type safety to the users. "Don't use strictFunctionTypes" is equivalent to "Cycle.js doesn't support strictFunctionTypes. The whole purpose of the option strictFunctionTypes even existing is backward compatibility with code that was written before this particular strict-ness was available in TypeScript. Otherwise, TypeScript is constantly moving toward more strict (thorough/strong) type checking.

• Handle undefined inputs in every single reducer you create (after all, you chose strictFunctionTypes)

No one wants to do that, right?

• Figure out new typings (not new runtime APIs) that fix this issue

The only new strictly-type change that would fix this issue would be the one listed below, adding an additional type parameter to the stream library.

• Fix TypeScript to allow us to express these cases

I'm not sure what additional feature in TypeScript you imagine.

• Fix stream types to allow describing the first emission type, versus the subsequent emission types, then make the state sink require a type for a Stream where the first emission is (u: undefined) => State and the subsequent emissions are Reducer
  • This idea wouldn't be immediately merged in, though, it would have to be thought out carefully and avoid other breaking changes

Understood to be non-realistic.

Inviable solutions are:

• Introduce withInitialState and deprecate withState

  • This issue @cycle/state Reducer to expect defined state? #948 is not critical enough to require all other @cycle/state users to migrate to a new API, issue @cycle/state Reducer to expect defined state? #948 is a corner case

It should be considered a serious matter, unless Cycle.js wants to tell its users "don't use strictFunctionTypes" or "we know these types are deficient — use assertions there".

Here's another proposal:

withState(main, name) // current call, where name is optional
withState(main, { initial: initialState, name }) // new overload

This could be a non-breaking change, right?

The sink type would depend on whether initialState was provided. The sink type when initialState is provided will be the one that does not include undefined, because initial state was provided. TypeScript is able to do that. Here is simplified example code.

[staltz]

@mightyiam Please quit your insistence on saying this is a serious matter and that the only solution is a new withState API.

The viable versus inviable solutions I listed are my same arguments. If you use strictFunctionTypes then you handle all undefined inputs in reducers. If you don't use it, you don't need to handle it. Cycle.js doesn't have perfect TypeScript types (there are also type assertion issues with cycle/isolate) because at some point we would have to choose between perfect TypeScript support or Cycle.js architecture properties (such as app logic only in main), and these are sacrifices we don't need to do just because you opened this corner case issue.

If you proceed to insist on your same arguments, I'll ignore you because I have to spend my time wisely.

[gomain]

Setting an initial state is an app logic realm, should be done inside Main.

Main having state could also be regarded as app logic. Consider...

const main = withState(
  (sources) => {
    const state$ = sources.state.stream
    // ... do your thing ...
    const stateReducer$: Stream<(s: State | undefined) => State | undefined> = ...
    return {
      state: stateReducer$
    }
  }
)

Then...

const main = withInitialState(
  (sources) => {
    const state$ = sources.state.stream
    // ... do your thing ...
    const stateReducer$: Stream<(s: State) => State> = ...
    return {
      state: stateReducer$
    }
  },
  initialState
)

[staltz]

I don't understand where this having argument came from. The only way main has state is by receiving an input which is a Cycle.js source. So main receives state management utilities, it doesn't "define the having" of such utilities. And more importantly: the way it does that is the same treatment for DOM, HTTP, and other channels. So if we put "definition of having state" inside main, we'd have to put all the other "definitions of havings" inside main as well. Which would undermine the Cycle.js architecture that aims to allow mocking the channels for testing, or swapping the channels for different implementations of the effects.

[staltz]

Setting an initial state is an app logic realm, should be done inside Main.

Main having state could also be regarded as app logic. Consider...

const main = withState(
  (sources) => {
    const state$ = sources.state.stream
    // ... do your thing ...
    const stateReducer$: Stream<(s: State | undefined) => State | undefined> = ...
    return {
      state: stateReducer$
    }
  }
)

Then...

const main = withInitialState(
  (sources) => {
    const state$ = sources.state.stream
    // ... do your thing ...
    const stateReducer$: Stream<(s: State) => State> = ...
    return {
      state: stateReducer$
    }
  },
  initialState
)

There's probably also a misunderstanding. We typically say that main is the inner function (the anonymous arrow function), not the outer one. The outer function also doesn't define the "having of initial state", because the outer function wasn't written by scratch, it was generated by the utility function.

[gomain]

@staltz Thanks for explaining. Let me get my head around the Cycle.js architecture.

[mightyiam]

If you proceed to insist on your same arguments, I'll ignore you because I have to spend my time wisely.

I have found that it doesn't make sense to provide initial state in the call to withState because initial state may depend on emits from the component's own sources.

The type of the sink is still an inconvenience, but now I can't imagine a reasonable change in the API.

[mightyiam]

How about this: if a stream is provided as the sink, then it reduces a possibly undefined state. If an object with initial state and a reducer stream, then the reducer stream will reduce from defined state. The type of the state sink would be something like this:

// Current API
type A<T> = Observable<(s: T | undefined) => T>
// Additional API
type B<T> = {
  initial: T
  reducer$: Observable<(s: T) => T>
}
type StateSink<T> = A<T> | B<T> 

This could be backward compatible and provide a means to initialize state on component initialization (which makes sense to me) and prevents ever having undefined in a reducer.

Initial state occurs in the component and not outside of it.

Also, it guarantees that a component has an initial state emit from its state source stream, which could be considered a benefit, because it could help prevent the no-UI-because-one-of-the-components-did-not-emit-on-DOM situation.

[wclr]

This just probably need to be set to type Reducer<T> = (state: T) => T if one needs undefined thing just use Reducer<State | undefined>

[staltz]

Sinks in Cycle.js are only streams.

[mightyiam]

Sinks in Cycle.js are only streams.

Is that open for discussion?

[staltz]

As a solution to this corner case TypeScript bug? I don't think so. Small and specific type-level bugs should not require runtime-level shoehorn solutions that make exceptions to the app architecture.

[mightyiam]

Every usage of @cycle/state in TypeScript will experience this issue, right? Does this qualify as a corner case?

The possibility of this issue being solved somehow in TypeScript itself or in the types of the various stream/observable libraries is practically zero, right? If so, and if this type safety issue is not solved by making a runtime adjustment, then it's pretty much given up on, isn't it?

[staltz]

I refer again to #948 (comment)

Every usage of @cycle/state in TypeScript will experience this issue, right?

Don't enable strictFunctionTypes (I said this already)