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

Resolve host binding type issues #60481

Closed
wants to merge 6 commits into from

Conversation

crisbeto
Copy link
Member

@crisbeto crisbeto commented Mar 20, 2025

Updates our build setup to type check host bindings in our own directives, and fixes any pre-existing issues. This is a prerequisite for enabling the new type checking internally.

@crisbeto crisbeto added action: review The PR is still awaiting reviews from at least one requested reviewer target: major This PR is targeted for the next major release labels Mar 20, 2025
@angular-robot angular-robot bot added area: build & ci Related the build and CI infrastructure of the project area: common Issues related to APIs in the @angular/common package area: forms labels Mar 20, 2025
@ngbot ngbot bot modified the milestone: Backlog Mar 20, 2025
@@ -279,7 +274,8 @@ export interface ImagePlaceholderConfig {
'[style.background-position]': 'placeholder ? "50% 50%" : null',
'[style.background-repeat]': 'placeholder ? "no-repeat" : null',
'[style.background-image]': 'placeholder ? generatePlaceholder(placeholder) : null',
'[style.filter]': `placeholder && shouldBlurPlaceholder(placeholderConfig) ? "blur(${PLACEHOLDER_BLUR_AMOUNT}px)" : null`,
'[style.filter]':
'placeholder && shouldBlurPlaceholder(placeholderConfig) ? "blur(15px)" : null',
Copy link
Member Author

Choose a reason for hiding this comment

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

I removed the string interpolation here, because it was preventing type checking. It also seemed like it was there purely for the unit tests.

@@ -687,7 +683,7 @@ export class NgOptimizedImage implements OnInit, OnChanges {
* * A base64 encoded image, which is wrapped and passed through.
* * A boolean. If true, calls the image loader to generate a small placeholder url.
*/
private generatePlaceholder(placeholderInput: string | boolean): string | boolean | null {
protected generatePlaceholder(placeholderInput: string | boolean): string | boolean | null {
Copy link
Member Author

Choose a reason for hiding this comment

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

These had to be made protected, because it's not allowed to use private methods in templates/host bindings.

@@ -46,7 +46,7 @@ const CHECKBOX_VALUE_ACCESSOR: Provider = {
@Directive({
selector:
'input[type=checkbox][formControlName],input[type=checkbox][formControl],input[type=checkbox][ngModel]',
host: {'(change)': 'onChange($event.target.checked)', '(blur)': 'onTouched()'},
host: {'(change)': 'onChange($any($event.target).checked)', '(blur)': 'onTouched()'},
Copy link
Member Author

Choose a reason for hiding this comment

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

These are cast to any, because $event.target is an EventTarget and we have no way of casting it to HTMLInputElement. The other option we be to have a type assertion in a separate method, but I didn't want to add more runtime code here.

@crisbeto crisbeto force-pushed the host-binding-errors branch 2 times, most recently from e305e2e to 6d83cbf Compare March 20, 2025 09:34
@angular-robot angular-robot bot added the area: docs-infra Angular.dev application and infrastructure label Mar 20, 2025
@crisbeto crisbeto force-pushed the host-binding-errors branch 5 times, most recently from c27cef3 to 25d60e4 Compare March 20, 2025 10:15

Verified

This commit was signed with the committer’s verified signature.
riyavsinha Riya Sinha
Enables strict templates and type checking of host bindings against our own code.

Verified

This commit was signed with the committer’s verified signature.
riyavsinha Riya Sinha
Fixes type checking issues in the dev tools that weren't showing up, because we had `strictTemplates` turned off.
Fixes type issues in the host bindings of `NgOptimizedImage`.
Fixes some type issues that are being flagged now that we have type checking of host bindings.
Resolves typing issues inside host binding in adev.
Fixes some type checking issues in our own testing code that weren't showing up, because `strictTemplates` was turned off.
@crisbeto crisbeto force-pushed the host-binding-errors branch from 25d60e4 to 4d6d1cc Compare March 20, 2025 10:42
@crisbeto crisbeto marked this pull request as ready for review March 20, 2025 11:57
@pullapprove pullapprove bot requested a review from josephperrott March 20, 2025 11:57
Copy link
Member

@josephperrott josephperrott left a comment

Choose a reason for hiding this comment

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

LGTM

@pullapprove pullapprove bot requested a review from alxhub March 20, 2025 13:43
@pullapprove pullapprove bot requested review from devversion and mmalerba March 20, 2025 13:43
Copy link
Contributor

@thePunderWoman thePunderWoman left a 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

@pullapprove pullapprove bot requested a review from devversion March 20, 2025 14:06
Copy link
Member

@devversion devversion left a 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

@crisbeto crisbeto added action: merge The PR is ready for merge by the caretaker and removed action: review The PR is still awaiting reviews from at least one requested reviewer labels Mar 20, 2025
@crisbeto crisbeto removed request for alxhub and mmalerba March 20, 2025 15:22
@alxhub
Copy link
Member

alxhub commented Mar 20, 2025

This PR was merged into the repository by commit 911ad40.

The changes were merged into the following branches: main

@alxhub alxhub closed this in 2637a0c Mar 20, 2025
alxhub pushed a commit that referenced this pull request Mar 20, 2025
Fixes type checking issues in the dev tools that weren't showing up, because we had `strictTemplates` turned off.

PR Close #60481
alxhub pushed a commit that referenced this pull request Mar 20, 2025
Fixes type issues in the host bindings of `NgOptimizedImage`.

PR Close #60481
alxhub pushed a commit that referenced this pull request Mar 20, 2025
Fixes some type issues that are being flagged now that we have type checking of host bindings.

PR Close #60481
alxhub pushed a commit that referenced this pull request Mar 20, 2025
Resolves typing issues inside host binding in adev.

PR Close #60481
alxhub pushed a commit that referenced this pull request Mar 20, 2025
Fixes some type checking issues in our own testing code that weren't showing up, because `strictTemplates` was turned off.

PR Close #60481
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
action: merge The PR is ready for merge by the caretaker area: build & ci Related the build and CI infrastructure of the project area: common Issues related to APIs in the @angular/common package area: devtools area: docs-infra Angular.dev application and infrastructure area: forms target: major This PR is targeted for the next major release
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants