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

Circular dependency #632

Closed
hags033 opened this issue Oct 30, 2018 · 11 comments
Closed

Circular dependency #632

hags033 opened this issue Oct 30, 2018 · 11 comments

Comments

@hags033
Copy link

hags033 commented Oct 30, 2018

I'm submitting a...


[ ] Regression (a behavior that used to work and stopped working in a new release)
[ ] Bug report  
[x] Performance issue
[ ] Feature request
[ ] Documentation issue or request
[ ] Support request => https://github.com/ngxs/store/blob/master/CONTRIBUTING.md
[ ] Other... Please describe:

Current behavior

With 2 stores, if you do a selectSnapshot in each from the other store you get a circular dependency warning.

Expected behavior

No circular dependency warning

Minimal reproduction of the problem with instructions

Create 2 stores and do a select snapshot from each

What is the motivation / use case for changing the behavior?

Environment


Libs:
- @angular/core version: X.Y.Z
- @ngxs/store version: X.Y.Z


Browser:
- [ ] Chrome (desktop) version XX
- [ ] Chrome (Android) version XX
- [ ] Chrome (iOS) version XX
- [ ] Firefox version XX
- [ ] Safari (desktop) version XX
- [ ] Safari (iOS) version XX
- [ ] IE version XX
- [ ] Edge version XX
 
For Tooling issues:
- Node version: XX  
- Platform:  

Others:

I understand why this is happening, but in reality there shouldn't be a warning for a circular dependency or another way to get a slice of the other stores state without creating the dependency.

@splincode
Copy link
Member

Please, add example on stackblitz for reproduce

@hags033
Copy link
Author

hags033 commented Oct 31, 2018

Not sure if we will see the circular dependency warning on stackblitz, I also tried to do a selectSnapshot in a stackblitz ngxs https://stackblitz.com/edit/angular-ngxs-app
but the app wouldn't recognize the property I was trying to select (ex: in router.state.ts in an action I tried to get this.store.selectSnapshot(AppState.username) and it couldn't find username even though it's clearly defined in the app.state.ts).

To reproduce, if you have an app that has 2 stores, just do a selectSnapshot() from each and at build time you get the circular dependency warning.

@hags033
Copy link
Author

hags033 commented Oct 31, 2018

I was able to recreate in stackblitz but since you can't see warnings I have attached the zip file, just install and ng serve.
angular-ngxs-app (1).zip

@eranshmil
Copy link
Member

This is a JavaScript/TypeScript issue.
You need to avoid importing from both files.
What is your goal in doing that?

@hags033
Copy link
Author

hags033 commented Oct 31, 2018

We are using ngxs in a very big app, with multiple stores. With our actions in one store we need an attribute from another store, so we use selectSnapshot to get it. But with the circular dependency we can only get snapshots inside of one store, because if we use selectsnapshot from the first store we use it originally in, it gives the circular dependency warning. I'm also not importing one state into the other, I'm only importing the top level store: Store in the constructor, and using this.store.selectSnapshot(otherstore.attribute).

@eranshmil
Copy link
Member

You import FakeAppState in app.state.ts and AppState in fake.state.ts.

I still don't understand what you're trying to achieve.
In your example, you can take the username from the action's payload, so you don't need to take the username from the other state.

@hags033
Copy link
Author

hags033 commented Oct 31, 2018

It's just a very simple re-creation to show the circular dependency. Our app is much more complex currently with 4 stores and over 100 actions, so setting up optional payloads just to get pieces of another state as work around isn't viable.

So basically I am trying to see if there is a fix on your end, as this actually seems like a common function to get snapshots from other states to use in actions. Or if there is a better way to get that slice of state without the circular dependency.

@eranshmil
Copy link
Member

eranshmil commented Oct 31, 2018

I guess this is your only option:

this.store.selectSnapshot(state => state.fakeState.username);
// or
this.store.selectSnapshot('fakeState.username');

And get rid of the imports of both states.

@hags033
Copy link
Author

hags033 commented Oct 31, 2018

This seems to work, thank you.

@hags033 hags033 closed this as completed Oct 31, 2018
@Dedme
Copy link

Dedme commented Sep 25, 2019

hate to bring up an old issue, but i am having this exact issue, but with a dynamic selector.
i am unable to use the solution provided, because the dynamic selectors arent on the state like that

@splincode
Copy link
Member

@Dedme please add example for reproduce

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

4 participants