Initial implementation of output()#54217
Conversation
As we are introducing the new `output()` function as an inituive alternative to `@Output()` that matches with signal-based inputs, this commit prepares the compiler to detect such initializer-based outputs.
This commit introduces the `output()` function and corresponding runtime code. In practice, `output()` will defer to `EventEmitter` as outlined in the RFC, but we are considering limiting the type to a minimal version that is not coupled with RxJS, less complex, and also has better type safety around emitting of values. E.g. currently `EventEmitter.emit` can always be called without any value, even though the output may be typed to always pass around values of type `T`. This could cause subtle and confusing bugs.
Adds compliance output tests for `output()` to verify that we are emitting proper full compilation output, as well as proper partial compilation output that can be linked to match the full output.
Adds an ngtsc diagnostic and compilation output test for `output()`. The test will verify certain recognition restrictions and ensures that diagnostics are raised, in addition to proper full compilation output being generated (aside from the compliance tests verifying output more closely).
Generalizes the type check table scenario testing infrastructure so that it can also be used for testing outputs in a table-scheme without a lot of TS code repetition.
Adds type check diagnostic tests for the `output()` API. This is necessary because we are maintaining a separate emitter for `output()` as with the current design (still discussed - but this is a starting foundation). Note: `OutputEmitter` currently does not publicly expose `.subscribe`, while the testing infrastructure exposes it for now. That is because we are still discussing this before making changes in the TCB to account for the case where `.subscribe` might be `@internal` ultimately.
…itializer API Similar to `input()`, initializer-based `output()`'s need to be transformed in JIT to be annotated with an `@Output()` decorator. This is necessary so that Angular can statically collect all defined outputs without instantiating the class (which would not be possible upon directive definition computation). This commit introduces a transform next to the input transform that automatically runs with the Angular CLI and `ng test`.
Adds an extra unit test for the language-service to verify that type definitions for signal-based inputs can be resolved.
Adds unit tests to verify that the language service supports the `output()` function with completions, and definition jumping.
The `listener` instruction currently always assumes RxJS subscribables, and verifies this via `isSubscribable`. The type narrowing is not ignored and the type remains `any` given the `ngDevMode` check. This commit improves type safety, and actually switches to a dedicated interface for "output subscribable" values. This is needed because `Subscribable` from `RxJS` is typed to expect an observer in object literal form- which is not correct and doesn't apply to `EventEmitter` and matches the form of `.subscribe` we are using in the `listener` instruction.
Adds `output()` to the signal inputs integration test to verify full integration with the Angular CLI. In the future we may consider renaming the test to something less specific to signal inputs, but for now this serves the purpose and `output()` is a closely related part to signal-based inputs. it doesn't warrant creating another new integration test, as those are quite expensive to maintain and run.
0d6b2e3 to
f574ca1
Compare
|
#my2cents about dynamic component: |
|
I think I'd prefer This would help to tie it to the state management parts, where a signal is computed within it and is provided to a component. This would be similar to how an Observable is created today and then annotated with If we stick with only |
|
A signal would always require a value. I.e. we would need to have an initial value for outputs. There might be ways, but this would be tricky. I was also considering signals/computed and effects for programmatic "subscriptions", but at this point- I don't know if those are suitable. These things are still subject to change. As noted, the PR is just laying the initial detection w/ some basic code for now, re-using |
|
Question here, why is |
|
@j-perezr Thanks for the feedback. This is just an initial boilerplate of the |
|
Note: No need for Alex' review as we just added unit tests for language-service + an integration test (both groups pending) |
This commit introduces the `output()` function and corresponding runtime code. In practice, `output()` will defer to `EventEmitter` as outlined in the RFC, but we are considering limiting the type to a minimal version that is not coupled with RxJS, less complex, and also has better type safety around emitting of values. E.g. currently `EventEmitter.emit` can always be called without any value, even though the output may be typed to always pass around values of type `T`. This could cause subtle and confusing bugs. PR Close #54217
|
This PR was merged into the repository by commit 6cb47a7. |
Adds an ngtsc diagnostic and compilation output test for `output()`. The test will verify certain recognition restrictions and ensures that diagnostics are raised, in addition to proper full compilation output being generated (aside from the compliance tests verifying output more closely). PR Close #54217
Generalizes the type check table scenario testing infrastructure so that it can also be used for testing outputs in a table-scheme without a lot of TS code repetition. PR Close #54217
Adds type check diagnostic tests for the `output()` API. This is necessary because we are maintaining a separate emitter for `output()` as with the current design (still discussed - but this is a starting foundation). Note: `OutputEmitter` currently does not publicly expose `.subscribe`, while the testing infrastructure exposes it for now. That is because we are still discussing this before making changes in the TCB to account for the case where `.subscribe` might be `@internal` ultimately. PR Close #54217
…itializer API (#54217) Similar to `input()`, initializer-based `output()`'s need to be transformed in JIT to be annotated with an `@Output()` decorator. This is necessary so that Angular can statically collect all defined outputs without instantiating the class (which would not be possible upon directive definition computation). This commit introduces a transform next to the input transform that automatically runs with the Angular CLI and `ng test`. PR Close #54217
The `listener` instruction currently always assumes RxJS subscribables, and verifies this via `isSubscribable`. The type narrowing is not ignored and the type remains `any` given the `ngDevMode` check. This commit improves type safety, and actually switches to a dedicated interface for "output subscribable" values. This is needed because `Subscribable` from `RxJS` is typed to expect an observer in object literal form- which is not correct and doesn't apply to `EventEmitter` and matches the form of `.subscribe` we are using in the `listener` instruction. PR Close #54217
Adds `output()` to the signal inputs integration test to verify full integration with the Angular CLI. In the future we may consider renaming the test to something less specific to signal inputs, but for now this serves the purpose and `output()` is a closely related part to signal-based inputs. it doesn't warrant creating another new integration test, as those are quite expensive to maintain and run. PR Close #54217
I know this is close now, but just to post an idea , maybe having an extra method like computedOutput similar to computed But returns like a ready only output emitter without an emit |
|
This issue has been automatically locked due to inactivity. Read more about our automatic conversation locking policy. This action has been performed automatically by a bot. |
This PR introduces the
output()function. The function can be used to declare directive/componentoutputs without the use of decorators. This allows for a more consistent and intuitive experience with
signal inputs, queries or upcoming model.
E.g.
Note: This PR is the initial implementation, laying the groundwork for output(). Points discussed below are not decided at all, and just additional considerations at this point
Current idea of design will return an
OutputEmitterinstead of anEventEmitterwith the goals of:EventEmitterimplementing aSubject..emitsignature.EventEmitter#emitcurrently takesT, but also can be called without a value. This is confusing and might cause subtle bugs (see for example: https://stackblitz.com/edit/stackblitz-starters-bvs1rn?file=src%2Fmain.ts)An additional consideration is to not expose
.subscribepublicly. Conceptually aligning with signal inputs,where you can only write to outputs, while subscription is only possible for consumers via the template. A difficult question will be dynamically created components as those currently have no way to subscribe, unlike with signal inputs where users could reach for
ComponentRef#setInput. This is something to explore.Note: We are still discussing design more closely and incorporating feedback from the RFC. This PR introduces the initial logic that we can re-use for changes we are making as part finalizing/discussing design.