Skip to content

refactor(language-service): integrate let declarations#56270

Closed
crisbeto wants to merge 1 commit intoangular:mainfrom
crisbeto:let-language-service
Closed

refactor(language-service): integrate let declarations#56270
crisbeto wants to merge 1 commit intoangular:mainfrom
crisbeto:let-language-service

Conversation

@crisbeto
Copy link
Copy Markdown
Member

@crisbeto crisbeto commented Jun 5, 2024

Integrates let declarations in the various places within the language service (quick info, completions etc).

Integrates let declarations in the various places within the language service (quick info, completions etc).
@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 Jun 5, 2024
@ngbot ngbot bot modified the milestone: Backlog Jun 5, 2024
@crisbeto crisbeto marked this pull request as ready for review June 5, 2024 07:36
@pullapprove pullapprove bot requested a review from alxhub June 5, 2024 07:37
@crisbeto crisbeto requested review from atscott and dylhunn and removed request for alxhub June 5, 2024 07:37
expectContain(completions, ts.ScriptElementKind.memberVariableElement, ['title', 'hero']);
});

it('should complete a single let declaration without a terminating character', () => {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Can you add a test for completions when there are things on the next line and no terminating character?

@let x = |
<div></div>
{{'hello'}}

It would be good to have similar tests in other features (quick info, etc). The answer might be that this doesn't work, which is fine for now but something I've expressed concern over in the past.

Copy link
Copy Markdown
Member Author

@crisbeto crisbeto Jun 5, 2024

Choose a reason for hiding this comment

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

The answer might be that this doesn't work

I suspect that it won't work, because everything until ;/EOF will be considered part of the @let value which in this case would result in a parser error once it hits the expression parser. Maybe a follow-up could be to only allow multi-line values if there's at least one non-space character on the same like as the =?

}
// TODO(crisbeto): only allow `false` when the syntax is enabled by default.
if (config['enableLetSyntax'] != null) {
options['_enableLetSyntax'] = config['enableLetSyntax'];
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

For later: with allowSignalsInTwoWayBindings, we determine whether to enable it based on an exported symbol. When we release this as a feature not behind a private flag, would it make sense to determine compatibility in the same way and export a symbol from angular/core for the sole purpose of making it visible to the language service/compiler?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Yeah I think so. I'll add it to my list.

@crisbeto crisbeto removed the request for review from dylhunn June 5, 2024 18:43
@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 Jun 5, 2024
thePunderWoman pushed a commit that referenced this pull request Jun 5, 2024
Integrates let declarations in the various places within the language service (quick info, completions etc).

PR Close #56270
@thePunderWoman
Copy link
Copy Markdown
Contributor

This PR was merged into the repository by commit dd869e4.

The changes were merged into the following branches: main, 18.0.x

@angular-automatic-lock-bot
Copy link
Copy Markdown

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 Jul 6, 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: 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

Development

Successfully merging this pull request may close these issues.

3 participants