Store: 馃悶[BUG]: No reset of NgxsModule.forRoot after test run

Created on 24 Jul 2019  路  21Comments  路  Source: ngxs/store

I'm submitting a...


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

Current behavior

I want to test a component that has a @Select() on a state.

@Component({
  selector: 'my-user',
  templateUrl: './user.component.html',
  styleUrls: ['./user.component.scss'],
})
export class UserComponent {
  /** select current user */
  @Select(UserState)
  user$: Observable<UserDto>;

At my test, I prefill the store with:

 beforeEach(async(() => {
    TestBed.configureTestingModule({
      declarations: [UserComponent],
      imports: [
       ...
      ],
    }).compileComponents();

    const store = TestBed.get(Store);
    store.reset({
        user: {
            id: 3,
            name: 'MyUser',
        },
    });

Obviously, this will fail, because I forget the TestBed-Import of the NgxsModule with the according state:

      imports: [
        NgxsModule.forRoot([UserState], {
          developmentMode: true,
        }),
      ],

But, if there is any other test that runs before the UserComponent-test and imports this UserState then my UserComponent-test will succeed.

At the moment, we have over 500 tests running in random order and from time to time there are failures because someone forget to provide the neccessary state at NgxsModule.forRoot(). But with an other random order, this test will succeed again.

Expected behavior

NgxsModule.forRoot() should be isolated for each TestBed.

Environment


Libs:
- @angular/core version: 7.2.15
- @ngxs/store version: 3.3.4


Browser:
- [x] Chrome (desktop) version 75.0.3770 (Windows 10.0.0) 
- [ ] 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: v12.6.0  
- Platform: Windows 10

Others:

core ready to release has PR bufix

Most helpful comment

Indeed, it works! Thank you.

All 21 comments

Please add repo for reproduce

Here is a stackblitz: https://stackblitz.com/edit/ngxs-not-isolating-ngxsmodule

AppComponent uses the UserState. The AppComponent-Spec is successful, but there is no NgxsModule.forRoot([UserState]). This is because there is a HelloComponent-Spec that is running before and provides the state.

@amjmhs Sorry, why you use store.reset in test?

store.reset - overwrites the state always and therefore you can always access it

Works for me (even if I remove the HelloComponent spec)

image

if it is not, give me another example to reproduce

Works for me (even if I remove the HelloComponent spec)
[...]
if it is not, give me another example to reproduce

I think there is a misunderstanding.
Your NgxsModule setup in the test is not the same as in @amjmhs's original Stackblitz.
You added UserState explicitly, but amjmhs 'forgot' to add UserState there on purpose to demonstrate the issue.
The weird behavior is that the test passes if another, completely unrelated spec inits ngxs with the UserState.

BTW: Resetting the store during test setup is complete valid according to ngxs docs.

@arturovt @markwhitfeld please help me for understand, is correct behavior for us?

@markwhitfeld I found big problem

image

If we use in component

 @Select(UserState) user$: Observable<User>;

And forgot add state

NgxsModule.forRoot([])
// next
store.reset({
        userdata: {
          id: 13,
          name: 'John Doe'
        }
});

Then the subscription will not work

But if we use

 @Select((state) => state.userdata) user$: Observable<User>;

Then the subscription will work (even if we do not add state to forRoot)

@markwhitfeld I found problem. This is due to the fact that we mutate meta-information for the state.

@State(..)
class AppState {}


// Analogue

class AppState {
  public static META_KEY = {
     name: 'appState',
     //...
  }
}

Therefore, we need to be able to reset meta information.

Is there any further information on a fix or workaround for this? I think I've hit this same bug, but in my case one test that imports a particular state module is interfering with a completely unrelated test that is also importing the same state module.

@johnmcase You can try to do so

describe('...', () => {
  ///...

  afterAll(() => {
     // restore meta information in state
     MyState['NGXS_META'] = {
      name: null,
      actions: {},
      defaults: {},
      path: null,
      selectFromAppState: null,
      children: []
    };
  });
})

@splincode Thanks for that. I get an error saying that the 'NGXS_META' property is read-only:

TypeError: Cannot assign to read only property 'NGXS_META' of function 'class PersonalContactState {...

Maybe use Object.defineProperty

@amjmhs I have fixed this issue in master.
Please could you test your projects with the latest @dev version and confirm if it is fixed for you.

I forked and upgraded your stackblitz and the tests successfully failed 馃槣 :
https://stackblitz.com/edit/ngxs-not-isolating-ngxsmodule-58ippc

Indeed, it works! Thank you.

Hello @markwhitfeld

I have just updated my app to Angular v11 with [email protected] and It seems this issue is back

Here is updated stackblitz: https://stackblitz.com/edit/ngxs-not-isolating-ngxsmodule-ihldes

Could you please check it out ?

@vojtesaak Please could you describe your angular and NGXS versions before and after the upgrade in order to assist in debugging this issue. There was no change in the handling of metadata in the move from 3.7.0 to 3.7.1, so I suspect that you may be coming from an earlier version?

@markwhitfeld yes you are right, I updated from :

"@angular/core": "^9.1.12",
"@ngxs/store": "^3.6.2",

@vojtesaak could I ask you to do a bit of debugging for me and test with different ngxs versions to see when this bug is introduced?

@markwhitfeld I tested it with versions mentioned in my previous comment and it also didn't work. Here is the code

@vojtesaak Hmmm... If it didn't ever work then you may have picked up a different bug to this one...

Was this page helpful?
0 / 5 - 0 ratings

Related issues

sanchezcarlosjr picture sanchezcarlosjr  路  40Comments

internalsystemerror picture internalsystemerror  路  33Comments

markwhitfeld picture markwhitfeld  路  41Comments

Carniatto picture Carniatto  路  22Comments

thomas-burko picture thomas-burko  路  25Comments