search

ngneat / effects

🪄 A framework-agnostic RxJS effects implementation
https://www.netbasal.com
MIT License
63 stars 11 forks source link

Not supported for @angular/router@15.2.1 #45

Closed utpaulBS23 closed 1 year ago

utpaulBS23 commented 1 year ago

Which @ngneat/effects-* package(s) are the source of the bug?

effects-ng

Is this a regression?

Yes

Description

Hi, I'm using node version 18.14. I'm getting a problem after uprating my angular version 14 to 15. I saw it's a peer dependency. Here is the screenshot.

Screen Shot 2023-03-04 at 5 49 13 PM

Please provide a link to a minimal reproduction of the bug

No response

Please provide the exception or error you saw

No response

Please provide the environment you discovered this bug in

Node version 18.14.0 and npm version 9.3.1 angular cil 15.2.1

Anything else?

No response

Do you want to create a pull request?

No

NetanelBasal commented 1 year ago

@EricPoul

EricPoul commented 1 year ago

I added a router for test purposes and didn't add to the peerDependencies. Should I leave tests and add router to peerDependencies?

NetanelBasal commented 1 year ago

How the router related to the library?

EricPoul commented 1 year ago

Only imports in tests for checking lazy provided effects

EricPoul commented 1 year ago

I see in tests in NGRX they depend on both @angular/router/testing and @angular/router in tests. They do not add it in peerDependencies.

NetanelBasal commented 1 year ago

I don't understand how the router is related. The only dependency is Angular:

https://github.com/ngneat/effects/blob/master/libs/effects-ng/package.json#L39

EricPoul commented 1 year ago

Yes, I know, it's strange. I could find a similar issue and a few referenced issues inside. https://github.com/ngrx/platform/issues/2830

utpaulBS23 commented 1 year ago

Do you resolve this issue @EricPoul ?

EricPoul commented 1 year ago

Not yet. I can't just update every angular version. Use --legacy-peer-deps for now. I'll try to research what we can do.

utpaulBS23 commented 1 year ago

@EricPoul you do research and make it comparable with all angular version

EricPoul commented 1 year ago

@NetanelBasal Hard to test it until it's merged. I went through ngrx repo to check what the problem can be. The only thing I think can differ is that they store all specs in the spec folder which is on the same level as src folder.

NetanelBasal commented 1 year ago

@EricPoul when you install the library it installs the router?

EricPoul commented 1 year ago

I've just installed @ngrx/effects that uses Router in its test and it hasn't argue about peedDependencies.

NetanelBasal commented 1 year ago

There is no reason that it will yell in our library. It's not a dependency. Did you try it yourself?

EricPoul commented 1 year ago

Not in our lib. I'm testing in the new test angular app ^15.2.1

NetanelBasal commented 1 year ago

Yes, and it yells?

EricPoul commented 1 year ago

It yells. When I build effects-ng I see @angular/router added to peer deps. I think I should check NX config.

NetanelBasal commented 1 year ago

Oh yes, It's Nx. They have a configuration to disable it

EricPoul commented 1 year ago

Yes, I encountered it as I remember but didn't think that it was actually NX

EricPoul commented 1 year ago

That's it. NGRX did the same in December as I see... I'll open PR

NetanelBasal commented 1 year ago

Yes, I know this issue and use it in my repositories. https://github.com/ngneat/elf/blob/master/packages/store/project.json#L10