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

regression in zone patch in 8.1.1 with it.each as tagged template literal #359

Closed
bgoscinski opened this issue Mar 13, 2020 · 3 comments · Fixed by #361
Closed

regression in zone patch in 8.1.1 with it.each as tagged template literal #359

bgoscinski opened this issue Mar 13, 2020 · 3 comments · Fixed by #361
Labels

Comments

@bgoscinski
Copy link

bgoscinski commented Mar 13, 2020

The fix released in 8.1.1 didn't consider usage of it.each as tagged template literal and currently it breaks with

 FAIL  src/app/app.component.spec.ts (12.248s)
  ● AppComponent › encountered a declaration exception

    TypeError: row.map is not a function

      113 |     foo  | bar
      114 |     ${1} | ${2}
    > 115 |   `('it.each should work with table as a tagged template literal', ({foo, bar}) => {
          |    ^
      116 |     expect(foo).toBe(1);
      117 |     expect(bar).toBe(2);
      118 |   });

      at node_modules/jest-preset-angular/build/zone-patch/index.js:137:73
          at Array.map (<anonymous>)
      at src/app/app.component.spec.ts:115:4
      at Object.<anonymous> (src/app/app.component.spec.ts:9:1)

This usage works in 8.1.0. I'll submit a PR with regression test in example app it's here: #360

@bgoscinski bgoscinski changed the title regression in zone patch in 8.1.1 regression in zone patch in 8.1.1 with it.each as tagged template literal Mar 13, 2020
@ahnpnl
Copy link
Collaborator

ahnpnl commented Mar 13, 2020

cc @JiaLiPassion could you please take a look ?

@JiaLiPassion
Copy link
Contributor

@ahnpnl , sure , will check it now.

@JiaLiPassion
Copy link
Contributor

@ahnpnl , I created a refactor PR to simplify the logic and handle all the cases, it should work for all the known cases for now. And @bgoscinski thank you for the test cases.

@wtho wtho closed this as completed in #361 Mar 15, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants