#2 Signal inputs #53571

devversion · 2023-12-14T15:50:31Z

Please refer to #53521 for information on signal inputs. This is the second PR towards completing input signals.

See individual commits.

JoostK

Nice work! 👏

packages/core/src/authoring/input.ts

packages/core/src/authoring/input_signal.ts

packages/compiler/src/render3/view/util.ts

packages/core/src/render3/util/discovery_utils.ts

packages/core/test/authoring/type_tester.ts

packages/compiler-cli/test/ngtsc/authoring_spec.ts

...r_compliance/components_and_directives/lifecycle_hooks/lifecycle_hooks_lifecycle_comp_def.js

packages/compiler/src/render3/view/util.ts

packages/core/src/render3/definition.ts

packages/core/src/render3/instructions/shared.ts

...r_compliance/components_and_directives/lifecycle_hooks/lifecycle_hooks_lifecycle_comp_def.js

packages/core/src/authoring/input_signal_node.ts

packages/core/src/authoring/input_signal.ts

devversion · 2023-12-16T00:22:08Z

@alxhub @JoostK Pushed all addressed feedback as fixup commits (in between commits needed for order of changes). All individual commits auto-squashed should pass in order.

packages/core/src/authoring/input_signal.ts

packages/core/src/render3/component_ref.ts

packages/core/src/render3/util/discovery_utils.ts

packages/core/src/render3/instructions/shared.ts

packages/core/src/render3/instructions/write_to_directive_input.ts

packages/core/src/render3/component_ref.ts

packages/core/src/render3/instructions/write_to_directive_input.ts

packages/core/src/authoring/input_signal_node.ts

pkozlowski-opensource · 2023-12-18T13:11:04Z

packages/core/src/util/empty.ts

@@ -14,7 +14,7 @@ import {initNgDevMode} from './ng_dev_mode';
 * code.
 */

-export const EMPTY_OBJ: {} = {};
+export const EMPTY_OBJ: never = {} as never;


pkozlowski-opensource · 2023-12-18T13:11:43Z

packages/core/test/authoring/signal_input_signature_test.ts

+// import preserved to simplify `.d.ts` emit and simplify the `type_tester` logic.
+import {InputSignal} from '../../src/authoring/input_signal';
+
+export class InputSignatureTest {


Those tests are super-useful! 👍

pkozlowski-opensource · 2023-12-18T13:18:23Z

packages/core/src/render3/instructions/write_to_directive_input.ts

+import {applyValueToInputField} from '../apply_value_input_field';
+import {DirectiveDef, InputFlags} from '../interfaces/definition';
+
+export function writeToDirectiveInput<T>(


super-nit: this is not enforced / followed today but the initial idea was that the instructions folder would be just the entry point to the instructions called from the generated code with the actual logic living in the main renderer3 folder (in the, say inputs.ts).

Curious about your take on it.

That makes sense. Yeah, it's doesn't seem fully compliant right now either, but I agree. Will follow-up on that in separate PR.

pkozlowski-opensource

LGTM

Reviewed-for: public-api
Reviewed-for: size-tracking
Reviewed-for: fw-core

… compiler output (angular#53571) Instead of computing the bit input flags at compile-time and inling the final bit flag number, we will use the `InputFlags` enum directly. This is a little more code in the compiler side, but will allow us to have better debuggable development code, and also prevents problems where runtime flag bitmasks differ from the compiler flag bitmasks. This is in practice a noop for optimized applications as the enum values would be inlined anyway. This matches existing compiler emit for e.g. change detection strategy, or view encapsulation enums. PR Close angular#53571

…inputs (angular#53571) This commit adds additional type-check transform tests for signal inputs. These tests verify some of the problems with covariance, contravariance and bivariance that we were suspecting to be problematic if we would assign `InputSignal`'s directly to the type constructors. PR Close angular#53571

angular#53571) The `SignalNode` interface, describing the reactive node for a `Signal`, seemingly exposes the `SIGNAL` symbol as a class member. This is not true as the `SIGNAL` reactive node only exists on the getter function, as a way to retrieve the signal underlying reactive node. This commit fixes this, enabling improved type-safety later, in a follow-up commit where `SIGNAL_NODE` can now be typed to match the `SignalNode` interface (unlike now where it's typed as just `object`). PR Close angular#53571

… of signals (angular#53571) For the implementation of input signals, we want to extend the signal primitive. The basic methods exposed here are not suitable as we'd like to store additional metadata on the reactive node, and also have a custom getter (for required inputs and throwing). To enable this, one small piece was missing. This commit exposes it and also improves type safety, now that `SignalNode` is typed properly after the previous commit. PR Close angular#53571

…ar#53571) This commit introduces the runtime `InputSignal` implementation. Input initializers using `input` or `input.required` will result in an instance of `InputSignal` to be created. An input signal extends the signal primtive, with a couple of small differences: - it's a readonly signal. There is no public `set` or `update`. - equality is non-configurable. As per CD semantics, the value is guaranteed to be different when the `property` instruction attempts to update an input signal. - we support a `transform` function, that allows transforming input values. The transform is called whenever the input is set. An alternative could have been to follow computed-semantics and call the transform upon accessing, if dirty. In the future, we might change this to extend the computed reactive node, so that we can support computed inputs that do not rely on continious bound value assignments. See signal based components RFC. PR Close angular#53571

…ignals (angular#53571) At this point, we have the following pieces in place: * the input signature is implemented * the compiler properly parses and recognizes signal inputs * the compiler supports type-checking of signal inputs * input signal metadata is passed to partial output This commit adds a naive runtime solution to distinguishing between signal inputs and decorator inputs when the `property` instruction invokes. This is not ideal and non-performant as we introduce additional megamorphic reads for every property instruction invocation, or if we'd use `instanceof`, introducing a hard dependency on `InputSignal` and risking potentially slower detection. This code exists purely for testing, to enable playing with input signals in the playground. In a future commit, we will pass around the input signal metadata at runtime and can perform highly optimized checks to distinguish between signal or non-signal inputs- when assigning values. More information: https://docs.google.com/document/d/1FpnFruviKb6BFTQfMAP2AMEqEB0FI7z-3mT_qm7lzX8/edit#heading=h.oloxympe902x PR Close angular#53571

…en (angular#53571) Currently when a base class defines an input with a transform, derived classes re-defining the input via `@Input`, or `inputs: [<..>]`, end up inherting the transform due to a bug in the inherit definitions feature. This commit fixes this. We verified in the Google codebase that this is an unlikely occurrence and it's trivial to fix on user side by removing the re-declaration/override, or explictly adding the necessary transform. Conceptually, the behavior was quite inconsistent as everything else of inputs was overridden as expected. i.e. alias, required state etc. The exception were input transforms. This commit fixes this. PR Close angular#53571

…gular#53571) This commit introduces a new enum for capturing additional metadata about inputs. Called `InputFlags`. These will be built up at compile time and then propagated into the runtime logic, in a way that does not require additional lookup dictionaries data structures, or additional memory allocations for "common inputs" that do not have any flags. The flags will incorporate information on whether an input is signal based. This can then be used to avoid megamorphic accesses when such input is set- as we'd not need to check the input field value. This also avoids cases where an input signal may be used as initial value for an input (as we'd not incorrectly detect the input as a signal input then). The new metadata emit will be useful for incorporating additional metadata for inputs, such as whether they are required etc (although required inputs are a build-time only construct right now- but this is a good illustration of why input flags can be useful). An alternative could have been to have an additional boolean entry for signal inputs, but allocating a number with more flexible input flags seems more future proof and more reasonable andreadable. More information on the megamorphic access when updating an input signal https://docs.google.com/document/d/1FpnFruviKb6BFTQfMAP2AMEqEB0FI7z-3mT_qm7lzX8/edit. PR Close angular#53571

…ngular#53571) Consider a snippet like: ``` const x = directiveDef.inputs || EMPTY_OBJ ``` this currently results in `x` being inferred as just `{}`- ending up turning of potential future assignment checks. This surfaced in the `DirectiveDefinition` -> `DirectiveDef` conversion. Note: This has the effect that assigning `EMPTY_OBJ` to a field of anything would _always_ pass. It's questionable if this rather impacts type-safety in a more negative way. There seem to be trade-offs in both ways... Maybe worth considering just using `{}` directly as fallbacks in some places, and treating this as an unique symbol?! https://www.typescriptlang.org/play?#code/MYewdgzgLgBAHgLhmApgNxQJxgXhgbwF8YBDCZdLAbgCgbRJYAHJfGmDmAGxC6SkwBXFABoahAD6CwAExQAzAJaoZuZIK5dS5EmACeteuGgxBapjAkT4VIA PR Close angular#53571

Adds tests that allow us to ensure that the `input` API works as expected and that resulting return types match our expectations- without silently regressing in the future, or missing potential edge-cases. Testing signatures is hard because of covariance and contravariance, especially when it comes to the different semantics of `ReadT` and `WriteT` of input signals. We enable reliable testing by validating the `d.ts` of the "fake directive class". This ensures clear results, compared to relying on e.g. type assertions that might accidentally/silently pass due to covariance/contravariance or biavariance in the type system. PR Close angular#53571

…53571) This commit adds a final test for input signals, integrating all major parts: * type-checking * compiler detection * compiler emit * API signature tests PR Close angular#53571

…lar#53571) The linker compliance tests did not run for a while. There were a couple of new tests that were not passing as this wasn't flagged on CI. This commit fixes this. Fortunately there was no problematic code that did indicate issues with linking. In the follow-up commit, we fix the compliance test infrastructure to re-enable linker testing.. One clear issue is still that the defer blocks are not handled properly in linked output- hence making defer not actually "lazy" for compiled libraries. This needs to be handled separately by the framework team. PR Close angular#53571

The linker compliance tests were disabled with a Babel update and nobody realized for quite a while, via angular#49914. As we've came across this lost coverage, which also is quite impactful as all libraries depend on linked output- I've took initiative to debug the root cause as there was no follow-up. angular#51647. It turned out to be a highly complex issue that is non-trivial to fix, but at least we should try to resurrect the significant portion of test coverage by still running the linker tests- avoiding regressions, or unexpected issues (like with defer being developed). We can work on re-enabling and fixing source-maps separately. Tracked via angular#51647. PR Close angular#53571