Follow up changes for signal-based inputs #54053
Closed
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
angular-robot
bot
added
detected: feature
devversion
added
area: core
|
Deployed aio for 6134ba4 to: https://ng-dev-previews-fw--pr-angular-angular-54053-aio-wwwo0cyo.web.app Note: As new commits are pushed to this pull request, this link is updated after the preview is rebuilt. |
|
Deployed adev-preview for 6134ba4 to: https://ng-dev-previews-fw--pr-angular-angular-54053-adev-prev-1047nbmw.web.app Note: As new commits are pushed to this pull request, this link is updated after the preview is rebuilt. |
devversion
force-pushed
the
signal-inputs-followup
branch
from
89b2068
to
1819061
Compare
angular-robot
bot
added
detected: feature
devversion
added
the
action: review
devversion
changed the title
pkozlowski-opensource
approved these changes
Jan 26, 2024
|
LGTM but it looks like this PR will need a rebase after recent merges. |
This commit separates `InputSignal` for input signals with transforms. The reason being that most of the time, signal inputs are not using transforms and the generics are rather confusing. Especially for users with inferred types displayed in their IDEs, the input signal types are seemingly complex, even if no transform is used. For this reason, we are introducing a new type called `InputSignalWithTransform`. This type will be used for inputs with transforms, while non-transform inputs just use `InputSignal`. A notable fact is that `InputSignal` extends `InputSignalWithTransform`, with the "identity transform". i.e. there is no transform. This allows us to share the code for input signals. In practice, we don't expect users to pass around `InputSignal`'s anyway.
devversion
force-pushed
the
signal-inputs-followup
branch
from
1819061
to
c9c23c0
Compare
JoostK
approved these changes
Jan 26, 2024
packages/compiler-cli/src/ngtsc/typecheck/src/template_symbol_builder.ts
Outdated
Show resolved
Hide resolved
packages/compiler-cli/src/ngtsc/typecheck/src/template_symbol_builder.ts
Outdated
Show resolved
Hide resolved
This fixes the definitions for signal-based inputs in the language service and type checking symbol builder. Signal inputs emit a slightly different output. The output works well for comppletion and was designed to affect language service minimally. Turns out there is a small adjustment needed for the definition symbols.
This adds initial support for extracting and rendering call and construct signatures of classes, like within the new `InputFunction` for signal inputs. For now, signatures are a rare occasion and represented as class member entries. In the future we might consider exposing this via its own entry type, and field on the class/interface entry.
clang-format seems to have problems with the call signature for `input.required`. This commit works around the formatting issues that obfuscate the signature. Users will actually see similar output when they are looking for the `input` function definition of `@angular/core`.
This enables us to show overloads of methods in the API overview. This is useful for e.g. showing the various signatures of the signal input function, or for signal-based queris. There seems to be some issues with the length of the `InputFunction` overloads. There is some line wrapping that doesn't make it _super_ readable but this is an unrelated problem to this change, but rather a question of UI / API representation in the angular.io site.
I've been working on framework parts and compiler since pre-Ivy, and helped with Ivy, runtime and compiler. Adding myself as a reviewer to ease future work and to help with review load.
devversion
force-pushed
the
signal-inputs-followup
branch
from
c9c23c0
to
6134ba4
Compare
devversion
added
PullApprove: disable
and removed
action: review
devversion
added
action: merge
jessicajaniuk
pushed a commit
that referenced
this pull request
Jan 26, 2024
…uts (#54053) This fixes the definitions for signal-based inputs in the language service and type checking symbol builder. Signal inputs emit a slightly different output. The output works well for comppletion and was designed to affect language service minimally. Turns out there is a small adjustment needed for the definition symbols. PR Close #54053
jessicajaniuk
pushed a commit
that referenced
this pull request
Jan 26, 2024
…es (#54053) This adds initial support for extracting and rendering call and construct signatures of classes, like within the new `InputFunction` for signal inputs. For now, signatures are a rare occasion and represented as class member entries. In the future we might consider exposing this via its own entry type, and field on the class/interface entry. PR Close #54053
jessicajaniuk
pushed a commit
that referenced
this pull request
Jan 26, 2024
…d` (#54053) clang-format seems to have problems with the call signature for `input.required`. This commit works around the formatting issues that obfuscate the signature. Users will actually see similar output when they are looking for the `input` function definition of `@angular/core`. PR Close #54053
jessicajaniuk
pushed a commit
that referenced
this pull request
Jan 26, 2024
This enables us to show overloads of methods in the API overview. This is useful for e.g. showing the various signatures of the signal input function, or for signal-based queris. There seems to be some issues with the length of the `InputFunction` overloads. There is some line wrapping that doesn't make it _super_ readable but this is an unrelated problem to this change, but rather a question of UI / API representation in the angular.io site. PR Close #54053
|
This PR was merged into the repository by commit 23eeea5. |
jessicajaniuk
pushed a commit
that referenced
this pull request
Jan 26, 2024
I've been working on framework parts and compiler since pre-Ivy, and helped with Ivy, runtime and compiler. Adding myself as a reviewer to ease future work and to help with review load. PR Close #54053
nikvarma
pushed a commit
to nikvarma/angular
that referenced
this pull request
Jan 31, 2024
…angular#54053) This commit separates `InputSignal` for input signals with transforms. The reason being that most of the time, signal inputs are not using transforms and the generics are rather confusing. Especially for users with inferred types displayed in their IDEs, the input signal types are seemingly complex, even if no transform is used. For this reason, we are introducing a new type called `InputSignalWithTransform`. This type will be used for inputs with transforms, while non-transform inputs just use `InputSignal`. A notable fact is that `InputSignal` extends `InputSignalWithTransform`, with the "identity transform". i.e. there is no transform. This allows us to share the code for input signals. In practice, we don't expect users to pass around `InputSignal`'s anyway. PR Close angular#54053
nikvarma
pushed a commit
to nikvarma/angular
that referenced
this pull request
Jan 31, 2024
…uts (angular#54053) This fixes the definitions for signal-based inputs in the language service and type checking symbol builder. Signal inputs emit a slightly different output. The output works well for comppletion and was designed to affect language service minimally. Turns out there is a small adjustment needed for the definition symbols. PR Close angular#54053
nikvarma
pushed a commit
to nikvarma/angular
that referenced
this pull request
Jan 31, 2024
…es (angular#54053) This adds initial support for extracting and rendering call and construct signatures of classes, like within the new `InputFunction` for signal inputs. For now, signatures are a rare occasion and represented as class member entries. In the future we might consider exposing this via its own entry type, and field on the class/interface entry. PR Close angular#54053
nikvarma
pushed a commit
to nikvarma/angular
that referenced
this pull request
Jan 31, 2024
…d` (angular#54053) clang-format seems to have problems with the call signature for `input.required`. This commit works around the formatting issues that obfuscate the signature. Users will actually see similar output when they are looking for the `input` function definition of `@angular/core`. PR Close angular#54053
nikvarma
pushed a commit
to nikvarma/angular
that referenced
this pull request
Jan 31, 2024
…54053) This enables us to show overloads of methods in the API overview. This is useful for e.g. showing the various signatures of the signal input function, or for signal-based queris. There seems to be some issues with the length of the `InputFunction` overloads. There is some line wrapping that doesn't make it _super_ readable but this is an unrelated problem to this change, but rather a question of UI / API representation in the angular.io site. PR Close angular#54053
nikvarma
pushed a commit
to nikvarma/angular
that referenced
this pull request
Jan 31, 2024
I've been working on framework parts and compiler since pre-Ivy, and helped with Ivy, runtime and compiler. Adding myself as a reviewer to ease future work and to help with review load. PR Close angular#54053
Closed
|
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. |
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
See individual commits (language service, docs etc.)
Note that the API docs are still not ideal, but we are incrementally working to support them better. Alternatively we can consider extracting the required function into its own interface for API generation. Lets discuss in follow-ups