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

replaceReducer type doesn't account for the mutation to the original store #3767

Closed
Methuselah96 opened this issue May 9, 2020 · 0 comments · Fixed by #3772
Closed

replaceReducer type doesn't account for the mutation to the original store #3767

Methuselah96 opened this issue May 9, 2020 · 0 comments · Fixed by #3772

Comments

@Methuselah96
Copy link
Member

Methuselah96 commented May 9, 2020

Prior Issues

What is the current behavior?

Code

// Here's the original reducer
type CounterAction = { type: 'INCREMENT' } | { type: 'DECREMENT' };
function counter(state = 0, action: CounterAction) {
  switch (action.type) {
    case 'INCREMENT':
      return state + 1;
    case 'DECREMENT':
      return state - 1;
    default:
      return state;
  }
}
const originalStore = createStore(counter);

// Here's a new reducer
type StringCaserAction = { type: 'UPPER_CASE' } | { type: 'LOWER_CASE' };
function stringCaser(state = 'testing', action: StringCaserAction) {
  switch (action.type) {
    case 'UPPER_CASE':
      return state.toUpperCase();
    case 'LOWER_CASE':
      return state.toLowerCase();
    default:
      return state;
  }
}
const newStore = originalStore.replaceReducer(stringCaser);

// ok
newStore.dispatch({ type: 'UPPER_CASE' });

// I can still use the old store ?!?
// But the old store has been mutated and now has the new reducer
// The state is incorrectly typed as a number here
// It will actually return a string because the reducer has been replaced
const myNumber = originalStore.getState();
Output
function counter(state = 0, action) {
    switch (action.type) {
        case 'INCREMENT':
            return state + 1;
        case 'DECREMENT':
            return state - 1;
        default:
            return state;
    }
}
const originalStore = createStore(counter);
function stringCaser(state = 'testing', action) {
    switch (action.type) {
        case 'UPPER_CASE':
            return state.toUpperCase();
        case 'LOWER_CASE':
            return state.toLowerCase();
        default:
            return state;
    }
}
const newStore = originalStore.replaceReducer(stringCaser);
// ok
newStore.dispatch({ type: 'UPPER_CASE' });
// I can still use the old store ?!?
// But the old store has been mutated and now has the new reducer
// The state is incorrectly typed as a number here
// It will actually return a string because the reducer has been replaced
const myNumber = originalStore.getState();
Compiler Options
{
  "compilerOptions": {
    "noImplicitAny": true,
    "strictNullChecks": true,
    "strictFunctionTypes": true,
    "strictPropertyInitialization": true,
    "strictBindCallApply": true,
    "noImplicitThis": true,
    "noImplicitReturns": true,
    "alwaysStrict": true,
    "esModuleInterop": true,
    "declaration": true,
    "experimentalDecorators": true,
    "emitDecoratorMetadata": true,
    "moduleResolution": 2,
    "target": "ES2017",
    "jsx": "React",
    "module": "ESNext"
  }
}

Playground Link: Provided

Steps to Reproduce

See code above.

What is the expected behavior?

There's no way to express this type of mutation in types. It would be better to remove the return type for replaceReducer. This would then force them to cast the reducer and the store so that they realize they shouldn't be using the original store because it's types are wrong.

Environment Details

N/A

@Methuselah96 Methuselah96 changed the title replaceReducer type is problematic replaceReducer doesn't account for the mutation to the original store May 9, 2020
@Methuselah96 Methuselah96 changed the title replaceReducer doesn't account for the mutation to the original store replaceReducer type doesn't account for the mutation to the original store May 9, 2020
This issue was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant