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

feat: add support for zone.js 0.11.1 #448

Merged
merged 1 commit into from
Aug 23, 2020
Merged

Conversation

JiaLiPassion
Copy link
Contributor

zone.js 0.11.1 support jest, so all logic of zone-patch are no longer needed,
also zone.js 0.11.1 change the dist bundles to Angular Package Format, so the
current require absolute path will not work any longer.

So this PR make the following changes.

  1. Add logic in setup-jest.ts, to load zone.js and zone.js/testing moudles,
    if the jest patch is already available which means zone.js is 0.11.1+, we don't need to
    import zone.js module and patch ourselves. If the jest patch is not available, we
    still need to use the current logic as fallback.
  2. Add e2e test for Angular 10 with zone.js 0.11.1

@JiaLiPassion JiaLiPassion requested a review from thymikee as a code owner August 20, 2020 12:18
@JiaLiPassion
Copy link
Contributor Author

@ahnpnl , please review, thank you!

@ahnpnl ahnpnl requested review from wtho and ahnpnl August 20, 2020 12:20
@JiaLiPassion
Copy link
Contributor Author

The CI is failing, but it works in my local, will check what is wrong.

e2e/test-app-v10-zone-v11/package.json Outdated Show resolved Hide resolved
src/setup-jest.ts Outdated Show resolved Hide resolved
@JiaLiPassion JiaLiPassion marked this pull request as draft August 20, 2020 13:11
src/setup-jest.ts Outdated Show resolved Hide resolved
@JiaLiPassion
Copy link
Contributor Author

@ahnpnl , I modified the logic, we don't need to check the __zone_patch__ flag any more, since zone.js 0.11.1 change the bundle folder structure, we just need to load it and check whether there is error. Please review, thanks!

@JiaLiPassion JiaLiPassion marked this pull request as draft August 21, 2020 03:41
@ahnpnl
Copy link
Collaborator

ahnpnl commented Aug 21, 2020

thanks a lot @JiaLiPassion !!, the changes LGTM.

@wtho @thymikee would you please also take a look ?

@JiaLiPassion
Copy link
Contributor Author

@ahnpnl , thanks, still need some final tuning, the bundle name is not correct, will update in a minute.

@JiaLiPassion JiaLiPassion marked this pull request as ready for review August 21, 2020 06:45
@JiaLiPassion
Copy link
Contributor Author

@ahnpnl, I have updated the commit, now it is ok for review, thanks.

// Fallback logic to load zone and zone-patch
// when the user still use zone.js 0.10.x
// TODO: @JiaLiPassion, remove the fallback when Angular 10 LTS finished.
require('zone.js/dist/zone');
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure if we can remove this fallback later because the preset supposes to support various Angular versions. If removing fallback means we don't have backward compatibility anymore for Angular < 9 ?

Copy link
Contributor Author

@JiaLiPassion JiaLiPassion Aug 21, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, totally depends on the policy of the library, of course we can support any version of Angular with the latest version of the jest-preset-angular, or we can support different version of Angular with different versions of the jest-preset-angular. I just removed the comment.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I talked to thymikee and the preset will follow Angular support policy starting from next major version. Once we do that, at some points we can drop this fallback.

zone.js 0.11.1 support jest, so all logic of `zone-patch` are no longer needed,
also zone.js 0.11.1 change the `dist` bundles to `Angular Package Format`, so the
current `require` absolute path will not work any longer.

So this PR make the following changes.
1. Add logic in `setup-jest.ts`, to load `zone.js` and `zone.js/testing` moudles,
if the jest patch is already available which means zone.js is 0.11.1+, we don't need to
import zone.js module and patch ourselves. If the jest patch is not available, we
still need to use the current logic as fallback.
2. Add e2e test for Angular 10 with zone.js 0.11.1
@ahnpnl ahnpnl merged commit 3879976 into thymikee:master Aug 23, 2020
ahnpnl added a commit that referenced this pull request Aug 23, 2020
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

Successfully merging this pull request may close these issues.

4 participants