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

Class 'NgRedux<RootState>' incorrectly implements interface 'ObservableStore<RootState>' #3466

Closed
eyalewin-zz opened this issue Jul 9, 2019 · 29 comments

Comments

@eyalewin-zz
Copy link

eyalewin-zz commented Jul 9, 2019

~7 hours ago a new Redux 4.0.2 was released.
but there is no backward compatibility with current running code.

We had redux : ^4.0.0.
therefore, when running on 4.0.1 everything was OK
but from today all our builds are failing due to the following message:

ERROR in node_modules/@angular-redux/store/lib/src/components/ng-redux.d.ts(10,31): error TS2420: Class 'NgRedux' incorrectly implements interface 'ObservableStore'

Property '[Symbol.observable]' is missing in type 'NgRedux' but required in type 'ObservableStore'

@MhMadHamster
Copy link

I'm receiving this error because in my package.json i have redux version strictly set to 4.0.1, I'm also using react-redux v.7.1.0 with @types/react-redux v.7.1.1, since @types/react-redux have redux as a not strictly dependency ^4.0.0, it's installed twice: one copy in the root node_modules v4.0.1 and the other one inside the node_modules/@types/react-redux/... This setup results in an error mentioned above: '[Symbol.observable]' is missing in type ... but required in type ...

I tried updating redux to version 4.0.2 to resolve duplicating, but even tho it's patch version it introduced breaking changes caused by #3411 i believe, so in 4.0.2 i just have type errors like: Expected 1 type arguments, but got 2. when using combineReducers and passing 2 type arguments and Type '...' does not satisfy the constraint 'ReducersMapObject<any, any>'. when passing 1.

I resolved this issue for now, by adding

"resolutions": {
  "redux": "4.0.1"
}

but it would be nice to get this types fixed.

yarn 1.12.1
typescript 3.5.2

@fabienbranchel
Copy link

Similar issue with React (16.8.6) / Redux (4.0.2) / TypeScript (3.5.3) :

Property '[Symbol.observable]' is missing in type 'Store<{}, AnyAction> & { ...; }' but required in type 'Store<any, AnyAction>'

@reduxjs reduxjs deleted a comment from sagitsofan Jul 9, 2019
@timdorr
Copy link
Member

timdorr commented Jul 9, 2019

Are there deeper error messages available? I'm having a hard time interpreting what's going on without seeing the full tree of what TS is reporting.

Can we get a small PoC? It sounds like these dependant types need to be updated. But a faster fix would be to revert #3067 and #3411.

@fabienbranchel
Copy link

Full error messages :

Property '[Symbol.observable]' is missing in type 'Store<{}, AnyAction> & { ...; }' but required in type 'Store<any, AnyAction>
ts(2741)
index.d.ts(248, 3): '[Symbol.observable]' is declared here.
index.d.ts(397, 5): The expected type comes from property 'store' which is declared here on type 'IntrinsicAttributes & IntrinsicClassAttributes<Provider<AnyAction>> & Readonly<ProviderProps<AnyAction>> & Readonly<{ children?: ReactNode; }>'
    47 |
    48 |     ReactDOM.render(
  > 49 |       <Provider store={store}>
       |                 ^
    50 |         <App />
    51 |       </Provider>,

@caipirginka
Copy link

Same error with redux 4.0.2 and @angular-redux/store (v 10 or 9).
Inside node_modules@angular-redux\store\components\ng-redux.d.ts
export declare abstract class NgRedux<RootState> implements ObservableStore<RootState> {

Inside node_modules@angular-redux\store\components\observable-store.d.ts
export interface ObservableStore<StateType> extends Store<StateType> {

Inside node_modules\redux\index.d.ts
Inside interface Store:
export interface Store<S = any, A extends Action = AnyAction> {

at line 248 of redux 4.0.2, there is:
[Symbol.observable](): Observable<S>

This line is not present in redux 4.0.1 and it seems dependent libraries aren't aware of it, hence the compilation error...

@MhMadHamster
Copy link

@timdorr sure check out this repo i quickly made to reproduce the issue https://github.com/MhMadHamster/redux-types-problem-poc
You can run yarn && yarn check-types or just open project in vscode to see the error about [Symbol.observable]. You can then upgrade redux yarn upgrade redux@latest and run yarn check-types again to see the issue caused by #3411.
Note that on redux 4.0.1 there are no problems with combineReducers types.

@timdorr
Copy link
Member

timdorr commented Jul 9, 2019

Thanks everyone. I'm digging in right now.

@markerikson
Copy link
Contributor

Two observations from the sideline:

  • This is why no one should ever rely on installing directly from NPM in their CI, but instead use something like Yarn's "offline mirror" feature. Also, something something lockfiles.
  • This is also why I really rather wish we didn't ship our own typings included. Any tweak to the typings could end up as an effective breaking change and borders on requiring a new major version. Bleh.

@timdorr
Copy link
Member

timdorr commented Jul 9, 2019

@MhMadHamster The [Symbol.observable] error appears to be from mismatched dependencies. @types/react-redux is pulling in the latest version of redux, whereas your top level dependency is limited to 4.0.1. That looks like this:

 ⇶ npm ls redux
redux-types-problem@1.0.0 /Users/timdorr/forks/redux-types-problem-poc
├─┬ @types/react-redux@7.1.1
│ └── redux@4.0.2
└── redux@4.0.1

Upgrading to 4.0.2 to align all the versions together makes this particular bug go away:

 ⇶ npm ls redux
redux-types-problem@1.0.0 /Users/timdorr/forks/redux-types-problem-poc
├─┬ @types/react-redux@7.1.1
│ └── redux@4.0.2  deduped
└── redux@4.0.2

I would advise others to do the same.

Still looking at the combineReducers issue. That may just need a straight up revert.

@timdorr
Copy link
Member

timdorr commented Jul 9, 2019

@markerikson The types still work, just not between different versions of them. It's similar to switching from the legacy Context API. If you used connect from one version and Provider from another, of course that's not going to work.

This isn't a breaking change, it's just broken.

@timdorr
Copy link
Member

timdorr commented Jul 9, 2019

I just did the #3411 revert: #3467.

Unfortunately, we have no tests that cover the type parameters of combineReducers, so changing that API is a breaking change. Also, in retrospect, I don't like that we're exposing an internal type (ReducersMapObject) as something users should have to know about to use that type param. I should have not merged in that PR without some further scrutiny and discussion. I still think it's a viable option, but needs a different implementation.

And we definitely need tests that cover those params (all params, really).

Edit: @MhMadHamster I can confirm that revert fixes your particular test case. I'll release that as 4.0.3 shortly.

@timdorr
Copy link
Member

timdorr commented Jul 9, 2019

@MhMadHamster
Copy link

@timdorr Thanks!
Regarding #3466 (comment), yes I tried to describe that exact issue in my first comment, updating fixes it, but, IMHO that is not really user-friendly since i'm forced to update redux version asap, so for TypeScript users it's more like a type-breaking change. Every package like react-redux or redux-logger or w'e that is depends on redux types will force me to update to latest version if you will decide to extend types, unless i will lock redux version via resolutions. So regarding your API example, i dont think it's just as simple as that, when you publish your new API for example, but do not remove legacy, it's not breaking cause users basically do not need to do anything to keep their code working, in the contrary, with types, even if i'm not updating redux itself or any package that depends on it, all it needs is a fresh installation, and you will get an error, so you're forced to update it anyways.

@timdorr
Copy link
Member

timdorr commented Jul 9, 2019

At the very least, this brings to light a potential risk in your project, where you are using different versions of the types for the same package. That's something you should fix, especially when dealing with external types (like DT) where a type version might be tied to a different library version than what you are using.

It ultimately comes down to how something like @types/react-redux selects the redux version (caret selection) and how you do it (exact versioning). That's not a TS or semver or Redux issue, that's just how npm resolves dependencies and how the node module resolution algorithm can be a problem sometimes.

This is actually something we're dealing with on React Router, where we have two packages and have switched from legacy to modern React Context. We've had a lot of issues where folks have mixed versions of react-router and react-router-dom. It's nothing the library can/should fix, so we just have to let folks know every time they run into it.

I wish I had a better answer for this. For now, you can either lock your dep resolutions, open up your version selector, or just keep your lockfile as-is to prevent version changes. Outside of never making changes to our types, there isn't really anything we can do to prevent or fix this.

@timdorr timdorr closed this as completed Jul 9, 2019
@Dammytrager
Copy link

I think it has to do with the dependencies, using redux 4.0.2 doesn't work with the present @angular-redux,
redux 4.0.1 should work, so npm i redux@4.0.1 to install the compatible redux.

@caipirginka
Copy link

Yes, I can confirm that redux@4.0.1 works with latest @angular-redux.
There seems to be a specular issue on @angular-redux side: angular-redux/platform#92
But I didn't understand if this is indeed a bug inside redux and it will get corrected if a future version, or if it is a bug inside angular-redux that needs to be modified to keet it working with future versions of redux...
I mean the [Symbol.observable](): Observable<S> problem...

@shahidbasheer
Copy link

redux@4.0.1 worked for me .

@ghanlohar
Copy link

redux@4.0.1 worked for me too. Thanks all..

@ahahn95
Copy link

ahahn95 commented Oct 22, 2019

I'm seeing this issue on redux 4.0.3, should I downgrade?

@bitflower
Copy link

Seeing it on redux@4.0.4 in a StencilJS environment.

@obsidian33
Copy link

I'm seeing this issue on redux 4.0.3, should I downgrade?

Downgrading to redux@4.0.1 will resolve this issue.

@myquery
Copy link

myquery commented Dec 19, 2019

I got this error when tried to configure angular-redux/store with my app, the app is still serving, i looked at the node module folder under the angular-redux folder at ng-redux.ts:

export declare abstract class NgRedux<RootState> implements ObservableStore<RootState> { /** @hidden, @deprecated */ static instance?: ObservableStore<any>;

been trying to fix it for a while now

@gusjabar
Copy link

I have this error, how could I fix it.

ERROR in ../node_modules/@angular-redux/store/components/ng-redux.d.ts:10:31 - error TS2420: Class 'NgRedux' incorrectly implements interface 'ObservableStore'.
Property '[Symbol.observable]' is missing in type 'NgRedux' but required in type 'ObservableStore'.

10 export declare abstract class NgRedux implements ObservableStore {
~~~~~~~

../node_modules/redux/index.d.ts:337:3
  337   [Symbol.observable](): Observable<S>
        ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
  '[Symbol.observable]' is declared here.

@paulterinho
Copy link

paulterinho commented Jan 26, 2020

Thank you!!!!

Initially, I tried to downgrade to 4.0.1 by changing it in the package.json, then issuing a generic npm install, which didn't work, BUT for those that are still having issues and not getting it (like I was). Just use a full npm install directive like:

npm install redux@4.0.1

@myquery
Copy link

myquery commented Jan 27, 2020

Thank you!!!!

Initially, I tried to downgrade to 4.0.1 by changing it in the package.json, then issuing a generic npm install, which didn't work, BUT for those that are still having issues and not getting it (like I was). Just use a full npm install directive like:

npm install redux@4.0.1

Thanks for this, that solved my issue

@carpenterdev
Copy link

I'm upgrading from Angular 5 to Angular 8, and have updated ng-redux to 4.0.1. Is the following compile error related to this topic?

ERROR in ../node_modules/@angular-redux/store/components/dev-tools.d.ts:6:15 - error TS2709: Cannot use namespace 'EnhancerOptions' as a type.

If so, how to fix?
Thanks.

@davilacs
Copy link

davilacs commented Mar 4, 2020

npm install redux@4.0.1

I tried the solutions before, but this was the worked for me.

Thanks!

@sobvan
Copy link

sobvan commented Mar 11, 2020

Thank you!!!!

Initially, I tried to downgrade to 4.0.1 by changing it in the package.json, then issuing a generic npm install, which didn't work, BUT for those that are still having issues and not getting it (like I was). Just use a full npm install directive like:

npm install redux@4.0.1

This cannot be different than changing the dependency in package.json and running npm i. If the two ways have different outcomes, that is certainly an npm bug. If however you forgot to remove the ^ sign form the version, that would be an explanation for your case.

@AJD-Apps
Copy link

redux@4.0.1 worked for me too. Thanks all..

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

No branches or pull requests