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

Error when running tests with Jest #22

Closed
dzhavat opened this issue Oct 3, 2024 · 11 comments
Closed

Error when running tests with Jest #22

dzhavat opened this issue Oct 3, 2024 · 11 comments

Comments

@dzhavat
Copy link

dzhavat commented Oct 3, 2024

Hey,
the library looks great and I just tried it in a project! Works fantastic at run-time but I'm getting errors when running the tests with Jest.

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
[ ] Other... Please describe:

Current behavior

Getting the following error

ReferenceError: Cannot access 'HotToastService' before initialization

      1 | import { Component, inject } from '@angular/core';
      2 | import { RouterOutlet } from '@angular/router';
    > 3 | import { HotToastService } from '@ngxpert/hot-toast';
        | ^
      4 |
      5 | @Component({
      6 |   selector: 'app-root',

      at Object.<anonymous> (node_modules/@ngxpert/hot-toast/fesm2022/ngxpert-hot-toast.mjs:848:188)
      at Object.<anonymous> (src/app/app.component.ts:3:1)
      at Object.<anonymous> (src/app/app.component.spec.ts:2:1)

Expected behavior

Tests run without error.

Note: Found a similar error in another repo. Maybe the fix is similar as well 🤔 valor-software/ng2-charts#1354

Minimal reproduction of the problem with instructions

  1. Clone the repo https://github.com/dzhavat/hot-toast-repro
  2. Install dependencies - npm install
  3. Run the tests - npm run test
  4. See tests fail
  5. Comment out injection of HotToastService in app.component.ts
  6. Run tests again
  7. See them succeed

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

Environment


Angular version: 18.2.7


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: 20.13.1
- Platform:  Windows

Others:

@JeanMeche
Copy link

madge --circular return 2 circular dependencies which might be the cause of this.

1) lib/components/hot-toast-container/hot-toast-container.component.ts > lib/components/hot-toast/hot-toast.component.ts > lib/components/hot-toast-group-item/hot-toast-group-item.component.ts > lib/hot-toast-ref.ts
2) lib/components/hot-toast-container/hot-toast-container.component.ts > lib/hot-toast.service.ts

@shhdharmen
Copy link
Contributor

I quickly checked with default angular tests settings (karma), and it does not throw any error. Maybe it's related to incorrect test configs in jest?

@dzhavat
Copy link
Author

dzhavat commented Oct 5, 2024

Maybe it's related to incorrect test configs in jest?

It could be. The reproduction project uses default Jest settings, so I'll have to dig deeper to see whether it's config related.

I was also trying to solve the circular dependency to see if that would help but haven't used much time on it yet. Will try to look at it during the weekend.

@JeanMeche
Copy link

@shhdharmen Different bundler will result in different output. We've often seen similar issues when people moved from webpack to esbuild.

@shhdharmen
Copy link
Contributor

shhdharmen commented Oct 5, 2024

@JeanMeche I understand. I do not have much expertise in writing test cases, so I would rely on community inputs for this bug. And I also think this is not a bug, but a discussion. What do you think?

@dzhavat I quickly tested with angular team's jest builder, and it worked fine there, too. In your repo, I followed below steps:

  1. Change karma to jest and remove unsupported options
"test": {
          "builder": "@angular-devkit/build-angular:jest", // 👈
          "options": {
            "polyfills": ["zone.js", "zone.js/testing"],
            "tsConfig": "tsconfig.spec.json"
          }
        }
  1. Change test script from jest to ng test in package.json
  2. Run npm run test
  3. Observe that it run successfully

@dzhavat
Copy link
Author

dzhavat commented Oct 6, 2024

@shhdharmen you're right, it works with Angular's Jest builder by making the above adjustments ... buuut I also get a nice warning in the console 🙃

NOTE: The Jest builder is currently EXPERIMENTAL and not ready for production use.

If the repo contains a custom Jest config, I get even more warnings. So while the Jest builder is there ... it's not yet ready for prime time.

Note: The reason it works is because Angular is executing Jest with experimental ESM support.


I just tried commenting out the HotToastService import and all places where it was used inside HotToastContainerComponent (thus removing the circular dependency), and after building the library, the compiled code looked fine. Haven't tried it but am pretty sure that the tests will run without errors too.

Do you think it would be possible to remove the circular dependency?

@dzhavat
Copy link
Author

dzhavat commented Oct 7, 2024

I got some time at work to look at the circular dependency to see if I can contribute to the project with a fix or a way around it. After using a few hours on it, I must acknowledge that it's not that simple as I had hoped for because I don't know the code that well, and the fix is not a "move this small function to another file".

So I found another solution and that is to mock the module import using Jest's moduleNameMapper config.

The config was as easy as

// jest.config.ts
...
moduleNameMapper: {
  '@ngxpert/hot-toast': 'path/to/custom/mock',
},

This was enough for the tests to start running again. It's not the best solution because we'll miss some other features but it's good enough in our case. We can still test the notifications using e2e tests if we have to.

@patrickmichalina
Copy link

Same issue here, mocking it for now

github-actions bot pushed a commit that referenced this issue Dec 10, 2024
# [3.1.0-beta.3](v3.1.0-beta.2...v3.1.0-beta.3) (2024-12-10)

### Bug Fixes

* use inject function instead of decorator ([9b63e61](9b63e61)), closes [#22](#22)
@shhdharmen
Copy link
Contributor

🎉 This issue has been resolved in version 3.1.0-beta.3 🎉

The release is available on:

Your semantic-release bot 📦🚀

github-actions bot pushed a commit that referenced this issue Dec 10, 2024
## [3.1.1-beta.1](v3.1.0...v3.1.1-beta.1) (2024-12-10)

### Bug Fixes

* use inject function instead of decorator ([9b63e61](9b63e61)), closes [#22](#22)
@shhdharmen
Copy link
Contributor

🎉 This issue has been resolved in version 3.1.1-beta.1 🎉

The release is available on:

Your semantic-release bot 📦🚀

github-actions bot pushed a commit that referenced this issue Dec 10, 2024
## [3.1.1](v3.1.0...v3.1.1) (2024-12-10)

### Bug Fixes

* use inject function instead of decorator ([9b63e61](9b63e61)), closes [#22](#22)
Copy link

🎉 This issue has been resolved in version 3.1.1 🎉

The release is available on:

Your semantic-release bot 📦🚀

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

No branches or pull requests

4 participants