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(ivy): implement unknown element detection in jit mode #33419
Conversation
errorMessage += | ||
`2. To allow any element add 'NO_ERRORS_SCHEMA' to the '@NgModule.schemas' of this component.`; | ||
} | ||
throw new Error(errorMessage); |
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.
Question for reviewer: should we be throwing here or logging a warning? For the property validation we decided to warn.
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.
We should be warning here. Unfortunately seeing this pretty late :(
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.
I'll submit another PR to switch it to a warning.
FYI, Ivy presubmit indicated a problem where the "unknown element" error (added in this PR) is thrown in the code where it was working before. I will investigate the problem on g3 side and will come up with a TestBed repro. Thank you. |
@crisbeto investigation in g3 revealed that failing tests are misconfigured (where some modules are imported via ngsummaries only and not present in imports list). The error message introduced in this PR and thrown in those tests, is actually a correct Ivy behavior. I will adjust the tests tomorrow to make sure they are passing with upcoming changes. Meanwhile, could you please rebase this PR on top of the latest master (so we run CI again) and may be change commit message to be |
In ViewEngine we used to throw an error if we encountered an unknown element while rendering. We have this already for Ivy in AoT, but we didn't in JiT. These changes implement the error for JiT mode.
36fe443
to
00db79a
Compare
New set of presubmits: |
FYI, VE and Ivy presubmits (including global for Ivy) are successful. Added "merge" label. Thank you. |
In ViewEngine we used to throw an error if we encountered an unknown element while rendering. We have this already for Ivy in AoT, but we didn't in JiT. These changes implement the error for JiT mode. PR Close #33419
…3419) In ViewEngine we used to throw an error if we encountered an unknown element while rendering. We have this already for Ivy in AoT, but we didn't in JiT. These changes implement the error for JiT mode. PR Close angular#33419
…3419) In ViewEngine we used to throw an error if we encountered an unknown element while rendering. We have this already for Ivy in AoT, but we didn't in JiT. These changes implement the error for JiT mode. PR Close angular#33419
This issue has been automatically locked due to inactivity. Read more about our automatic conversation locking policy. This action has been performed automatically by a bot. |
In ViewEngine we used to throw an error if we encountered an unknown element while rendering. We have this already for Ivy in AoT, but we didn't in JiT. These changes implement the error for JiT mode.