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.

Sign up for GitHub

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

Jump to bottom

fix(language-service): correctly handle host directive inputs/outputs #48147

Closed
wants to merge 1 commit into from
Closed

fix(language-service): correctly handle host directive inputs/outputs #48147

wants to merge 1 commit into from

Conversation

crisbeto
Copy link
Member

Adds some logic to correctly handle hidden or aliased inputs/outputs in the language service.

Fixes #48102.

crisbeto
crisbeto commented Nov 21, 2022
View reviewed changes
packages/compiler-cli/src/ngtsc/typecheck/src/template_symbol_builder.ts
tsSymbol: symbol.tsSymbol,
exposedInputs: current.inputs,
exposedOutputs: current.outputs,
selector: meta.selector || '',
Copy link
Member Author

Choose a reason for hiding this comment

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

The || '' here is weird, but I don't know the context behind why the language service requires a selector for all directives, even though directives can be declared without selectors.

Copy link
Contributor

Choose a reason for hiding this comment

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

Before the directive composition, a directive couldn't match anything unless it had a selector. I think the correct thing would be to make this optional now. Can you add a TODO or make the change if it's easy enough?

Copy link
Member Author

Choose a reason for hiding this comment

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

I left a TODO for myself. I'll send a follow-up PR once this one goes in.

@crisbeto crisbeto force-pushed the 48102/host-directives-language-service branch from 24e4803 to 4e7ddbc Compare November 21, 2022 15:16
@crisbeto crisbeto added action: review The PR is still awaiting reviews from at least one requested reviewer area: language-service Issues related to Angular's VS Code language service target: patch This PR is targeted for the next patch release labels Nov 21, 2022
@ngbot ngbot bot modified the milestone: Backlog Nov 21, 2022
@crisbeto crisbeto marked this pull request as ready for review November 21, 2022 15:35
atscott
atscott approved these changes Nov 21, 2022
View reviewed changes
Copy link
Contributor

@atscott atscott left a comment

Choose a reason for hiding this comment

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

👍 LGTM!

@crisbeto
fix(language-service): correctly handle host directive inputs/outputs
0d27a64 
Adds some logic to correctly handle hidden or aliased inputs/outputs in the language service.

Fixes angular#48102.
@crisbeto crisbeto force-pushed the 48102/host-directives-language-service branch from 4e7ddbc to 0d27a64 Compare November 21, 2022 21:48
@crisbeto crisbeto 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 Nov 21, 2022
@dylhunn
Copy link
Contributor

dylhunn commented Nov 22, 2022

This PR was merged into the repository by commit fd2eea5.

@dylhunn dylhunn closed this in fd2eea5 Nov 22, 2022
dylhunn pushed a commit that referenced this pull request Nov 22, 2022
@crisbeto @dylhunn
fix(language-service): correctly handle host directive inputs/outputs (…
cc8b76e 
…#48147)

Adds some logic to correctly handle hidden or aliased inputs/outputs in the language service.

Fixes #48102.

PR Close #48147
crisbeto added a commit to crisbeto/angular that referenced this pull request Nov 23, 2022
@crisbeto
refactor(language-service): make selector nullable
0e2004b 
This is a follow-up from angular#48147. Changes the `DirectiveSymbol.selector` to be nullable since it's possible to have directives without a selector.
@crisbeto crisbeto mentioned this pull request Nov 23, 2022
Closed
dylhunn pushed a commit that referenced this pull request Nov 23, 2022
@crisbeto @dylhunn
refactor(language-service): make selector nullable (#48193)
665d465 
This is a follow-up from #48147. Changes the `DirectiveSymbol.selector` to be nullable since it's possible to have directives without a selector.

PR Close #48193
dylhunn pushed a commit that referenced this pull request Nov 23, 2022
@crisbeto @dylhunn
refactor(language-service): make selector nullable (#48193)
0138d75 
This is a follow-up from #48147. Changes the `DirectiveSymbol.selector` to be nullable since it's possible to have directives without a selector.

PR Close #48193
@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 Dec 23, 2022
trekladyone pushed a commit to trekladyone/angular that referenced this pull request Feb 1, 2023
@crisbeto @trekladyone
fix(language-service): correctly handle host directive inputs/outputs (…
a21bbcb 
…angular#48147)

Adds some logic to correctly handle hidden or aliased inputs/outputs in the language service.

Fixes angular#48102.

PR Close angular#48147
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@atscott atscott atscott approved these changes

@aniket-kale aniket-kale aniket-kale approved these changes

Assignees
No one assigned
Labels
action: merge The PR is ready for merge by the caretaker area: language-service Issues related to Angular's VS Code language service target: patch This PR is targeted for the next patch release
Projects
None yet
Milestone
Backlog
Development

Successfully merging this pull request may close these issues.

Language Service doesn't support directive composition API
4 participants
@crisbeto @dylhunn @atscott @aniket-kale