-
Notifications
You must be signed in to change notification settings - Fork 24.8k
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(compiler-cli): Add an extended diagnostic for ngSkipHydration
#49512
Conversation
978367f
to
d6897f9
Compare
nSkipHydration
ngSkipHydration
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.
@JeanMeche huge thanks for the help with this change! 👍
The change looks great, I've just left a few minor comments.
goldens/public-api/compiler-cli/extended_template_diagnostic_name.md
Outdated
Show resolved
Hide resolved
packages/compiler-cli/src/ngtsc/typecheck/extended/checks/skip_hydration_binding/index.ts
Outdated
Show resolved
Hide resolved
packages/compiler-cli/src/ngtsc/typecheck/extended/checks/skip_hydration_binding/index.ts
Outdated
Show resolved
Hide resolved
d6897f9
to
8d10899
Compare
8d10899
to
baffe31
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.
@JeanMeche looks great, just a minor comment related to the error message text.
I also noticed that the commit message refers to the ApplicationRef (likely a commit message that was used in other PR). Could you please update the commit message?
Thank you.
packages/compiler-cli/src/ngtsc/typecheck/extended/checks/skip_hydration_not_static/index.ts
Outdated
Show resolved
Hide resolved
This diagnostic ensures that the special attribute `ngSkipHydration` is not a binding and has no other value than `"true"` or an empty value. Fixes angular#49501
baffe31
to
48272d7
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.
@JeanMeche looks great, thanks for the PR 👍
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.
LGTM!
reviewed-for: public-api
Caretaker note: presubmit is "green". |
This PR was merged into the repository by commit 03d1d00. |
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. |
This diagnostic ensures that the special attribute
ngSkipHydration
is not a binding and has no other value than"true"
or an empty value.Fixes #49501