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(core): add ability to transform input values #50420

Closed
wants to merge 1 commit into from

Conversation

crisbeto
Copy link
Member

@crisbeto crisbeto commented May 23, 2023

According to the HTML specification most attributes are defined as strings, however some can be interpreted as different types like booleans or numbers. In the HTML standard, boolean attributes are considered true if they are present on a DOM node and false if they are omitted. Common examples of boolean attributes are disabled on interactive elements like <button> or checked on <input type="checkbox">. Another example of an attribute that is defined as a string, but interpreted as a different type is the value attribute of <input type="number"> which logs a warning and ignores the value if it can't be parsed as a number.

Historically, authoring Angular inputs that match the native behavior in a type-safe way has been difficult for developers, because Angular interprets all static attributes as strings. While some recent TypeScript versions made this easier by allowing setters and getters to have different types, supporting this pattern still requires a lot of boilerplate and additional properties to be declared. For example, currently developers have to write something like this to have a disabled input that behaves like the native one:

import {Directive, Input} from '@angular/core';

@Directive({selector: 'mat-checkbox'})
export class MatCheckbox {
  @Input()
  get disabled() {
    return this._disabled;
  }
  set disabled(value: any) {
    this._disabled = typeof value === 'boolean' ? value : (value != null && value !== 'false');
  }
  private _disabled = false;
}

This feature aims to address the issue by introducing a transform property on inputs. If an input has a transform function, any values set through the template will be passed through the function before being assigned to the directive instance. The example from above can be rewritten to the following:

import {Directive, Input, booleanAttribute} from '@angular/core';

@Directive({selector: 'mat-checkbox'})
export class MatCheckbox {
  @Input({transform: booleanAttribute}) disabled: boolean = false;
}

These changes also add the booleanAttribute and numberAttribute utilities to @angular/core since they're common enough to be useful for most projects.

Fixes #8968.
Fixes #14761.

@crisbeto crisbeto added action: review The PR is still awaiting reviews from at least one requested reviewer area: core Issues related to the framework runtime target: minor This PR is targeted for the next minor release labels May 23, 2023
@ngbot ngbot bot modified the milestone: Backlog May 23, 2023
@angular-robot angular-robot bot added the detected: feature PR contains a feature commit label May 23, 2023
@crisbeto crisbeto marked this pull request as ready for review May 23, 2023 09:30
};
} else {
result[key] = {
bindingPropertyName: value[0],
classPropertyName: value[1],
transformFunction: value[2] || null,
Copy link
Contributor

Choose a reason for hiding this comment

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

Shouldn't you keep these properties sorted because of *morphism? (or at least in the same order as other usages)

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'm not sure if that makes a difference. My understanding is that only the shape of the object matters (e.g. it has the same set of properties).

Copy link

Choose a reason for hiding this comment

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

actually the order in which you add the properties is relevant and that also applies to object literals

According to the HTML specification most attributes are defined as strings, however some can be interpreted as different types like booleans or numbers. [In the HTML standard](https://html.spec.whatwg.org/multipage/common-microsyntaxes.html#boolean-attributes), boolean attributes are considered `true` if they are present on a DOM node and `false` if they are omitted. Common examples of boolean attributes are `disabled` on interactive elements like `<button>` or `checked` on `<input type="checkbox">`. Another example of an attribute that is defined as a string, but interpreted as a different type is the `value` attribute of `<input type="number">` which logs a warning and ignores the value if it can't be parsed as a number.

Historically, authoring Angular inputs that match the native behavior in a type-safe way has been difficult for developers, because Angular interprets all static attributes as strings. While some recent TypeScript versions made this easier by allowing setters and getters to have different types, supporting this pattern still requires a lot of boilerplate and additional properties to be declared. For example, currently developers have to write something like this to have a `disabled` input that behaves like the native one:

```typescript
import {Directive, Input} from '@angular/core';

@directive({selector: 'mat-checkbox'})
export class MatCheckbox {
  @input()
  get disabled() {
    return this._disabled;
  }
  set disabled(value: any) {
    this._disabled = typeof value === 'boolean' ? value : (value != null && value !== 'false');
  }
  private _disabled = false;
}
```

This feature aims to address the issue by introducing a `transform` property on inputs. If an input has a `transform` function, any values set through the template will be passed through the function before being assigned to the directive instance. The example from above can be rewritten to the following:

```typescript
import {Directive, Input, booleanAttribute} from '@angular/core';

@directive({selector: 'mat-checkbox'})
export class MatCheckbox {
  @input({transform: booleanAttribute}) disabled: boolean = false;
}
```

These changes also add the `booleanAttribute` and `numberAttribute` utilities to `@angular/core` since they're common enough to be useful for most projects.

Fixes angular#8968.
Fixes angular#14761.
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! 👍

AndrewKushnir

This comment was marked as duplicate.

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.

Reviewed-for: public-api

@ajafff
Copy link

ajafff commented May 26, 2023

Since there is no documentation yet, here are a few things I observed, which might be good to know for others interested in this change:

  • there's a test to ensure that this inside of the transform function is the directive instance. this is currently not reflected in the type of the Input#transform option
  • it looks like transform is called before comparing with the previous transformed values, so that the directive is not marked dirty if the transform function returns the same value for different input values. but there's no test for that

@crisbeto
Copy link
Member Author

crisbeto commented May 26, 2023

@ajafff

there's a test to ensure that this inside of the transform function is the directive instance. this is currently not reflected in the type of the Input#transform option

There's no way to get the this type in a decorator so that's why it isn't reflected. It would be up to the consumer to specify it.

it looks like transform is called before comparing with the previous transformed values, so that the directive is not marked dirty if the transform function returns the same value for different input values. but there's no test for that

This is incorrect, the directive will be marked as dirty, however ngOnChanges won't fire if the transform function returns the same value. This is difficult to fix, because we run change detection on a per-binding level, e.g. any time the expression in [someInput]="expr" changes, rather than per directive input. Fixing it would require a large (and likely breaking) refactor in the change detection system and would lead to higher memory usage since we would have to store more information. It's worth noting that setters have the same problem at the moment.

Copy link
Contributor

@dylhunn dylhunn 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: fw-core, fw-forms, 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 May 26, 2023
@ajafff
Copy link

ajafff commented May 26, 2023

@crisbeto

There's no way to get the this type in a decorator so that's why it isn't reflected. It would be up to the consumer to specify it.

That's not correct. The following solution works with experimentalDecorators. Not sure if/how it would work with ES decorators. Also not sure how breaking this would be. And it probably won't work with the proposed/upcoming signal inputs.

// @experimentalDecorators: true
declare function Input<This extends object, In, K extends keyof This>(
    transform: (this: This, v: In) => This[K] // provide 'this' in contextual type, ensure return type matches
): (target: This, key: K) => void;

https://www.typescriptlang.org/play?experimentalDecorators=true#code/CYUwxgNghgTiAEAzArgOzAFwJYHtXwElUAHZDAHgBUALLAZ3hAA8MRVgGcAjAK3AwA0hVEIDSjFmw7wA1iACeORPBr0AfAAoAUPF3wMMKKjqIcMALYAueBoy0611XSEA3a0QCU8ALxqV9gG1RAF0tD2tbWABzEAxHeyE5eWtRL194FxwsYABuLS1IKDoGAGF4AG8dPSgfeABGPL14AAEiUgwNFHRsPBs3eDoDLFQor0qmprt6ADoa73gAZhz4AHoV+AB3MxkGHmRBpGGQKon4OAxkGHwAahdGvQBfDxP4LlqAJjz8prX4MDM4JgIPIBtQcBsGEZGDAYGZ4FhlAByKZ0RHwYiwKDmWIgGDwYA4EB0VDI+DmKAYMDUF6tEhkTpoTC4fC2ezWcpMayDGDDKIPVzWVDIcxcXFjF5Nc6XfB3F5PF5gD5fH7rf4w-jA0HgyH4XGwvEGIwmMzY4D6eTEBAEokkjBkilU9Gwy0wDAgt2WmlteldJm9DT9bm88WnXRSq4Ze66eVNM3zT5aB5AA

the directive will be marked as dirty, however ngOnChanges won't fire if the transform function returns the same value.

I see. I mixed up DirectiveDef#setInput() which is used here and ComponentRef#setInput()

@crisbeto
Copy link
Member Author

@ajafff that's an interesting approach, but it doesn't appear to work with arrow functions. Either way, I'll keep the type annotations as they are and we an iterate over it once the feature is merged.

@dylhunn
Copy link
Contributor

dylhunn commented May 30, 2023

This PR was merged into the repository by commit 68017d4.

@dylhunn dylhunn closed this in 68017d4 May 30, 2023
@CodyTolene
Copy link

Excited to test this ❤️

@flobacher
Copy link

Great!
Would be awesome if this would work in combiniation with the directive-composition api, so one can transform inputs before the are provided to the host-directive... I just tested this.. the transform works, but is applied after the input value is passed on to the host-directive..

crapStone pushed a commit to Calciumdibromid/CaBr2 that referenced this pull request Jun 15, 2023
This PR contains the following updates:

| Package | Type | Update | Change |
|---|---|---|---|
| [@angular/animations](https://github.com/angular/angular) | dependencies | minor | [`16.0.4` -> `16.1.1`](https://renovatebot.com/diffs/npm/@angular%2fanimations/16.0.4/16.1.1) |
| [@angular/common](https://github.com/angular/angular) | dependencies | minor | [`16.0.4` -> `16.1.1`](https://renovatebot.com/diffs/npm/@angular%2fcommon/16.0.4/16.1.1) |
| [@angular/compiler](https://github.com/angular/angular) | dependencies | minor | [`16.0.4` -> `16.1.1`](https://renovatebot.com/diffs/npm/@angular%2fcompiler/16.0.4/16.1.1) |
| [@angular/compiler-cli](https://github.com/angular/angular/tree/main/packages/compiler-cli) ([source](https://github.com/angular/angular)) | devDependencies | minor | [`16.0.4` -> `16.1.1`](https://renovatebot.com/diffs/npm/@angular%2fcompiler-cli/16.0.4/16.1.1) |
| [@angular/core](https://github.com/angular/angular) | dependencies | minor | [`16.0.4` -> `16.1.1`](https://renovatebot.com/diffs/npm/@angular%2fcore/16.0.4/16.1.1) |
| [@angular/forms](https://github.com/angular/angular) | dependencies | minor | [`16.0.4` -> `16.1.1`](https://renovatebot.com/diffs/npm/@angular%2fforms/16.0.4/16.1.1) |
| [@angular/platform-browser](https://github.com/angular/angular) | dependencies | minor | [`16.0.4` -> `16.1.1`](https://renovatebot.com/diffs/npm/@angular%2fplatform-browser/16.0.4/16.1.1) |
| [@angular/platform-browser-dynamic](https://github.com/angular/angular) | dependencies | minor | [`16.0.4` -> `16.1.1`](https://renovatebot.com/diffs/npm/@angular%2fplatform-browser-dynamic/16.0.4/16.1.1) |
| [zone.js](https://github.com/angular/angular) ([changelog](https://github.com/angular/angular/blob/master/packages/zone.js/CHANGELOG.md)) | dependencies | patch | [`0.13.0` -> `0.13.1`](https://renovatebot.com/diffs/npm/zone.js/0.13.0/0.13.1) |

---

### Release Notes

<details>
<summary>angular/angular (@&#8203;angular/animations)</summary>

### [`v16.1.1`](https://github.com/angular/angular/blob/HEAD/CHANGELOG.md#&#8203;1611-2023-06-14)

[Compare Source](angular/angular@16.1.0...16.1.1)

##### compiler-cli

| Commit | Type | Description |
| -- | -- | -- |
| [71360b3a3e](angular/angular@71360b3) | fix | libraries compiled with v16.1+ breaking with Angular framework v16.0.x ([#&#8203;50715](angular/angular#50715)) |

##### core

| Commit | Type | Description |
| -- | -- | -- |
| [d9bed48eb5](angular/angular@d9bed48) | fix | extend toSignal to accept any Subscribable ([#&#8203;50162](angular/angular#50162)) |

##### migrations

| Commit | Type | Description |
| -- | -- | -- |
| [5e1d8444ae](angular/angular@5e1d844) | fix | Prevent a component from importing itself. ([#&#8203;50554](angular/angular#50554)) |

<!-- CHANGELOG SPLIT MARKER -->

### [`v16.1.0`](https://github.com/angular/angular/blob/HEAD/CHANGELOG.md#&#8203;1610-2023-06-13)

[Compare Source](angular/angular@16.0.6...16.1.0)

##### compiler

| Commit | Type | Description |
| -- | -- | -- |
| [4e663297c5](angular/angular@4e66329) | fix | error when reading compiled input transforms metadata in JIT mode ([#&#8203;50600](angular/angular#50600)) |
| [721bc72649](angular/angular@721bc72) | fix | resolve deprecation warning with TypeScript 5.1 ([#&#8203;50460](angular/angular#50460)) |

##### core

| Commit | Type | Description |
| -- | -- | -- |
| [68017d4e75](angular/angular@68017d4) | feat | add ability to transform input values ([#&#8203;50420](angular/angular#50420)) |
| [69dadd2502](angular/angular@69dadd2) | feat | support TypeScript 5.1 ([#&#8203;50156](angular/angular#50156)) |
| [c0ebe34cbd](angular/angular@c0ebe34) | fix | add additional component metadata to component ID generation ([#&#8203;50336](angular/angular#50336)) |

##### http

| Commit | Type | Description |
| -- | -- | -- |
| [85c5427582](angular/angular@85c5427) | feat | Introduction of the `fetch` Backend for the `HttpClient` ([#&#8203;50247](angular/angular#50247)) |

<!-- CHANGELOG SPLIT MARKER -->

### [`v16.0.6`](https://github.com/angular/angular/blob/HEAD/CHANGELOG.md#&#8203;1606-2023-06-13)

[Compare Source](angular/angular@16.0.5...16.0.6)

##### core

| Commit | Type | Description |
| -- | -- | -- |
| [05ac0868c9](angular/angular@05ac086) | fix | avoid duplicated content during hydration while processing a component with i18n ([#&#8203;50644](angular/angular#50644)) |

<!-- CHANGELOG SPLIT MARKER -->

### [`v16.0.5`](https://github.com/angular/angular/blob/HEAD/CHANGELOG.md#&#8203;1605-2023-06-08)

[Compare Source](angular/angular@16.0.4...16.0.5)

##### compiler

| Commit | Type | Description |
| -- | -- | -- |
| [703b8fcac1](angular/angular@703b8fc) | fix | do not remove comments in component styles ([#&#8203;50346](angular/angular#50346)) |

##### core

| Commit | Type | Description |
| -- | -- | -- |
| [2b6da93e19](angular/angular@2b6da93) | fix | incorrectly throwing error for self-referencing component ([#&#8203;50559](angular/angular#50559)) |
| [c992109d6c](angular/angular@c992109) | fix | wait for HTTP in `ngOnInit` correctly before server render ([#&#8203;50573](angular/angular#50573)) |

##### platform-server

| Commit | Type | Description |
| -- | -- | -- |
| [c0d4086c6e](angular/angular@c0d4086) | fix | surface errors during rendering ([#&#8203;50587](angular/angular#50587)) |

<!-- CHANGELOG SPLIT MARKER -->

</details>

<details>
<summary>angular/angular (zone.js)</summary>

### [`v0.13.1`](https://github.com/angular/angular/blob/HEAD/packages/zone.js/CHANGELOG.md#v0131-httpsgithubcomangularangularcomparezonejs-0130zonejs-v0131-2023-06-09)

[Compare Source](angular/angular@zone.js-0.13.0...zone.js-0.13.1)

##### Bug Fixes

-   **zone.js:** enable monkey patching of the `queueMicrotask()` API in node.js ([#&#8203;50467](angular/angular#50467)) ([381cb98](angular/angular@381cb98))
-   **zone.js:** enable monkey patching of the `queueMicrotask()` API in node.js ([#&#8203;50530](angular/angular#50530)) ([7837f71](angular/angular@7837f71))
-   **zone.js:** patch entire promise in node ([#&#8203;50552](angular/angular#50552)) ([cb31dbc](angular/angular@cb31dbc)), closes [#&#8203;50513](angular/angular#50513) [#&#8203;50457](angular/angular#50457) [#&#8203;50414](angular/angular#50414) [#&#8203;49930](angular/angular#49930)
-   **zone.js:** revert Mocha it.skip, describe.skip method patch ([#&#8203;49329](angular/angular#49329)) ([5a2b622](angular/angular@5a2b622))

##### Features

-   **zone.js:** jest 29 should ignore uncaught error console log ([#&#8203;49325](angular/angular#49325)) ([bc412fd](angular/angular@bc412fd)), closes [#&#8203;49110](angular/angular#49110)

##### Reverts

-   Revert "fix(zone.js): enable monkey patching of the `queueMicrotask()` API in node.js ([#&#8203;50467](angular/angular#50467))" ([#&#8203;50529](angular/angular#50529)) ([7567348](angular/angular@7567348)), closes [#&#8203;50467](angular/angular#50467) [#&#8203;50529](angular/angular#50529) [#&#8203;50529](angular/angular#50529)

</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.

👻 **Immortal**: This PR will be recreated if closed unmerged. Get [config help](https://github.com/renovatebot/renovate/discussions) if that's undesired.

---

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

---

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

Co-authored-by: cabr2-bot <cabr2.help@gmail.com>
Reviewed-on: https://codeberg.org/Calciumdibromid/CaBr2/pulls/1927
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 3, 2023
thomasturrell pushed a commit to thomasturrell/angular that referenced this pull request Aug 29, 2023
According to the HTML specification most attributes are defined as strings, however some can be interpreted as different types like booleans or numbers. [In the HTML standard](https://html.spec.whatwg.org/multipage/common-microsyntaxes.html#boolean-attributes), boolean attributes are considered `true` if they are present on a DOM node and `false` if they are omitted. Common examples of boolean attributes are `disabled` on interactive elements like `<button>` or `checked` on `<input type="checkbox">`. Another example of an attribute that is defined as a string, but interpreted as a different type is the `value` attribute of `<input type="number">` which logs a warning and ignores the value if it can't be parsed as a number.

Historically, authoring Angular inputs that match the native behavior in a type-safe way has been difficult for developers, because Angular interprets all static attributes as strings. While some recent TypeScript versions made this easier by allowing setters and getters to have different types, supporting this pattern still requires a lot of boilerplate and additional properties to be declared. For example, currently developers have to write something like this to have a `disabled` input that behaves like the native one:

```typescript
import {Directive, Input} from '@angular/core';

@directive({selector: 'mat-checkbox'})
export class MatCheckbox {
  @input()
  get disabled() {
    return this._disabled;
  }
  set disabled(value: any) {
    this._disabled = typeof value === 'boolean' ? value : (value != null && value !== 'false');
  }
  private _disabled = false;
}
```

This feature aims to address the issue by introducing a `transform` property on inputs. If an input has a `transform` function, any values set through the template will be passed through the function before being assigned to the directive instance. The example from above can be rewritten to the following:

```typescript
import {Directive, Input, booleanAttribute} from '@angular/core';

@directive({selector: 'mat-checkbox'})
export class MatCheckbox {
  @input({transform: booleanAttribute}) disabled: boolean = false;
}
```

These changes also add the `booleanAttribute` and `numberAttribute` utilities to `@angular/core` since they're common enough to be useful for most projects.

Fixes angular#8968.
Fixes angular#14761.

PR Close angular#50420
ChellappanRajan pushed a commit to ChellappanRajan/angular that referenced this pull request Jan 23, 2024
According to the HTML specification most attributes are defined as strings, however some can be interpreted as different types like booleans or numbers. [In the HTML standard](https://html.spec.whatwg.org/multipage/common-microsyntaxes.html#boolean-attributes), boolean attributes are considered `true` if they are present on a DOM node and `false` if they are omitted. Common examples of boolean attributes are `disabled` on interactive elements like `<button>` or `checked` on `<input type="checkbox">`. Another example of an attribute that is defined as a string, but interpreted as a different type is the `value` attribute of `<input type="number">` which logs a warning and ignores the value if it can't be parsed as a number.

Historically, authoring Angular inputs that match the native behavior in a type-safe way has been difficult for developers, because Angular interprets all static attributes as strings. While some recent TypeScript versions made this easier by allowing setters and getters to have different types, supporting this pattern still requires a lot of boilerplate and additional properties to be declared. For example, currently developers have to write something like this to have a `disabled` input that behaves like the native one:

```typescript
import {Directive, Input} from '@angular/core';

@directive({selector: 'mat-checkbox'})
export class MatCheckbox {
  @input()
  get disabled() {
    return this._disabled;
  }
  set disabled(value: any) {
    this._disabled = typeof value === 'boolean' ? value : (value != null && value !== 'false');
  }
  private _disabled = false;
}
```

This feature aims to address the issue by introducing a `transform` property on inputs. If an input has a `transform` function, any values set through the template will be passed through the function before being assigned to the directive instance. The example from above can be rewritten to the following:

```typescript
import {Directive, Input, booleanAttribute} from '@angular/core';

@directive({selector: 'mat-checkbox'})
export class MatCheckbox {
  @input({transform: booleanAttribute}) disabled: boolean = false;
}
```

These changes also add the `booleanAttribute` and `numberAttribute` utilities to `@angular/core` since they're common enough to be useful for most projects.

Fixes angular#8968.
Fixes angular#14761.

PR Close angular#50420
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: core Issues related to the framework runtime detected: feature PR contains a feature commit target: minor This PR is targeted for the next minor release
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Boolean properties as HTML binary attributes Proposal: add @BooleanInput and @NumberInput decorators
7 participants