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(compiler): Add extended diagnostic to warn when text attributes … #46161

Closed
wants to merge 3 commits into from

Conversation

atscott
Copy link
Contributor

@atscott atscott commented May 26, 2022

…are intended to be bindings

https://angular.io/guide/attribute-binding#attribute-class-and-style-bindings

Angular supports attr., style., and class. binding prefixes to
bind attributes, styles, and classes. If the key does not have the
binding syntax [] or the value does not have an interpolation {{}},
the attribute will not be interpreted as a binding.

This diagnostic warns the user when the attributes listed above will not
be interpreted as bindings.

resolves #46137

@atscott atscott added feature Issue that requests a new feature area: compiler Issues related to `ngc`, Angular's template compiler target: minor This PR is targeted for the next minor release labels May 26, 2022
@ngbot ngbot bot modified the milestone: Backlog May 26, 2022
@atscott atscott marked this pull request as ready for review June 27, 2022 15:13
@atscott atscott modified the milestones: Backlog, Fixit H1'2022 Jun 27, 2022
…are intended to be bindings

https://angular.io/guide/attribute-binding#attribute-class-and-style-bindings

Angular supports `attr.`, `style.`, and `class.` binding prefixes to
bind attributes, styles, and classes. If the key does not have the
binding syntax `[]` or the value does not have an interpolation `{{}}`,
the attribute will not be interpreted as a binding.

This diagnostic warns the user when the attributes listed above will not
be interpreted as bindings.

resolves angular#46137
Copy link
Contributor

@AndrewKushnir AndrewKushnir left a comment

Choose a reason for hiding this comment

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

Looks great, just a few minor comments.

@pullapprove pullapprove bot requested a review from dylhunn June 27, 2022 21:18
@AndrewKushnir
Copy link
Contributor

@atscott quick question: do you think we should try to include the check for #36256 in the same diagnostics (as a followup PR) or create a new one?

@atscott
Copy link
Contributor Author

atscott commented Jun 28, 2022

@atscott quick question: do you think we should try to include the check for #36256 in the same diagnostics (as a followup PR) or create a new one?

Potentially. Would it be limited to just .px suffix on attr. prefixes? Or warn whenever we encounter a . after the attr.?

@AndrewKushnir
Copy link
Contributor

Potentially. Would it be limited to just .px suffix on attr. prefixes? Or warn whenever we encounter a . after the attr.?

We can probably start with .px and .% suffixes (what is typically used for styles) and expand later if needed.

@atscott
Copy link
Contributor Author

atscott commented Jun 28, 2022

presubmit

Copy link
Contributor

@AndrewKushnir AndrewKushnir left a comment

Choose a reason for hiding this comment

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

Looks great, thanks for addressing the feedback! 👍

@pullapprove pullapprove bot requested a review from alxhub June 30, 2022 20:03
Copy link
Member

@alxhub alxhub 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, fw-compiler

@pullapprove pullapprove bot requested a review from jessicajaniuk June 30, 2022 20:04
Copy link
Member

@jelbourn jelbourn 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

@atscott atscott added the action: merge The PR is ready for merge by the caretaker label Jun 30, 2022
@atscott
Copy link
Contributor Author

atscott commented Jun 30, 2022

This PR was merged into the repository by commit 6f11a58.

@atscott atscott closed this in 6f11a58 Jun 30, 2022
crapStone pushed a commit to Calciumdibromid/CaBr2 that referenced this pull request Jul 24, 2022
This PR contains the following updates:

| Package | Type | Update | Change |
|---|---|---|---|
| [@angular/animations](https://github.com/angular/angular) | dependencies | minor | [`14.0.6` -> `14.1.0`](https://renovatebot.com/diffs/npm/@angular%2fanimations/14.0.6/14.1.0) |
| [@angular/common](https://github.com/angular/angular) | dependencies | minor | [`14.0.6` -> `14.1.0`](https://renovatebot.com/diffs/npm/@angular%2fcommon/14.0.6/14.1.0) |
| [@angular/compiler](https://github.com/angular/angular) | dependencies | minor | [`14.0.6` -> `14.1.0`](https://renovatebot.com/diffs/npm/@angular%2fcompiler/14.0.6/14.1.0) |
| [@angular/compiler-cli](https://github.com/angular/angular) | devDependencies | minor | [`14.0.6` -> `14.1.0`](https://renovatebot.com/diffs/npm/@angular%2fcompiler-cli/14.0.6/14.1.0) |
| [@angular/core](https://github.com/angular/angular) | dependencies | minor | [`14.0.6` -> `14.1.0`](https://renovatebot.com/diffs/npm/@angular%2fcore/14.0.6/14.1.0) |
| [@angular/forms](https://github.com/angular/angular) | dependencies | minor | [`14.0.6` -> `14.1.0`](https://renovatebot.com/diffs/npm/@angular%2fforms/14.0.6/14.1.0) |
| [@angular/platform-browser](https://github.com/angular/angular) | dependencies | minor | [`14.0.6` -> `14.1.0`](https://renovatebot.com/diffs/npm/@angular%2fplatform-browser/14.0.6/14.1.0) |
| [@angular/platform-browser-dynamic](https://github.com/angular/angular) | dependencies | minor | [`14.0.6` -> `14.1.0`](https://renovatebot.com/diffs/npm/@angular%2fplatform-browser-dynamic/14.0.6/14.1.0) |

---

### Release Notes

<details>
<summary>angular/angular</summary>

### [`v14.1.0`](https://github.com/angular/angular/blob/HEAD/CHANGELOG.md#&#8203;1410-2022-07-20)

[Compare Source](angular/angular@14.0.7...14.1.0)

#### Deprecations

##### core

-   The `createNgModuleRef` is deprecated in favor of newly added `createNgModule` one.
-   The bit field signature of `inject()` has been deprecated, in favor of the
    new options object. Correspondingly, `InjectFlags` is deprecated as well.

##### animations

| Commit | Type | Description |
| -- | -- | -- |
| [55308f2df5](angular/angular@55308f2) | feat | add `provideAnimations()` and `provideNoopAnimations()` functions ([#&#8203;46793](angular/angular#46793)) |

##### common

| Commit | Type | Description |
| -- | -- | -- |
| [4a2e7335b1](angular/angular@4a2e733) | feat | make the `CommonModule` pipes standalone ([#&#8203;46401](angular/angular#46401)) |
| [a7597dd080](angular/angular@a7597dd) | feat | make the CommonModule directives standalone ([#&#8203;46469](angular/angular#46469)) |

##### compiler

| Commit | Type | Description |
| -- | -- | -- |
| [33ce3883a5](angular/angular@33ce388) | feat | Add extended diagnostic to warn when missing let on ngForOf ([#&#8203;46683](angular/angular#46683)) |
| [6f11a58040](angular/angular@6f11a58) | feat | Add extended diagnostic to warn when text attributes are intended to be bindings ([#&#8203;46161](angular/angular#46161)) |
| [9e836c232f](angular/angular@9e836c2) | feat | warn when style suffixes are used with attribute bindings ([#&#8203;46651](angular/angular#46651)) |

##### compiler-cli

| Commit | Type | Description |
| -- | -- | -- |
| [93c65e7b14](angular/angular@93c65e7) | feat | add extended diagnostic for non-nullable optional chains ([#&#8203;46686](angular/angular#46686)) |
| [131d029da1](angular/angular@131d029) | feat | detect missing control flow directive imports in standalone components ([#&#8203;46146](angular/angular#46146)) |
| [6b8e60c06a](angular/angular@6b8e60c) | fix | improve the missingControlFlowDirective message ([#&#8203;46846](angular/angular#46846)) |

##### core

| Commit | Type | Description |
| -- | -- | -- |
| [e8e8e5f171](angular/angular@e8e8e5f) | feat | add `createComponent` function |
| [b5153814af](angular/angular@b515381) | feat | add `reflectComponentType` function |
| [96c6139c9a](angular/angular@96c6139) | feat | add ability to set inputs on ComponentRef ([#&#8203;46641](angular/angular#46641)) |
| [a6d5fe202c](angular/angular@a6d5fe2) | feat | alias `createNgModuleRef` as `createNgModule` ([#&#8203;46789](angular/angular#46789)) |
| [71e606d3c3](angular/angular@71e606d) | feat | expose EnvironmentInjector on ApplicationRef ([#&#8203;46665](angular/angular#46665)) |
| [19e6d9ccd3](angular/angular@19e6d9c) | feat | import AsyncStackTaggingZone if available ([#&#8203;46693](angular/angular#46693)) |
| [a7a14df5f8](angular/angular@a7a14df) | feat | introduce `EnvironmentInjector.runInContext` API ([#&#8203;46653](angular/angular#46653)) |
| [fa52b6e906](angular/angular@fa52b6e) | feat | options object to supersede bit flags for `inject()` ([#&#8203;46649](angular/angular#46649)) |
| [af20112222](angular/angular@af20112) | feat | support the descendants option for ContentChild queries ([#&#8203;46638](angular/angular#46638)) |
| [945a3ad359](angular/angular@945a3ad) | fix | Fix `runInContext` for `NgModuleRef` injector ([#&#8203;46877](angular/angular#46877)) |
| [bb7c80477b](angular/angular@bb7c804) | fix | make parent injector argument required in `createEnvironmentInjector` ([#&#8203;46397](angular/angular#46397)) |

##### http

| Commit | Type | Description |
| -- | -- | -- |
| [82acbf919b](angular/angular@82acbf9) | feat | improve error message for nullish header ([#&#8203;46059](angular/angular#46059)) |

##### router

| Commit | Type | Description |
| -- | -- | -- |
| [53ca936366](angular/angular@53ca936) | feat | Add ability to create `UrlTree` from any `ActivatedRouteSnapshot` ([#&#8203;45877](angular/angular#45877)) |
| [de058bba99](angular/angular@de058bb) | feat | Add CanMatch guard to control whether a Route should match ([#&#8203;46021](angular/angular#46021)) |
| [6c1357dd7d](angular/angular@6c1357d) | feat | Add stable cancelation code to `NavigationCancel` event ([#&#8203;46675](angular/angular#46675)) |
| [a4ce273e50](angular/angular@a4ce273) | feat | Add the target `RouterStateSnapshot` to `NavigationError` ([#&#8203;46731](angular/angular#46731)) |
| [abe3759e24](angular/angular@abe3759) | fix | allow to return `UrlTree` from `CanMatchFn` ([#&#8203;46455](angular/angular#46455)) |
| [e8c7dd10e9](angular/angular@e8c7dd1) | fix | Ensure `APP_INITIALIZER` of `enabledBlocking` option completes ([#&#8203;46026](angular/angular#46026)) |
| [ce20ed067f](angular/angular@ce20ed0) | fix | Ensure Route injector is created before running CanMatch guards ([#&#8203;46394](angular/angular#46394)) |
| [6a7b818d94](angular/angular@6a7b818) | fix | Ensure target `RouterStateSnapshot` is defined in `NavigationError` ([#&#8203;46842](angular/angular#46842)) |
| [f94c6f433d](angular/angular@f94c6f4) | fix | Expose CanMatchFn as public API ([#&#8203;46394](angular/angular#46394)) |
| [e8ae0fe3e9](angular/angular@e8ae0fe) | fix | Fix cancellation code for canLoad rejections ([#&#8203;46752](angular/angular#46752)) |

##### upgrade

| Commit | Type | Description |
| -- | -- | -- |
| [e9cb0454dc](angular/angular@e9cb045) | feat | more closely align `UpgradeModule#bootstrap()` with `angular.bootstrap()` ([#&#8203;46214](angular/angular#46214)) |

#### Special Thanks

AleksanderBodurri, Alex Rickabaugh, Andrew Kushnir, Andrew Scott, Cédric Exbrayat, Dmitrij Kuba, Dylan Hunn, George Kalpakas, Jessica Janiuk, JiaLiPassion, Joey Perrott, John Vandenberg, JoostK, Keith Li, Or'el Ben-Ya'ir, Paul Gschwendtner, Pawel Kozlowski, SyedAhm3r, arturovt, mariu, markostanimirovic and mgechev

<!-- CHANGELOG SPLIT MARKER -->

### [`v14.0.7`](https://github.com/angular/angular/blob/HEAD/CHANGELOG.md#&#8203;1407-2022-07-20)

[Compare Source](angular/angular@14.0.6...14.0.7)

##### animations

| Commit | Type | Description |
| -- | -- | -- |
| [5bdbb6285b](angular/angular@5bdbb62) | fix | make sure falsy values are added to \_globalTimelineStyles ([#&#8203;46863](angular/angular#46863)) |

##### compiler

| Commit | Type | Description |
| -- | -- | -- |
| [41253f9c46](angular/angular@41253f9) | fix | inputs/outputs incorrectly parsed in jit mode ([#&#8203;46813](angular/angular#46813)) |

##### core

| Commit | Type | Description |
| -- | -- | -- |
| [4e77c7fbf3](angular/angular@4e77c7f) | fix | do not invoke jasmine `done` callback multiple times with `waitForAsync` |

#### Special Thanks

Andrew Kushnir, Andrew Scott, Bob Watson, Cédric Exbrayat, Doug Parker, George Kalpakas, Jessica Janiuk, Kristiyan Kostadinov, Paul Gschwendtner, acvi, dario-piotrowicz, jnizet and piyush132000

<!-- CHANGELOG SPLIT MARKER -->

</details>

---

### Configuration

📅 **Schedule**: Branch creation - At any time (no schedule defined), Automerge - At any time (no schedule defined).

🚦 **Automerge**: Disabled by config. Please merge this manually once you are satisfied.

♻ **Rebasing**: Whenever PR becomes conflicted, or you tick the rebase/retry checkbox.

🔕 **Ignore**: Close this PR and you won't be reminded about these updates again.

---

 - [ ] <!-- rebase-check -->If you want to rebase/retry this PR, click this checkbox.

---

This PR has been generated by [Renovate Bot](https://github.com/renovatebot/renovate).
<!--renovate-debug:eyJjcmVhdGVkSW5WZXIiOiIzMi4xMTkuMiIsInVwZGF0ZWRJblZlciI6IjMyLjExOS4yIn0=-->

Co-authored-by: cabr2-bot <cabr2.help@gmail.com>
Reviewed-on: https://codeberg.org/Calciumdibromid/CaBr2/pulls/1472
Reviewed-by: Epsilon_02 <epsilon_02@noreply.codeberg.org>
Co-authored-by: Calciumdibromid Bot <cabr2_bot@noreply.codeberg.org>
Co-committed-by: Calciumdibromid Bot <cabr2_bot@noreply.codeberg.org>
@angular-automatic-lock-bot
Copy link

This issue has been automatically locked due to inactivity.
Please file a new issue if you are encountering a similar or related problem.

Read more about our automatic conversation locking policy.

This action has been performed automatically by a bot.

@angular-automatic-lock-bot angular-automatic-lock-bot bot locked and limited conversation to collaborators Jul 31, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
action: merge The PR is ready for merge by the caretaker area: compiler Issues related to `ngc`, Angular's template compiler feature Issue that requests a new feature target: minor This PR is targeted for the next minor release
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add extended diagnostic to warn when an attribute is not interpreted as a binding
4 participants