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

StoreEnhancer has incorrect parameter type #3768

Closed
Methuselah96 opened this issue May 9, 2020 · 4 comments · Fixed by #3776
Closed

StoreEnhancer has incorrect parameter type #3768

Methuselah96 opened this issue May 9, 2020 · 4 comments · Fixed by #3776

Comments

@Methuselah96
Copy link
Member

Methuselah96 commented May 9, 2020

Prior Issues

Introduced in #3524.

What is the current behavior?

Code

type MyEnhancerType = { enhancedStuff: number };

function createEnhancer(): StoreEnhancer<MyEnhancerType> {
  return (createStore) => <S, A extends Action>(
    reducer: Reducer<S, A>,
    initialState?: PreloadedState<S>
  ): Store<S, A, never, MyEnhancerType> & MyEnhancerType => {
    // This store that I just created does not have the enhancer applied to it yet
    // It also doesn't know what enhancers have been applied to it
    const nextStore = createStore(reducer, initialState);

    // And yet we can access a property that we have yet to add.
    // This should produce a type error, but it doesn't.
    const propertyThatsNotOnStoreYet = nextStore.enhancedStuff;

    // It's not until here that we have an enhanced store
    // And should be able to access that property
    const enhancedStore = enhanceStore(nextStore);
    const correctPropertyAccess = enhancedStore.enhancedStuff;

    return enhancedStore;
  };
}

function enhanceStore<S, A extends Action>(store: Store<S, A, unknown, unknown>): Store<S, A, never, MyEnhancerType> & MyEnhancerType {
  return {
    ...store,
    enhancedStuff: 5,
  }
}
Output
function createEnhancer() {
    return (createStore) => (reducer, initialState) => {
        // This store that I just created does not have the enhancor applied to it yet
        // It also doesn't know what enhancors have been applied to it
        const nextStore = createStore(reducer, initialState);
        // And yet we can access a property that we have yet to add.
        const propertyThatsNotOnStoreYet = nextStore.enhancedStuff;
        // It's not until here that we have an enhanced store
        // And should be able to access that property
        const enhancedStore = enhanceStore(nextStore);
        const correctPropertyAccess = enhancedStore.enhancedStuff;
        return enhancedStore;
    };
}
function enhanceStore(store) {
    return Object.assign(Object.assign({}, store), { enhancedStuff: 5, 
        // Ignore this for now. See https://github.com/reduxjs/redux/issues/3767 for more discussion
        replaceReducer(nextReducer) {
            return store.replaceReducer(nextReducer);
        } });
}
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?

Trying to access enhanced properties right after calling createStore should result in a type error.

Environment Details

N/A

@Methuselah96 Methuselah96 changed the title StoreEnhancor type has incorrect parameter type for StoreEnhancerStoreCreator StoreEnhancor type has incorrect parameter type May 9, 2020
@Methuselah96 Methuselah96 changed the title StoreEnhancor type has incorrect parameter type StoreEnhancer has incorrect parameter type May 9, 2020
@Methuselah96
Copy link
Member Author

Partially addressed by #3772, but there's more to be done after that PR gets merged.

@Methuselah96
Copy link
Member Author

Methuselah96 commented May 13, 2020

Never mind, just made #3776 as a separate PR that can get merged whether #3772 gets merged or not.

@alestor123
Copy link

If the code works in JS but fails only because of typing in TypeScript, I'd suggest you to use "any" to ignore it: const store = () => createStore( reducer, initialState as any )

@Methuselah96
Copy link
Member Author

Methuselah96 commented Jun 11, 2020

@alestor123 The issue is that there is no compilation error and there should be. The code above currently compiles; I don't need to do any casting.

I'm aware that I can use any to ignore basically any problems with types. My goal in creating this issue is to fix the types, not just ignore them (otherwise I might as well be using JS). This problem is more extensive than just the single example provided above. I can provide more examples of why the current types are problematic if that would be helpful (for example getting the correct type after composing enhancers is not possible with the current types).

I have also a PR (#3776) attached to this issue that fixes the problem. As the PR shows, having the correct types actually caught a few bugs in the tests that were easy to overlook with current types. Let me know if you think the proposed solution is incorrect.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
2 participants