@let bug fixes - #56752
Closed
crisbeto wants to merge 3 commits into
Closed
@let bug fixes#56752crisbeto wants to merge 3 commits into
crisbeto wants to merge 3 commits into
Conversation
Fixes that we were only capturing `@let` declarations at the top level of the scope, not any of the nested children.
crisbeto
added
action: review
crisbeto
commented
Jun 28, 2024
Member
Author
There was a problem hiding this comment.
I don't think that this is the most elegant way of solving it, but I think it's likely the most future-proof. Some alternatives that were considered, but didn't work:
- Iterating the instructions in reverse seems to break cases where variables depend on other variables. Otherwise this would've been the simplest.
- We can't hoist the local
@letvariables to the top, because they need anadvancecall.
crisbeto
marked this pull request as ready for review
June 28, 2024 13:07
dylhunn
approved these changes
Jun 28, 2024
thePunderWoman
approved these changes
Jun 28, 2024
thePunderWoman
left a comment
thePunderWoman
left a comment
Contributor
There was a problem hiding this comment.
reviewed-for: public-api
atscott
approved these changes
Jun 28, 2024
atscott
left a comment
atscott
left a comment
Contributor
There was a problem hiding this comment.
reviewed-for: public-api
crisbeto
added
the
action: cleanup
crisbeto
removed
the
action: review
…cal symbols Expands the check around conflicting `@let` declarations to also cover template variables and local references.
crisbeto
added
action: merge
alxhub
reviewed
Jun 28, 2024
…ones Currently the logic that maps a name to a variable looks at the variables in their definition order. This means that `@let` declarations from parent views will always come before local ones, because the local ones are declared inline whereas the parent ones are hoisted to the top of the function. These changes resolve the issue by giving precedence to the local variables. Fixes angular#56737.
thePunderWoman
pushed a commit
that referenced
this pull request
Jul 1, 2024
…ones (#56752) Currently the logic that maps a name to a variable looks at the variables in their definition order. This means that `@let` declarations from parent views will always come before local ones, because the local ones are declared inline whereas the parent ones are hoisted to the top of the function. These changes resolve the issue by giving precedence to the local variables. Fixes #56737. PR Close #56752
Contributor
|
This PR was merged into the repository by commit 2a1291e. The changes were merged into the following branches: main |
|
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 file contains hidden or 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
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
5 participants
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.
Includes the following fixes related to
@let:fix(compiler-cli): type check let declarations nested inside nodes
Fixes that we were only capturing
@letdeclarations at the top level of the scope, not any of the nested children.fix(compiler-cli): flag all conflicts between let declarations and local symbols
Expands the check around conflicting
@letdeclarations to also cover template variables and local references.fix(compiler): give precedence to local let declarations over parent ones
Currently the logic that maps a name to a variable looks at the variables in their definition order. This means that
@letdeclarations from parent views will always come before local ones, because the local ones are declared inline whereas the parent ones are hoisted to the top of the function.These changes resolve the issue by giving precedence to the local variables.
Fixes #56737.