Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Patch port of #54053 #54233

Closed
wants to merge 6 commits into from
Closed

Conversation

devversion
Copy link
Member

@devversion devversion commented Feb 3, 2024

Patch port of #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.
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.
@angular-robot angular-robot bot added detected: feature PR contains a feature commit area: build & ci Related the build and CI infrastructure of the project labels Feb 3, 2024
@ngbot ngbot bot added this to the Backlog milestone Feb 3, 2024
@devversion devversion added action: review The PR is still awaiting reviews from at least one requested reviewer PullApprove: disable target: patch This PR is targeted for the next patch release labels Feb 3, 2024
@devversion devversion marked this pull request as ready for review February 3, 2024 17:11
@JeanMeche
Copy link
Member

@devversion Can you double-check the issue mentioned ?

@devversion devversion added action: merge The PR is ready for merge by the caretaker and removed action: review The PR is still awaiting reviews from at least one requested reviewer labels Feb 4, 2024
jessicajaniuk pushed a commit that referenced this pull request Feb 5, 2024
…#54233)

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 #54233
jessicajaniuk pushed a commit that referenced this pull request Feb 5, 2024
…uts (#54233)

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 #54233
jessicajaniuk pushed a commit that referenced this pull request Feb 5, 2024
…es (#54233)

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 #54233
jessicajaniuk pushed a commit that referenced this pull request Feb 5, 2024
…d` (#54233)

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 #54233
jessicajaniuk pushed a commit that referenced this pull request Feb 5, 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 #54233
jessicajaniuk pushed a commit that referenced this pull request Feb 5, 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 #54233
@jessicajaniuk
Copy link
Contributor

This PR was merged into the repository by commit f466c89.

@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.

@angular-automatic-lock-bot angular-automatic-lock-bot bot locked and limited conversation to collaborators Mar 7, 2024
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 detected: feature PR contains a feature commit PullApprove: disable target: patch This PR is targeted for the next patch release
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants