Skip to content
This repository has been archived by the owner on Feb 14, 2023. It is now read-only.

Global reducers do not work in TypeScript 4. #174

Open
fny opened this issue Oct 30, 2020 · 4 comments
Open

Global reducers do not work in TypeScript 4. #174

fny opened this issue Oct 30, 2020 · 4 comments
Labels
bug Something isn't working as expected. help wanted The owner cannot resolve this on their own.

Comments

@fny
Copy link

fny commented Oct 30, 2020

Type inference for reducers appears to be broken with TypeScript v4 and reactn 2.2.7.

For some reason, useDispatch(reducerName) results in the following error:

// ERROR: Type of property 'increment' circularly references itself in mapped type 'DispatcherMap<State, Reducers>'.ts(2615)

Below is an example.

// global.d.ts
import 'reactn';
import { Dispatch } from 'react';

declare module 'reactn/default' {
  export interface Reducers {
    increment: (global: State, dispatch: Dispatch, i: number) => Pick<State, 'x'>
  }

  export interface State {
    x: number
  }
}
// App.tsx
import React, { setGlobal, useGlobal, useDispatch, addReducer } from 'reactn'

interface ProviderState {
  x: number
}

const INITIAL_STATE: ProviderState = {
  x: 0
}

setGlobal(INITIAL_STATE)

addReducer('increment', (global, dispatch, i = 0) => {
  return { x: global.x + i }
})

export default function () {
    const increment = useDispatch('increment') // <--- ERROR
    const [ x, ] = useGlobal('x')

    return (
      <div>
        <p>{}</p>
        <button onClick={() => increment(1) }>Increment</button>
      </div>
    )
}

Here is a second, related issue. The types for Provider.useDispatch does not accept strings even though the global version does.

import React, { createProvider } from 'reactn'

interface ProviderState {
  x: number
}

const INITIAL_STATE: ProviderState = {
  x: 0
}

const Provider = createProvider(INITIAL_STATE)

Provider.addReducer('increment', (state, dispatch, x = 0) => ({ 
  x: x + 1
})

export default function () {
    const increment = Provider.useDispatch('increment') // <---- ERROR
    const [ x, ] = Provider.useGlobal('x')

    return (
      <div>
        <p>{}</p>
        <button onClick={() => increment() }>Increment</button>
      </div>
    )
}
ERROR

No overload matches this call.
  Overload 1 of 4, '(reducer: Reducer<ProviderState, Reducers, any[], NewGlobalState<ProviderState>>): Dispatcher<ProviderState, any[]>', gave the following error.
    Argument of type 'string' is not assignable to parameter of type 'Reducer<ProviderState, Reducers, any[], NewGlobalState<ProviderState>>'.
  Overload 2 of 4, '(reducer: never): Dispatcher<ProviderState, never>', gave the following error.
    Argument of type 'string' is not assignable to parameter of type 'never'.
@quisido
Copy link
Collaborator

quisido commented Nov 3, 2020

This is accurate. I am not aware of a solution. The reducers do circularly reference themselves, so TypeScript is not wrong to say that; but I feel it is wrong to fail to compile, since it worked in earlier versions and works at runtime.

In the meantime, you should be able to migrate your global reducers from addReducer() to the local useDispatch, which does not inject other global reducers. It's not as convenient as global reducers, but it unblocks this error and is likely a better practice for strong typing.

@quisido quisido added bug Something isn't working as expected. help wanted The owner cannot resolve this on their own. labels Nov 3, 2020
@quisido quisido changed the title Cyclical Reference when Adding Reducers Reducers do not work in TypeScript 4. Nov 3, 2020
@quisido quisido changed the title Reducers do not work in TypeScript 4. Global reducers do not work in TypeScript 4. Nov 3, 2020
@fny
Copy link
Author

fny commented Nov 3, 2020

migrate your global reducers from addReducer() to the local useDispatch

I'm not sure what you mean by this, can you clarify?

@quisido
Copy link
Collaborator

quisido commented Nov 3, 2020

Right now you are using addReducer, which requires TypeScript import your global.d.ts file, which is where the infinite recursion exists.

// index.ts
Provider.addReducer('increment', (previousState, dispatch, i) => ({ 
  x: previousState.x + i
});

// component.ts
const increment = useDispatch('increment');
increment(1); // global.x = global.x + 1

This would be how you can refactor it to be defined locally instead of on the global ReactN object:

// reducers/increment.ts
// Here, the dispatch prop is unused, and as long as the typeof reducers
//   is empty in your global.d.ts, there is no recursive definition.
const increment = (previousState, _dispatch, i = 1) => ({
  x: previousState.x + i
});

// component.ts
import incrementReducer from 'reducers/increment';

const increment = useDispatch(incrementReducer);
increment(1); // global.x = global.x + 1

You can also replace this with a property reducer to simplify it further:

// reducers/increment
const increment = (previous, i) => previous + i;

// component.ts
import increment from 'reducers/increment';

const increment = useDispatch(increment, 'x');
increment(1); // global.x = global.x + 1

The idea is to get rid of addReducer, which is what TypeScript 4 does not support. You can do this by defining these reducer functions separately, importing them, and passing them to useDispatch instead of strings. Hope this helps.

@monteiz
Copy link
Contributor

monteiz commented Feb 2, 2021

@CharlesStover The approach you described here is totally legit, and I am using it successfully even with Server Side Rendering, thanks.

addReducer does not work with Server Side Rendering, so my suggestion is to mark it as deprecated, and update the documentation describing this approach.

js-jslog pushed a commit to js-jslog/harpguru that referenced this issue Sep 20, 2021
The global reducer idea would have actually been very appropriate in
this context. Have a global reducer sat in front of all the global state
to make it clear that no global state should be updated directly. As it
stands there is a referencial cycle which can't be avoided when using
typescripts `addReducer` function which reactn uses for it's own
`addReducer`: CharlesStover/reactn#174

Charles's own advice is to use local reducers instead. I still want to
get some kind of benefit like that described above, and while I don't
think it's actually as semantically clear doing it locally like this
there probably is scope to have saga's like reducers like the one I've
created here at `setNewHarpStrataReducer`. Making this pattern
ubiquitous in the way required though would be out of scope for this
feature. I'll have to raise it as techdebt.
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
bug Something isn't working as expected. help wanted The owner cannot resolve this on their own.
Projects
None yet
Development

No branches or pull requests

3 participants