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
fix(core): add warning when using zoneless but zone.js is still loaded #55769
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM (with one nit comment) but as @JeanMeche noted the negative error code suggest that there are docs for this error, while docs are not part of this PR. Either add docs or flip the negative sign on the error code.
6faed77
to
9c5be5a
Compare
I've switched it to a positive error code. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
reviewed-for: fw-core
packages/core/src/change_detection/scheduling/zoneless_scheduling_impl.ts
Outdated
Show resolved
Hide resolved
packages/core/src/change_detection/scheduling/zoneless_scheduling_impl.ts
Outdated
Show resolved
Hide resolved
packages/core/src/change_detection/scheduling/zoneless_scheduling_impl.ts
Show resolved
Hide resolved
9c5be5a
to
d0ef1ee
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM (just 1 non-blocking comment) 👍
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reviewed-for: public-api
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
reviewed-for: public-api
Users may be using zoneless, but are still loading Zone.js in which case they won't get the full benefits like reduced bundle size. These changes detect such a case and log a warning.
d0ef1ee
to
9d87582
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
reviewed-for: fw-core, public-api
This PR was merged into the repository by commit ae0baa2. |
Just FYI, @angular/fire does not work without Zones, so the only way to use that with zoneless, is to still load zoneJS in polyfills, but use the zoneless change detection. Removal of zoneJS from polyfills will lead to @angular/fire crashing out badly. So for a while , until they get that fixed (if they do), this will give a warning for a lot of people trying out zoneless with @angular/fire. |
Users may be using zoneless, but are still loading Zone.js in which case they won't get the full benefits like reduced bundle size. These changes detect such a case and log a warning.