#1 Initial PR for signal inputs #53521

devversion · 2023-12-12T14:40:31Z

See individual commits.

Implements signal inputs for existing Zone based components.
This is a next step we are taking to bring signal inputs earlier to the Angular community.

The goal is to enable early access for the ecosystem to signal inputs, while we are continuing
development of full signal components as outlined in the RFC. This will allow the ecosystem
to start integrating signals more deeply, prepare for future migrations, and improves code quality
and DX for existing components (especially for OnPush).

Based on our work on full signal components, we've gathered more information and learned
new things. We've improved the API by introducing a way to intuitively declare required inputs,
as well as improved the API around initial values. We even support non-primitive initial values
as the first argument to the input function now.

@Directive({..})
export class MyDir {
  firstName = input<string>();            // string|undefined
  lastName = input.required<string>();    // string
  age = input(0);                         // number

Technical notes
Be aware that signal inputs in Zone components do not necessarily follow all the semantics
as expected in the RFC. Signal inputs in Zone components are not treated as "computeds".
This means, signal inputs are only updated when change detection runs. Not to be confused
with the semantics for full signal components in the RFC.

Components can have inputs defined with the @Input decorator and input() function for signal inputs. The aim is to ease
migration/adoption of signal inputs. Full signal components are not expected to support this. For example:

// Both of these are valid in the same component
age = input();
@Input() name = '';

As stated above, we have slightly changed the API signature for inputs based on our learnings
with the RFC-proposed API. We were facing a technical limitation that made it impractical to support
the API where the first argument of input could take either options or an initial value. This was mostly
related to complexity around static analysis and enabling future single file compilations. Other than that,
the changes also allowed us to unlock a few more things as mentioned above.

e-oz · 2023-12-12T15:06:07Z

Wow! Yeah! Thanks a lot!

A mix of @input inputs is possible. Full signal components are not expected to support this.

Could you please provide a more detailed explanation of this? Just a small micro-example (in pseudo-code), perhaps.

eneajaho · 2023-12-12T15:09:52Z

Wow! Yeah! Thanks a lot!

A mix of @input inputs is possible. Full signal components are not expected to support this.

Could you please provide a more detailed explanation of this? Just a small micro-example (in pseudo-code), perhaps.

We can use @input and input() both in the same component, while with signal component we won't be able to use both, but only signal inputs.

packages/core/src/authoring/input.ts

JoostK

This is going to take some time, went over just the first commit for now.

Comments starting with ⏩ are candidates for follow-up work and fine (recommended even) to keep out of this PR.

packages/compiler-cli/src/ngtsc/annotations/directive/src/input_function.ts

packages/compiler-cli/src/ngtsc/annotations/directive/src/shared.ts

packages/core/src/authoring/input.ts

packages/core/src/authoring/input_signal.ts

alxhub

Okay, review pass complete. This is awesome 👏 a few points:

I've added a couple of the followup tasks to the project board :)
Let's change all of the feat commits to refactor. Only commits which expose a user-facing feature should be tagged feat, as they get included in the changelog as such.
There are some performance considerations around the TCB generation. I think this design is workable, but we should discuss.

In particular, each time we change the import graph of the program, we invalidate a good part of the incremental build cache, and the next incremental build (which is often the build which actually ingests/checks the TCB) is expensive.

This has a few implications. For normal type check files, we shouldn't conditionally import helpers. We should always import them, even if they're not used, to ensure the shape of the import graph is stable regardless of which features are used in component at any point.

For inline TCBs, the problem is much more significant. Each Angular compilation is two incremental builds: one with the user's code in their component file, and a second which adds the inline TCB. On the next change the user makes, the third build will restore the code they wrote, and the fourth will add the inline TCB again. In that sense, component files with inline TCBs flip-flop in builds between the user's code and the inline TCB code.

If we're adding an import edge in the inline TCB that gets removed the next incremental step, then we change the import graph on each step - add, remove, add, remove, etc. This is expensive.

Fortunately, I think we're okay in this case. Because only files which contain components get inline TCBs, we're guaranteed that an edge already exists from such files to @angular/core. Therefore importing additional identifiers from @angular/core shouldn't modify the shape of the import graph. We should validate that this works in practice.

packages/core/src/authoring/input_signal.ts

packages/compiler/src/render3/partial/directive.ts

packages/compiler-cli/src/ngtsc/typecheck/test/diagnostics_spec.ts

packages/compiler-cli/src/ngtsc/annotations/directive/src/shared.ts

devversion · 2023-12-13T19:15:49Z

@alxhub I've looked into this. The program re-use issue is trivial to fix for type check block files- but as you said, for inline constructors, or inline type check blocks this is actually more complicated. The hypothesis of the structure being completely re-used as there is an existing edge to @angular/core unfortunately did not seem to be true. TypeScript looks at .imports of a SourceFile (which is basically a non-deduped list of import statements) and then compares these for program re-use (see http://shortn/_rfbXgkUKdg, and http://shortn/_cqYh2rtqQt).

One option could be potentially advancing the import manager to re-use existing imports. This is something we have in place for schematics, but it will involve some refactorings given that it would need to manipulate existing source file text, compared to just prepending.

…ction This commit introduces a function for declaring inputs in components. The function is called `input`. It comes in two flavors: - `input` for optional inputs with initial values - `input.required` for required inputs Inputs are declared as class members, like with `@Input`- except that the class field will no longer hold the input value directly. Angular takes control over the input field and exposes the input value as a signal. The runtime implementation will follow in future commits. This commit simply introduces: - initial compiler detection to recognize such inputs in classes - the initial signature of `input` and `input.required`. Note: the defer size test is flawed and there is no minification- hence this commit also needs to incorporate the new dependency graph changes.

This commit defines the initial metadata for inputs passed around in the compiler-cli. Inputs will now capture additional metadata on whether they are signal-based or not. This is stored on a per-input basis as a Zone component may contain both, signal inputs or `@Input` inputs. The metadata is later used for type-checking, for partial output generation, or full compilation output generation.

…cade When working on integrating a new metadata field for inputs, I realized there are quite a lot of duplications of interfaces. Turns out, the facade input map type can be replaced in favor of just `R3DirectiveInput`- even improving type safety-ness of e.g. the wrapped node expressions of transform functions.

…tial compilation output This commit captures the metadata on whether an input is signal based or not, in the `.d.ts` of directives and components. This exposes this information to consumers of the directives. This is needed because libraries may use signal inputs, and we need to know whether bound inputs to this library are signal-based or not- so that we can generate proper type-checking code (account for `InputSignal` or not). Additionally, this commit introduces a new structure for the partial compilation output of directive inputs. With the current emit, inputs are captured in a data structure that is equivalent to the internal data structure passed to `defineDirective` (the full compilation output). This worked fine as we only captured a few strings, but in ends up being a bad practice because partial compilation output should NOT capture internal data structures that might be specific to a certian Angular core version. Instead, we introduce a new "future proof" structure that: - can hold additional metadata in backwards-compatible ways, like `isSignal` or `isRequired`. - can be parsed trivially using the `AstHost` for the linker, instead of having to unwrap/parse an array structure. The new structure is only emitted when we discover that some inputs are signal based (or ultimately end up configuring input flags). This is done for backwards compatibility, so that libraries without signal inputs remain compatible with older linker versions. In the future, this might be the only emit. Compliance tests for this follow in future commits, when the linker portion is also in place. This commit specialices on the code generation. With the linker, and compliance test infrastructure fixed (that is broken right now), we can test the full integration.

This commit introduces the initial type-checking for signal inputs. To enable type-checking od signal inputs, there are a couple of tricks needed. It's not trivial as it would look like at first glance. Initial attempts could have been to generate additional statements in type-checking blocks for signal inputs to simply call a method like `InputSignal#applyNewValue`. This would seem natural, as it would match what will happen at runtime, but this would break the language-service auto completion in a highly subtle way. Consider the case where multiple directives match the same input. Consider the directives have some overlap in accepted input values, but they also have distinct diverging values, like: ```ts class DirA { value = input<'apple'|'shared'>(); } class DirB { value = input<'orange'|'shared'>(); } ``` In such cases, auto completion for the binding expression should suggest the following values: `apple`, `shared`, `orange` and `undefined`. The language service achieves this by getting completions in the type-check block where the user expression would live. This BREAKS if we'd have multiple places where the expression from the user is used. Two different places, or more, surface additional problems with diagnostic collection. Previously diagnostics would surface the union type of allowed values, but with multiple places, we'd have to work with potentially 1+ diagnostics. This is non-ideal. Another important consideration is test coverage. It might sound problematic to consider the existing test infrastructure as relevant, but in practice, we have thousands of diagnostic type check block tests that would greatly benefit if the general emit structure would still match conceptually. This is another bonus argument on why changing the way inputs are applied is probably an option we should consider as a last resort. Ultimately, there is a good solution where we unwrap directive signal inputs, based on metadata, and access a brand type field on the `InputSignal`. This ensures auto-completion continues to work as is, and also the structure of type check blocks doesn't change conceptually. In future commits we also need to handle type-inference for generic signal inputs. Note: Another alternative considered, in terms of using metadata or not. We could have type helpers to unwrap signal inputs using type helpers like: `T extends InputSignal<any, WriteT> ? WriteT : T`. This would allow us to drop the input signal metadata dependency, but in reality, this has a few issues: - users might have `@Input`'s passing around `InputSignal`'s. This is unlikely, but shows that the solution would not be fully correct. - we need the metadata regardless, as we plan on accessing it at runtime as well, to distinguish between signal inputs and normal inputs when applying new values. This was not clear when this option was considered initially.

…ransforms (angular#53521) Signal inputs do not need coercion members for their transforms. That is because the `InputSignal` type- which is accessible in the class member- already holds the type of potential "write values". This eliminates the need for coercion members which were simply used to somehow capture this write type (especially when libraries are consumed and only `.d.ts` is available). We can simplify this, and also significantlky loosen restrictions of transform functions- given that we can fully rely on TypeScript for inferring the type. There is no requirement in being able to "transplant" the type into different places- hence also allowing supporting transform functions with generics, or overloads. In a follow-up commit, once more parts are place, there will be some compliance tests to ensure these new "loosend restrictions". PR Close angular#53521

…or code (angular#53521) For signal inputs we are looking at generating additional code inside type constructors. This code is planned to reference an external type from `@angular/core` to unwrap `InputSignal`'s class fields. The existing `Environment` class contains helpers for emitting such references / and translating them from the output AST. We extract this logic into a superclass for only emitting references. A similar type already existed to avoid circular dependencies- but now we have actual use-cases to populate this as a base class. This allows us to create more-suitable minimal emit environments when we e.g. generate type constructors inline- which are not part of any type check block. The existing `Environment` class is scoped to type check blocks and therefore was not suitable. PR Close angular#53521

…ngular#53521) This commit adds the last remaining piece for signal input type-checking. Bound values to signal inputs are already checked properly at this point, but inference of generic directive/component types through their inputs is not implemented. This commit fixes this. To achieve this, there are a couple of potential solutions. The generics of a directive are inferred based on input value expressions using a so-called type constructor. The constructor looks something like this: ``` const _ctor = <T>(v: Pick<Dir<T>, 'input1', 'input2'>) => Dir<T>; _ctor({input1: expr1, input2: expr2}); ``` This works very well for non-signal inputs where the class member is directly holding the input values. For signal inputs, this does NOT work because the class member will actually hold the `InputSignal` instance. There are a couple of solutions to this: 1. Calling `_ctor` with an `InputSignal<typeof value>` 2. Converting the `_ctor` input signal fields to their write types (unwrapping the input signals). We've decided to go with the second option as TypeScript is very sensitive with assignments and its checks. i.e. co-variance, contravariance or bivariance. Semantically it makes more sense to unwrap the input signal "write type" directly and "assign to it". This is safer and conceptually also easier to follow. A type constructor continues to only receive the "expresison values". This simplifies code as well. It's worth noting that the unwrapping as per option 2 also comes at a cost. We need to be able to generate imports in type constructors. This was not possible until the previous commit because inline type constructors did not have an associated type-check block `Environment` and we were missing access to expression translation and correct import generation. Overall, solution 2 is now implemented as works as expected. This commit adds additional unit tests to ensure this. PR Close angular#53521

…ular#53521) This will allow us to write unit tests for the new input function, allowing us eventually to test type checking, emitted code etc. PR Close angular#53521

…/ signal inputs (angular#53521) Whenever a signal input is captured in a type check block, we will insert an import. This will change the import graph so that the full TypeScript program cannot be structurally re-used. We can fix this trivially by ensuring the import graph remains stable, by always generating an import to e.g. `@angular/core`. This fixes the issue nicely for type-check block files. A test verifies this. For inline code, such as TCB inline or the type constructors inline, this fix is not applicable because we would change user-input source files, adding new edges that would not exist for subsequent builds- causing the program to be not re-used completely. One idea was to rely on the existing edge that can be assumed to exist for directive code files. This is true technically, but in practice TS does not deduplicate imports- so our new namespace import when referencing our symbols will invalidate the re-use. We will address this in a follow-up. There are a couple of options, such as working with the TS team, updating the existing edge, or inlining our helpers as well. PR Close angular#53521

…irectives (angular#53521) Whenever an input of a directive changes, the semantic symbol should reflect this change for the type check API. This is important because signal inputs require special output in the type checking blocks- hence we need to ensure that such type checking blocks are re-generated properly. Test verify that incremental type-checking builds work as expected now. PR Close angular#53521

…ction (angular#53521) This commit introduces a function for declaring inputs in components. The function is called `input`. It comes in two flavors: - `input` for optional inputs with initial values - `input.required` for required inputs Inputs are declared as class members, like with `@Input`- except that the class field will no longer hold the input value directly. Angular takes control over the input field and exposes the input value as a signal. The runtime implementation will follow in future commits. This commit simply introduces: - initial compiler detection to recognize such inputs in classes - the initial signature of `input` and `input.required`. Note: the defer size test is flawed and there is no minification- hence this commit also needs to incorporate the new dependency graph changes. PR Close angular#53521

…ata (angular#53521) This commit defines the initial metadata for inputs passed around in the compiler-cli. Inputs will now capture additional metadata on whether they are signal-based or not. This is stored on a per-input basis as a Zone component may contain both, signal inputs or `@Input` inputs. The metadata is later used for type-checking, for partial output generation, or full compilation output generation. PR Close angular#53521

…cade (angular#53521) When working on integrating a new metadata field for inputs, I realized there are quite a lot of duplications of interfaces. Turns out, the facade input map type can be replaced in favor of just `R3DirectiveInput`- even improving type safety-ness of e.g. the wrapped node expressions of transform functions. PR Close angular#53521

…tial compilation output (angular#53521) This commit captures the metadata on whether an input is signal based or not, in the `.d.ts` of directives and components. This exposes this information to consumers of the directives. This is needed because libraries may use signal inputs, and we need to know whether bound inputs to this library are signal-based or not- so that we can generate proper type-checking code (account for `InputSignal` or not). Additionally, this commit introduces a new structure for the partial compilation output of directive inputs. With the current emit, inputs are captured in a data structure that is equivalent to the internal data structure passed to `defineDirective` (the full compilation output). This worked fine as we only captured a few strings, but in ends up being a bad practice because partial compilation output should NOT capture internal data structures that might be specific to a certian Angular core version. Instead, we introduce a new "future proof" structure that: - can hold additional metadata in backwards-compatible ways, like `isSignal` or `isRequired`. - can be parsed trivially using the `AstHost` for the linker, instead of having to unwrap/parse an array structure. The new structure is only emitted when we discover that some inputs are signal based (or ultimately end up configuring input flags). This is done for backwards compatibility, so that libraries without signal inputs remain compatible with older linker versions. In the future, this might be the only emit. Compliance tests for this follow in future commits, when the linker portion is also in place. This commit specialices on the code generation. With the linker, and compliance test infrastructure fixed (that is broken right now), we can test the full integration. PR Close angular#53521

…lar#53521) This commit introduces the initial type-checking for signal inputs. To enable type-checking od signal inputs, there are a couple of tricks needed. It's not trivial as it would look like at first glance. Initial attempts could have been to generate additional statements in type-checking blocks for signal inputs to simply call a method like `InputSignal#applyNewValue`. This would seem natural, as it would match what will happen at runtime, but this would break the language-service auto completion in a highly subtle way. Consider the case where multiple directives match the same input. Consider the directives have some overlap in accepted input values, but they also have distinct diverging values, like: ```ts class DirA { value = input<'apple'|'shared'>(); } class DirB { value = input<'orange'|'shared'>(); } ``` In such cases, auto completion for the binding expression should suggest the following values: `apple`, `shared`, `orange` and `undefined`. The language service achieves this by getting completions in the type-check block where the user expression would live. This BREAKS if we'd have multiple places where the expression from the user is used. Two different places, or more, surface additional problems with diagnostic collection. Previously diagnostics would surface the union type of allowed values, but with multiple places, we'd have to work with potentially 1+ diagnostics. This is non-ideal. Another important consideration is test coverage. It might sound problematic to consider the existing test infrastructure as relevant, but in practice, we have thousands of diagnostic type check block tests that would greatly benefit if the general emit structure would still match conceptually. This is another bonus argument on why changing the way inputs are applied is probably an option we should consider as a last resort. Ultimately, there is a good solution where we unwrap directive signal inputs, based on metadata, and access a brand type field on the `InputSignal`. This ensures auto-completion continues to work as is, and also the structure of type check blocks doesn't change conceptually. In future commits we also need to handle type-inference for generic signal inputs. Note: Another alternative considered, in terms of using metadata or not. We could have type helpers to unwrap signal inputs using type helpers like: `T extends InputSignal<any, WriteT> ? WriteT : T`. This would allow us to drop the input signal metadata dependency, but in reality, this has a few issues: - users might have `@Input`'s passing around `InputSignal`'s. This is unlikely, but shows that the solution would not be fully correct. - we need the metadata regardless, as we plan on accessing it at runtime as well, to distinguish between signal inputs and normal inputs when applying new values. This was not clear when this option was considered initially. PR Close angular#53521

…frastructure (angular#53521) This commit ensures that the type-check diagnostic testing infrastructure is prepared to validate signal inputs. i.e. providing the necessary "mocks" in the fake "d.ts" of `@angular/core`. The commit then sets up a Golang-style table driven testing environment that allows us to validate/verify signal input type-checking in a readable way. With this infrastructure set up, this commit defines an initial set of unit tests for type checking of input signals. PR Close angular#53521