Skip to content

#3 Signal inputs#53808

Closed
devversion wants to merge 23 commits intoangular:mainfrom
devversion:signal-pr3
Closed

#3 Signal inputs#53808
devversion wants to merge 23 commits intoangular:mainfrom
devversion:signal-pr3

Conversation

@devversion
Copy link
Member

See individual commits and #53521 for more context.

@angular-robot angular-robot bot added the area: build & ci Related the build and CI infrastructure of the project label Jan 5, 2024
@ngbot ngbot bot added this to the Backlog milestone Jan 5, 2024
@devversion devversion force-pushed the signal-pr3 branch 2 times, most recently from e088b19 to 6f74029 Compare January 5, 2024 14:00
@devversion devversion added action: review The PR is still awaiting reviews from at least one requested reviewer target: minor This PR is targeted for the next minor release cross-cutting: signals labels Jan 5, 2024
@devversion devversion marked this pull request as ready for review January 5, 2024 15:33
@devversion devversion removed the request for review from twerske January 5, 2024 16:18
@devversion devversion force-pushed the signal-pr3 branch 3 times, most recently from 517f82e to 627cae4 Compare January 8, 2024 20:56
Copy link
Member

@JoostK JoostK left a comment

Choose a reason for hiding this comment

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

Left mostly nits

Copy link
Member

@pkozlowski-opensource pkozlowski-opensource left a comment

Choose a reason for hiding this comment

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

LGTM! I went through the runtime / core parts and only skimmed through the compiler / integration tests bits.

@devversion devversion requested a review from JoostK January 9, 2024 15:29
@devversion devversion force-pushed the signal-pr3 branch 3 times, most recently from 1420420 to ac2707a Compare January 9, 2024 20:14
devversion added a commit that referenced this pull request Jan 10, 2024
This commit introduces a new integration test to ensure signal inputs
work as expected for end uses in Angular CLI applications.

PR Close #53808
devversion added a commit that referenced this pull request Jan 10, 2024
…ts (#53808)

Whenever a required input is accessed too early in a
directive/component, the signal input will throw an error.

This is necessary so that we can support required inputs
with intuitive typings that do not include `undefined` for
the short period of time where Angular is creating the component and
then assigning inputs later (Angular currently has no way of setting
inputs as part of the class creation when `new Dir()` happens)

PR Close #53808
devversion added a commit that referenced this pull request Jan 10, 2024
This commit adds some runtime unit tests to ensure that input
signal is behaving properly at runtime.

PR Close #53808
devversion added a commit that referenced this pull request Jan 10, 2024
This commit creates a small http server Angular application playground
for playing with signal inputs. This is useful for development and also
validates some of the common input patterns.

PR Close #53808
devversion added a commit that referenced this pull request Jan 10, 2024
… constructors (#53808)

We recently landed a commit to introduce support for generic type
checking of signal inputs. For that we had to implement logic that
will generate imports for inline type constructors. This required
changes to the context logic and `TypeCtorOp` file-level op.

This commit ensures that everything is working as expected, specifically
in cases where an inline type ctor is generated and imports would be
needed to unwrap the class members for `InputSignal`.

PR Close #53808
devversion added a commit that referenced this pull request Jan 10, 2024
…members (#53808)

Moves the signal input class member extraction logic into the dedicated
input function file. This makes the code for signal inputs more
self-contained.

This commit then re-exposes the function as part of `ngtsc/annotations`
so that it can be used later for a transform that will take the signal
input metadata and translate it into a `@Input` decorator. This allows
us to remove code duplication and guarantees consistency/correctness

PR Close #53808
devversion added a commit that referenced this pull request Jan 10, 2024
…#53808)

This commit does two things:

- exposes `addImports` so that it can be used by transforms that we are
  adding to the compiler. e.g. the signal input to `@Input` transform.

- `updates `addImports` to support/use the transform context factory.
  This will allow us to write proper transforms using `addImports`. Also
  leverages this in the Ivy JS/DTS transforms.

PR Close #53808
devversion added a commit that referenced this pull request Jan 10, 2024
…53808)

This commit adds a transform for supporting input signals in JIT
environments. The transform will be wired up for Angular CLI
applications automatically. An integration test verifies that this fixes
unit testing with signal inputs.

The transform basically will take the signal input metadata and
transform it into `@Input` decorators that can provide static
information to the Angular JIT runtime when the directive/component
definition is compiled.

PR Close #53808
devversion added a commit that referenced this pull request Jan 10, 2024
…53808)

Follow-up for the inital signal inputs PR, fixing a typo that was made.

PR Close #53808
devversion added a commit that referenced this pull request Jan 10, 2024
…53808)

Given that the TCB output changes with signal inputs, and one of our
important considerations was auto-completion, we need some unit tests
that verify and guarantee proper completion with signal inputs being
bound in templates. This commit adds these.

PR Close #53808
devversion added a commit that referenced this pull request Jan 10, 2024
…ed properly (#53808)

This commit addds two diagnostics for two scenarios where signal inputs
are declared incorrectly:

- a signal input is also annotated with `@Input` in the TypeScript
  sources.
- a signal input is also declared in the `inputs` option of
  `@Directive/`@Component`.

PR Close #53808
devversion added a commit that referenced this pull request Jan 10, 2024
…members (#53808)

Currently when someone declares a signal or non-signal input on a static
class member, the compiler will not yield any diagnostic. We can detect
these mistakes and report a diagnostic to help our users.

PR Close #53808
devversion added a commit that referenced this pull request Jan 10, 2024
… recognition (#53808)

Improves the recognition of the `input`/`input.required` functions to
not depend on external module resolution. This is useful for local
compilation and for transforms operating on a single file/ isolated
module.

PR Close #53808
devversion added a commit that referenced this pull request Jan 10, 2024
…AOT (#53808)

Adds infrastructure to run signal input tests with JIT (using the
transform) and AOT. Acceptance tests for signal inputs will run with
both variants. In the future we can consider expanding this
infrastructure for all of our acceptance tests, but that's a different
story.

PR Close #53808
devversion added a commit that referenced this pull request Jan 10, 2024
Adds AOT and JIT unit tests for signal inputs that verify integration of
signal inputs for our users.

PR Close #53808
ChellappanRajan pushed a commit to ChellappanRajan/angular that referenced this pull request Jan 23, 2024
)

The linker AST is abstracted to be agnostic of the underlying
implementation AST. i.e. TS AST or Babel AST.

This abstraction also intends to provide some type-safety-ness to
parsing of various partial declarations. This commit improves type
safety further by fixing that `AstValue'`s were not checked for
assignability of `T`- potentially hiding issues/unaccounted values.

Additionally, we fix that `getObject()` does not properly narrow
union types to actual object literals. This happend because e.g. arrays
are of type `object`. We can improve type safety here. Using `Record`
did not help as an array would still assign to that.

PR Close angular#53808
ChellappanRajan pushed a commit to ChellappanRajan/angular that referenced this pull request Jan 23, 2024
In some cases, the input files for a partial output generation
compliance tests may be invalid and lead to compilation errors.

The golden partial would be silently generated with the remaining
test cases. Instead of hiding errors, we will now print these and
cause the script to fail properly.

Note that the error logging is pretty minimalistic, but it's sufficient.

PR Close angular#53808
ChellappanRajan pushed a commit to ChellappanRajan/angular that referenced this pull request Jan 23, 2024
…tor (angular#53808)

We are adding internal support for declaring signal inputs via the
`@Input` decorator. This is needed for JIT unit testing, or JIT
applications.

In JIT, Angular is not able to recognize signal inputs due to the
lack of static reflection metadata. Decorators attach their information
on the class- without it needing to be instantiated. This allows Angular
to know inputs when preparing/generating the directive definition. With
signal inputs this is not possible- so we need a way to tell Angular
about inputs for JIT applications. We've decided that this is not
something users should have to deal with, so a transform will be added
in a follow-up that will automatically derive/and add the decorators
for signal inputs when requested in JIT environments.

PR Close angular#53808
ChellappanRajan pushed a commit to ChellappanRajan/angular that referenced this pull request Jan 23, 2024
…ngular#53808)

This commit changes the `HasTransform` flag to be only concerned with
decorator inputs. This allows us to automatically detect signal input
transforms without reliance on the flag, resulting in less complexity in
the compiler (as outlined in the design doc) and various  other places,
while it also allows us to simplify JIT support for signal inputs
because there would be no need to capture the "hasTransform" state in
the decorator so that JIT can generate the according input flags.

`isSignal` will still persist as an input flag to allow for monomorphic
and highly efficient distinguishing at runtime, whether an input is
signal based or not. JIT transform will also need to propagate this
information to the runtime somehow.

PR Close angular#53808
ChellappanRajan pushed a commit to ChellappanRajan/angular that referenced this pull request Jan 23, 2024
…3808)

As part of testing we did accidentally use `bitwiseAnd` for the input
flags, given we started without an extra flag for `HasTransform`.

This commit teaches the compiler to support emitting bitwise OR
and uses it when combining input flags, fully re-enabling transforms
for signal components after the new flag mechanism was introduced in
previous commits.

PR Close angular#53808
ChellappanRajan pushed a commit to ChellappanRajan/angular that referenced this pull request Jan 23, 2024
…r signal inputs (angular#53808)

As we introduced the new partial output for signal inputs, we also need
to update the linker code to be able to parse this. This commit adds
this functionality.

In the follow-up commit, compliance tests for linking, partial output,
and full compilations are added.

PR Close angular#53808
ChellappanRajan pushed a commit to ChellappanRajan/angular that referenced this pull request Jan 23, 2024
…nd full compilations (angular#53808)

Adds signal input compliance tests, ensuring linking works as expected,
partial output is generated properly, types are inferred properly, and
that the full, or linked output matches our expected runtime structure.

PR Close angular#53808
ChellappanRajan pushed a commit to ChellappanRajan/angular that referenced this pull request Jan 23, 2024
This commit introduces a new integration test to ensure signal inputs
work as expected for end uses in Angular CLI applications.

PR Close angular#53808
ChellappanRajan pushed a commit to ChellappanRajan/angular that referenced this pull request Jan 23, 2024
…ts (angular#53808)

Whenever a required input is accessed too early in a
directive/component, the signal input will throw an error.

This is necessary so that we can support required inputs
with intuitive typings that do not include `undefined` for
the short period of time where Angular is creating the component and
then assigning inputs later (Angular currently has no way of setting
inputs as part of the class creation when `new Dir()` happens)

PR Close angular#53808
ChellappanRajan pushed a commit to ChellappanRajan/angular that referenced this pull request Jan 23, 2024
This commit adds some runtime unit tests to ensure that input
signal is behaving properly at runtime.

PR Close angular#53808
ChellappanRajan pushed a commit to ChellappanRajan/angular that referenced this pull request Jan 23, 2024
This commit creates a small http server Angular application playground
for playing with signal inputs. This is useful for development and also
validates some of the common input patterns.

PR Close angular#53808
ChellappanRajan pushed a commit to ChellappanRajan/angular that referenced this pull request Jan 23, 2024
… constructors (angular#53808)

We recently landed a commit to introduce support for generic type
checking of signal inputs. For that we had to implement logic that
will generate imports for inline type constructors. This required
changes to the context logic and `TypeCtorOp` file-level op.

This commit ensures that everything is working as expected, specifically
in cases where an inline type ctor is generated and imports would be
needed to unwrap the class members for `InputSignal`.

PR Close angular#53808
ChellappanRajan pushed a commit to ChellappanRajan/angular that referenced this pull request Jan 23, 2024
…members (angular#53808)

Moves the signal input class member extraction logic into the dedicated
input function file. This makes the code for signal inputs more
self-contained.

This commit then re-exposes the function as part of `ngtsc/annotations`
so that it can be used later for a transform that will take the signal
input metadata and translate it into a `@Input` decorator. This allows
us to remove code duplication and guarantees consistency/correctness

PR Close angular#53808
ChellappanRajan pushed a commit to ChellappanRajan/angular that referenced this pull request Jan 23, 2024
…angular#53808)

This commit does two things:

- exposes `addImports` so that it can be used by transforms that we are
  adding to the compiler. e.g. the signal input to `@Input` transform.

- `updates `addImports` to support/use the transform context factory.
  This will allow us to write proper transforms using `addImports`. Also
  leverages this in the Ivy JS/DTS transforms.

PR Close angular#53808
@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.

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: build & ci Related the build and CI infrastructure of the project cross-cutting: signals merge: caretaker note Alert the caretaker performing the merge to check the PR for an out of normal action needed or note PullApprove: disable target: minor This PR is targeted for the next minor release

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants