X Tutup
The Wayback Machine - https://web.archive.org/web/20220321130750/https://github.com/angular/angular/issues/42834
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

zone.js fails to patch mocha v8.3.1 and up #42834

Closed
Alorel opened this issue Jul 13, 2021 · 8 comments
Closed

zone.js fails to patch mocha v8.3.1 and up #42834

Alorel opened this issue Jul 13, 2021 · 8 comments

Comments

@Alorel
Copy link

@Alorel Alorel commented Jul 13, 2021

Which @angular/* package(s) are the source of the bug?

Don't known / other

Is this a regression?

No

Description

I've created a fresh Angular 12 project with the CLI, refactored tests to use Mocha instead of Jasmine and the tests failed.

  • Clone the repo and run npm test - everything will work fine
  • Open package.json and set the mocha dependency to 8.3.1 or higher, then run npm test again
  • You'll now get the exception below.

Please provide a link to a minimal reproduction of the bug

https://github.com/Alorel/zone.js-mocha-patch-bug-reproduction

Please provide the exception or error you saw

RangeError: Maximum call stack size exceeded
      at wrapTestInZone (http://localhost:9876/_karma_webpack_/webpack:/node_modules/zone.js/fesm2015/zone-testing.js:1024:1)
      at global.beforeEach.global.setup.Mocha.beforeEach (http://localhost:9876/_karma_webpack_/webpack:/node_modules/zone.js/fesm2015/zone-testing.js:1077:1)
      at exports.beforeEach (node_modules/mocha/mocha.js:28402:65)
      at global.beforeEach.global.setup.Mocha.beforeEach (http://localhost:9876/_karma_webpack_/webpack:/node_modules/zone.js/fesm2015/zone-testing.js:1077:1)
      at exports.beforeEach (node_modules/mocha/mocha.js:28402:65)
      at global.beforeEach.global.setup.Mocha.beforeEach (http://localhost:9876/_karma_webpack_/webpack:/node_modules/zone.js/fesm2015/zone-testing.js:1077:1)
      at exports.beforeEach (node_modules/mocha/mocha.js:28402:65)
      at global.beforeEach.global.setup.Mocha.beforeEach (http://localhost:9876/_karma_webpack_/webpack:/node_modules/zone.js/fesm2015/zone-testing.js:1077:1)
      at exports.beforeEach (node_modules/mocha/mocha.js:28402:65)
      at global.beforeEach.global.setup.Mocha.beforeEach (http://localhost:9876/_karma_webpack_/webpack:/node_modules/zone.js/fesm2015/zone-testing.js:1077:1)

Please provide the environment you discovered this bug in

Angular CLI: 12.1.1
Node: 14.16.1
Package Manager: npm 7.11.2
OS: linux x64

Angular: 12.1.1
... animations, cli, common, compiler, compiler-cli, core, forms
... platform-browser, platform-browser-dynamic, router

Package                         Version
---------------------------------------------------------
@angular-devkit/architect       0.1201.1
@angular-devkit/build-angular   12.1.1
@angular-devkit/core            12.1.1
@angular-devkit/schematics      12.1.1
@schematics/angular             12.1.1
rxjs                            6.6.7
typescript                      4.3.5

Anything else?

The zone.js version is 0.11.4. The same happens on Angular 11 and zone 0.11.3

@crfrolik
Copy link

@crfrolik crfrolik commented Oct 28, 2021

Would it be possible to get this prioritized? It will impact anyone using Mocha with Angular unit tests.

See also: mochajs/mocha#4660 (comment)

rictic added a commit to rictic/custom-elements-everywhere that referenced this issue Dec 13, 2021
Note that, due to angular/angular#42834 we
had to downgrade our karma-mocha version to use mocha 8.3.0, otherwise
zone.js fails to patch mocha in such a way that beforeEach calls
itself recursively.
@ngbot ngbot bot removed this from the needsTriage milestone Dec 13, 2021
@ngbot ngbot bot added this to the Backlog milestone Dec 13, 2021
@JiaLiPassion
Copy link
Contributor

@JiaLiPassion JiaLiPassion commented Feb 9, 2022

@Alorel , sorry for the late reply, I will check this one.

JiaLiPassion pushed a commit to JiaLiPassion/angular that referenced this issue Feb 11, 2022
Close angular#42834

In the new version fo Mocha, all global test functions are from `global`
object instead of `Mocha` object. Adn the current `zone.js` Mocha
patch's logic looks like this.

```
global.describe = Mocha.describe = function() {
  return originalMochaDescribe.apply(this, arguments);
}
```

and `originalMochaDescribe` is the unpathced Mocha implementation
looks like this

```
function describe() {
  return context.describe(...);
}
```

And the `context` will finally delegate to `global.describe()`,
so the current `zone.js` patch causes infinite loop.

This commit will not patch function of `Mocha` object any longer.
JiaLiPassion pushed a commit to JiaLiPassion/angular that referenced this issue Feb 11, 2022
Close angular#42834

In the new version fo Mocha, all global test functions are from `global`
object instead of `Mocha` object. Adn the current `zone.js` Mocha
patch's logic looks like this.

```
global.describe = Mocha.describe = function() {
  return originalMochaDescribe.apply(this, arguments);
}
```

and `originalMochaDescribe` is the unpathced Mocha implementation
looks like this

```
function describe() {
  return context.describe(...);
}
```

And the `context` will finally delegate to `global.describe()`,
so the current `zone.js` patch causes infinite loop.

This commit will not patch function of `Mocha` object any longer.
alxhub added a commit that referenced this issue Feb 16, 2022
Close #42834

In the new version fo Mocha, all global test functions are from `global`
object instead of `Mocha` object. Adn the current `zone.js` Mocha
patch's logic looks like this.

```
global.describe = Mocha.describe = function() {
  return originalMochaDescribe.apply(this, arguments);
}
```

and `originalMochaDescribe` is the unpathced Mocha implementation
looks like this

```
function describe() {
  return context.describe(...);
}
```

And the `context` will finally delegate to `global.describe()`,
so the current `zone.js` patch causes infinite loop.

This commit will not patch function of `Mocha` object any longer.

PR Close #45047
@juergba
Copy link

@juergba juergba commented Feb 18, 2022

@JiaLiPassion there have been new Angular versions recently, but zone.js@latest is still on v0.11.4.
Do you know when this patch will be published? In zone.js? Thank you.

@JiaLiPassion
Copy link
Contributor

@JiaLiPassion JiaLiPassion commented Feb 18, 2022

@juergba , I am not sure about the release schedule, I will plan to cut a new release after several other issues of zone.js are fixed.

@reduckted
Copy link

@reduckted reduckted commented Mar 2, 2022

@JiaLiPassion Which other issues are you planning to fix before releasing? I'm waiting for this fix to be released, and it would be nice to follow along and potentially help out with those issues if possible.

@jackwagons
Copy link

@jackwagons jackwagons commented Mar 4, 2022

@reduckted @juergba

I noticed v0.11.5 is released. A quick sanity with mocha v9.2.1 passed locally.

@JiaLiPassion
Copy link
Contributor

@JiaLiPassion JiaLiPassion commented Mar 5, 2022

@jackwagons , yes, please update to zone.js 0.11.5 , and thank you for the confirmation.

@reduckted
Copy link

@reduckted reduckted commented Mar 5, 2022

Fantastic, thank you for that.

crapStone pushed a commit to Calciumdibromid/CaBr2 that referenced this issue Mar 8, 2022
This PR contains the following updates:

| Package | Type | Update | Change |
|---|---|---|---|
| [@angular/animations](https://github.com/angular/angular) | dependencies | patch | [`13.2.4` -> `13.2.5`](https://renovatebot.com/diffs/npm/@angular%2fanimations/13.2.4/13.2.5) |
| [@angular/common](https://github.com/angular/angular) | dependencies | patch | [`13.2.4` -> `13.2.5`](https://renovatebot.com/diffs/npm/@angular%2fcommon/13.2.4/13.2.5) |
| [@angular/compiler](https://github.com/angular/angular) | dependencies | patch | [`13.2.4` -> `13.2.5`](https://renovatebot.com/diffs/npm/@angular%2fcompiler/13.2.4/13.2.5) |
| [@angular/compiler-cli](https://github.com/angular/angular) | devDependencies | patch | [`13.2.4` -> `13.2.5`](https://renovatebot.com/diffs/npm/@angular%2fcompiler-cli/13.2.4/13.2.5) |
| [@angular/core](https://github.com/angular/angular) | dependencies | patch | [`13.2.4` -> `13.2.5`](https://renovatebot.com/diffs/npm/@angular%2fcore/13.2.4/13.2.5) |
| [@angular/forms](https://github.com/angular/angular) | dependencies | patch | [`13.2.4` -> `13.2.5`](https://renovatebot.com/diffs/npm/@angular%2fforms/13.2.4/13.2.5) |
| [@angular/platform-browser](https://github.com/angular/angular) | dependencies | patch | [`13.2.4` -> `13.2.5`](https://renovatebot.com/diffs/npm/@angular%2fplatform-browser/13.2.4/13.2.5) |
| [@angular/platform-browser-dynamic](https://github.com/angular/angular) | dependencies | patch | [`13.2.4` -> `13.2.5`](https://renovatebot.com/diffs/npm/@angular%2fplatform-browser-dynamic/13.2.4/13.2.5) |
| [@angular/router](https://github.com/angular/angular) | dependencies | patch | [`13.2.4` -> `13.2.5`](https://renovatebot.com/diffs/npm/@angular%2frouter/13.2.4/13.2.5) |
| [zone.js](https://github.com/angular/angular) ([changelog](https://github.com/angular/angular/blob/master/packages/zone.js/CHANGELOG.md)) | dependencies | patch | [`0.11.4` -> `0.11.5`](https://renovatebot.com/diffs/npm/zone.js/0.11.4/0.11.5) |

---

### Release Notes

<details>
<summary>angular/angular (@&#8203;angular/animations)</summary>

### [`v13.2.5`](https://github.com/angular/angular/blob/HEAD/CHANGELOG.md#&#8203;1325-2022-03-02)

[Compare Source](angular/angular@13.2.4...13.2.5)

##### animations

| Commit | Type | Description |
| -- | -- | -- |
| [6c61d20476](angular/angular@6c61d20) | fix | allow animations with unsupported CSS properties ([#&#8203;45185](angular/angular#45185)) |

##### common

| Commit | Type | Description |
| -- | -- | -- |
| [64da1daa78](angular/angular@64da1da) | fix | canceled JSONP requests won't throw console error with missing callback function ([#&#8203;36807](angular/angular#36807)) |
| [56ca7d385b](angular/angular@56ca7d3) | perf | make `NgLocalization` token tree-shakable ([#&#8203;45118](angular/angular#45118)) ([#&#8203;45226](angular/angular#45226)) |

##### compiler-cli

| Commit | Type | Description |
| -- | -- | -- |
| [6c906a5bb9](angular/angular@6c906a5) | fix | Support resolve animation name from the DTS ([#&#8203;45169](angular/angular#45169)) |

##### core

| Commit | Type | Description |
| -- | -- | -- |
| [e8fd452bd2](angular/angular@e8fd452) | fix | remove individual commands for updating gold files ([#&#8203;45198](angular/angular#45198)) |
| [82d772857c](angular/angular@82d7728) | perf | make `Compiler`, `ApplicationRef` and `ApplicationInitStatus` tree-shakable ([#&#8203;45102](angular/angular#45102)) ([#&#8203;45222](angular/angular#45222)) |
| [71ff12c1cc](angular/angular@71ff12c) | perf | make `LOCALE_ID` and other tokens from `ApplicationModule` tree-shakable ([#&#8203;45102](angular/angular#45102)) ([#&#8203;45222](angular/angular#45222)) |

##### localize

| Commit | Type | Description |
| -- | -- | -- |
| [d388522745](angular/angular@d388522) | fix | avoid imports into `compiler-cli` package ([#&#8203;45180](angular/angular#45180)) |

#### Special Thanks

Andrew Kushnir, Andrew Scott, Charles Lyding, Guillaume Bonnet, Jessica Janiuk, JoostK, Martin Sikora, Paul Gschwendtner, Theodore Brown, dario-piotrowicz and ivanwonder

<!-- CHANGELOG SPLIT MARKER -->

</details>

<details>
<summary>angular/angular (zone.js)</summary>

### [`v0.11.5`](https://github.com/angular/angular/blob/HEAD/packages/zone.js/CHANGELOG.md#&#8203;0115-httpsgithubcomangularangularcomparezonejs-0114zonejs-0115-2022-03-03)

[Compare Source](angular/angular@zone.js-0.11.4...zone.js-0.11.5)

##### Bug Fixes

-   **zone.js:** async-test should only call done once ([#&#8203;45025](angular/angular#45025)) ([dea7234](angular/angular@dea7234))
-   **zone.js:** defineProperties should also set symbol props ([#&#8203;45098](angular/angular#45098)) ([b437d12](angular/angular@b437d12)), closes [#&#8203;44095](angular/angular#44095)
-   **zone.js:** fix several test cases which trigger `done()` multiple times ([#&#8203;45025](angular/angular#45025)) ([d5565cc](angular/angular@d5565cc))
-   **zone.js:** only one listener should also re-throw an error correctly ([#&#8203;41868](angular/angular#41868)) ([299f92c](angular/angular@299f92c)), closes [#&#8203;41867](angular/angular#41867) [/github.com/angular/angular/pull/41562#issuecomment-822696973](https://github.com//github.com/angular/angular/pull/41562/issues/issuecomment-822696973)
-   **zone.js:** patch global instead of Mocha object ([#&#8203;45047](angular/angular#45047)) ([8efbdb5](angular/angular@8efbdb5)), closes [#&#8203;42834](angular/angular#42834)
-   **zone.js:** should continue to executue listeners when throw error ([#&#8203;41562](angular/angular#41562)) ([008eaf3](angular/angular@008eaf3)), closes [#&#8203;41522](angular/angular#41522)
-   **zone.js:** update several flaky cases ([#&#8203;41526](angular/angular#41526)) ([25a83eb](angular/angular@25a83eb)), closes [#&#8203;41434](angular/angular#41434)

##### Features

-   **zone.js:** add Promise.any() implementation ([#&#8203;45064](angular/angular#45064)) ([4d494d2](angular/angular@4d494d2)), closes [#&#8203;44393](angular/angular#44393)
-   **zone.js:** update electron patch to support electron/remote 14 ([#&#8203;45073](angular/angular#45073)) ([d65706a](angular/angular@d65706a)), closes [#&#8203;43346](angular/angular#43346)

</details>

---

### Configuration

📅 **Schedule**: At any time (no schedule defined).

🚦 **Automerge**: Disabled by config. Please merge this manually once you are satisfied.

 **Rebasing**: Whenever PR becomes conflicted, or you tick the rebase/retry checkbox.

👻 **Immortal**: This PR will be recreated if closed unmerged. Get [config help](https://github.com/renovatebot/renovate/discussions) if that's undesired.

---

 - [ ] <!-- rebase-check -->If you want to rebase/retry this PR, click this checkbox.

---

This PR has been generated by [Renovate Bot](https://github.com/renovatebot/renovate).

Co-authored-by: cabr2-bot <cabr2.help@gmail.com>
Reviewed-on: https://codeberg.org/Calciumdibromid/CaBr2/pulls/1197
Reviewed-by: Epsilon_02 <epsilon_02@noreply.codeberg.org>
Co-authored-by: Calciumdibromid Bot <cabr2_bot@noreply.codeberg.org>
Co-committed-by: Calciumdibromid Bot <cabr2_bot@noreply.codeberg.org>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
8 participants
X Tutup