#2 Signal inputs #53571

…inputs 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.

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`).

… of signals 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.

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.

Follow-up TODO for runtime error. Commit ready for 3rd PR

Simplify required signal node creation as per feedback

Transforms are run manually, plus ensure monomorphic access.

…ignals 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

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.

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.

Address review feedback e98870c6a18f34cbfd00702411e7f47d625ad8e6

Introduce new `HasTransform` flag that will be used Fix transforms for input signals with ngOnChanges. The value needs to be transformed before

Reflect the new input flags for inputs having transforms

Account for rebase and new local compilation tests

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

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.

This commit adds a final test for input signals, integrating all major parts: * type-checking * compiler detection * compiler emit * API signature tests

Fixes a typo in a unit test, reported in PR review.

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.

Disable new defer complaince tests to run with the linker. We disabled most of these already, but new ones were introduced 3 days ago. This commit disables those as well because defer is not yet handled properly with partial compilation output (This is a known & planned task). The tests started failing because in this PR we fixed that the linker tests were not running for quite a while..

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.

… compiler output 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.

…or full compiler output Update after new HasTransform flag

…or full compiler output Account for rebase and new local compilation tests

Simplify if check

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

#2 Signal inputs #53571

#2 Signal inputs #53571

Commits on Jan 3, 2024